Refactor checks for deleted GiST pages.

The explicit check in gistScanPage() isn't currently really necessary, as
a deleted page is always empty, so the loop would fall through without
doing anything, anyway. But it's a marginal optimization, and it gives a
nice place to attach a comment to explain how it works.

Backpatch to v12, where GiST page deletion was introduced.

Reviewed-by: Andrey Borodin
Discussion: https://www.postgresql.org/message-id/835A15A5-F1B4-4446-A711-BF48357EB602%40yandex-team.ru

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 169bf6fcfed316e0a50a66e7b60015376b09198c..e9ca4b825279e5e6f48faace1cbc6ee1b3cc271a 100644 (file)
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -709,14 +709,15 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace,
                        continue;
                }
 
-               if (stack->blkno != GIST_ROOT_BLKNO &&
-                       stack->parent->lsn < GistPageGetNSN(stack->page))
+               if ((stack->blkno != GIST_ROOT_BLKNO &&
+                        stack->parent->lsn < GistPageGetNSN(stack->page)) ||
+                       GistPageIsDeleted(stack->page))
                {
                        /*
-                        * Concurrent split detected. There's no guarantee that the
-                        * downlink for this page is consistent with the tuple we're
-                        * inserting anymore, so go back to parent and rechoose the best
-                        * child.
+                        * Concurrent split or page deletion detected. There's no
+                        * guarantee that the downlink for this page is consistent with
+                        * the tuple we're inserting anymore, so go back to parent and
+                        * rechoose the best child.
                         */
                        UnlockReleaseBuffer(stack->buffer);
                        xlocked = false;
@@ -735,9 +736,6 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace,
                        GISTInsertStack *item;
                        OffsetNumber downlinkoffnum;
 
-                       /* currently, internal pages are never deleted */
-                       Assert(!GistPageIsDeleted(stack->page));
-
                        downlinkoffnum = gistchoose(state.r, stack->page, itup, giststate);
                        iid = PageGetItemId(stack->page, downlinkoffnum);
                        idxtuple = (IndexTuple) PageGetItem(stack->page, iid);
@@ -858,12 +856,13 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace,
                                         * leaf/inner is enough to recognize split for root
                                         */
                                }
-                               else if (GistFollowRight(stack->page) ||
-                                                stack->parent->lsn < GistPageGetNSN(stack->page))
+                               else if ((GistFollowRight(stack->page) ||
+                                                 stack->parent->lsn < GistPageGetNSN(stack->page)) &&
+                                                GistPageIsDeleted(stack->page))
                                {
                                        /*
-                                        * The page was split while we momentarily unlocked the
-                                        * page. Go back to parent.
+                                        * The page was split or deleted while we momentarily
+                                        * unlocked the page. Go back to parent.
                                         */
                                        UnlockReleaseBuffer(stack->buffer);
                                        xlocked = false;
@@ -872,18 +871,6 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace,
                                }
                        }
 
-                       /*
-                        * The page might have been deleted after we scanned the parent
-                        * and saw the downlink.
-                        */
-                       if (GistPageIsDeleted(stack->page))
-                       {
-                               UnlockReleaseBuffer(stack->buffer);
-                               xlocked = false;
-                               state.stack = stack = stack->parent;
-                               continue;
-                       }
-
                        /* now state.stack->(page, buffer and blkno) points to leaf page */
 
                        gistinserttuple(&state, stack, giststate, itup,
@@ -947,6 +934,9 @@ gistFindPath(Relation r, BlockNumber child, OffsetNumber *downlinkoffnum)
                        break;
                }
 
+               /* currently, internal pages are never deleted */
+               Assert(!GistPageIsDeleted(page));
+
                top->lsn = BufferGetLSNAtomic(buffer);
 
                /*

diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c
index 46d08e0635042b79d3656f2578ff8775b8aa0782..4e0c500a22e093e764b27b4931af82f9445e37e4 100644 (file)
--- a/src/backend/access/gist/gistget.c
+++ b/src/backend/access/gist/gistget.c
@@ -377,6 +377,20 @@ gistScanPage(IndexScanDesc scan, GISTSearchItem *pageItem, double *myDistances,
                MemoryContextSwitchTo(oldcxt);
        }
 
+       /*
+        * Check if the page was deleted after we saw the downlink. There's
+        * nothing of interest on a deleted page. Note that we must do this
+        * after checking the NSN for concurrent splits! It's possible that
+        * the page originally contained some tuples that are visible to us,
+        * but was split so that all the visible tuples were moved to another
+        * page, and then this page was deleted.
+        */
+       if (GistPageIsDeleted(page))
+       {
+               UnlockReleaseBuffer(buffer);
+               return;
+       }
+
        so->nPageData = so->curPageData = 0;
        scan->xs_hitup = NULL;          /* might point into pageDataCxt */
        if (so->pageDataCxt)