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
This commit is contained in:
Heikki Linnakangas 2019-07-24 20:24:05 +03:00
parent 1a721248f3
commit 9eb5607e69
2 changed files with 29 additions and 25 deletions

View File

@ -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);
/*

View File

@ -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)