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)