Fix GiST buffering build bug, which caused "failed to re-find parent" errors.

We use a hash table to track the parents of inner pages, but when inserting
to a leaf page, the caller of gistbufferinginserttuples() must pass a
correct block number of the leaf's parent page. Before gistProcessItup()
descends to a child page, it checks if the downlink needs to be adjusted to
accommodate the new tuple, and updates the downlink if necessary. However,
updating the downlink might require splitting the page, which might move the
downlink to a page to the right. gistProcessItup() doesn't realize that, so
when it descends to the leaf page, it might pass an out-of-date parent block
number as a result. Fix that by returning the block a tuple was inserted to
from gistbufferinginserttuples().

This fixes the bug reported by Zdeněk Jílovec.
This commit is contained in:
Heikki Linnakangas 2012-08-16 12:42:11 +03:00
parent de3773d951
commit 89911b3ab8
3 changed files with 41 additions and 14 deletions

View File

@ -148,16 +148,22 @@ gistinsert(PG_FUNCTION_ARGS)
* pages are released; note that new tuple(s) are *not* on the root page * pages are released; note that new tuple(s) are *not* on the root page
* but in one of the new child pages. * but in one of the new child pages.
* *
* If 'newblkno' is not NULL, returns the block number of page the first
* new/updated tuple was inserted to. Usually it's the given page, but could
* be its right sibling if the page was split.
*
* Returns 'true' if the page was split, 'false' otherwise. * Returns 'true' if the page was split, 'false' otherwise.
*/ */
bool bool
gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate,
Buffer buffer, Buffer buffer,
IndexTuple *itup, int ntup, OffsetNumber oldoffnum, IndexTuple *itup, int ntup, OffsetNumber oldoffnum,
BlockNumber *newblkno,
Buffer leftchildbuf, Buffer leftchildbuf,
List **splitinfo, List **splitinfo,
bool markfollowright) bool markfollowright)
{ {
BlockNumber blkno = BufferGetBlockNumber(buffer);
Page page = BufferGetPage(buffer); Page page = BufferGetPage(buffer);
bool is_leaf = (GistPageIsLeaf(page)) ? true : false; bool is_leaf = (GistPageIsLeaf(page)) ? true : false;
XLogRecPtr recptr; XLogRecPtr recptr;
@ -199,7 +205,6 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate,
BlockNumber oldrlink = InvalidBlockNumber; BlockNumber oldrlink = InvalidBlockNumber;
GistNSN oldnsn = 0; GistNSN oldnsn = 0;
SplitedPageLayout rootpg; SplitedPageLayout rootpg;
BlockNumber blkno = BufferGetBlockNumber(buffer);
bool is_rootsplit; bool is_rootsplit;
is_rootsplit = (blkno == GIST_ROOT_BLKNO); is_rootsplit = (blkno == GIST_ROOT_BLKNO);
@ -319,9 +324,19 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate,
for (i = 0; i < ptr->block.num; i++) for (i = 0; i < ptr->block.num; i++)
{ {
if (PageAddItem(ptr->page, (Item) data, IndexTupleSize((IndexTuple) data), i + FirstOffsetNumber, false, false) == InvalidOffsetNumber) IndexTuple thistup = (IndexTuple) data;
if (PageAddItem(ptr->page, (Item) data, IndexTupleSize(thistup), i + FirstOffsetNumber, false, false) == InvalidOffsetNumber)
elog(ERROR, "failed to add item to index page in \"%s\"", RelationGetRelationName(rel)); elog(ERROR, "failed to add item to index page in \"%s\"", RelationGetRelationName(rel));
data += IndexTupleSize((IndexTuple) data);
/*
* If this is the first inserted/updated tuple, let the caller
* know which page it landed on.
*/
if (newblkno && ItemPointerEquals(&thistup->t_tid, &(*itup)->t_tid))
*newblkno = ptr->block.blkno;
data += IndexTupleSize(thistup);
} }
/* Set up rightlinks */ /* Set up rightlinks */
@ -436,6 +451,9 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate,
recptr = GetXLogRecPtrForTemp(); recptr = GetXLogRecPtrForTemp();
PageSetLSN(page, recptr); PageSetLSN(page, recptr);
} }
if (newblkno)
*newblkno = blkno;
} }
/* /*
@ -1074,9 +1092,9 @@ gistinserttuple(GISTInsertState *state, GISTInsertStack *stack,
* is kept pinned. * is kept pinned.
* - Lock and pin on 'rightchild' are always released. * - Lock and pin on 'rightchild' are always released.
* *
* Returns 'true' if the page had to be split. Note that if the page had * Returns 'true' if the page had to be split. Note that if the page was
* be split, the inserted/updated might've been inserted to a right sibling * split, the inserted/updated tuples might've been inserted to a right
* of stack->buffer instead of stack->buffer itself. * sibling of stack->buffer instead of stack->buffer itself.
*/ */
static bool static bool
gistinserttuples(GISTInsertState *state, GISTInsertStack *stack, gistinserttuples(GISTInsertState *state, GISTInsertStack *stack,
@ -1091,7 +1109,8 @@ gistinserttuples(GISTInsertState *state, GISTInsertStack *stack,
/* Insert the tuple(s) to the page, splitting the page if necessary */ /* Insert the tuple(s) to the page, splitting the page if necessary */
is_split = gistplacetopage(state->r, state->freespace, giststate, is_split = gistplacetopage(state->r, state->freespace, giststate,
stack->buffer, stack->buffer,
tuples, ntup, oldoffnum, tuples, ntup,
oldoffnum, NULL,
leftchild, leftchild,
&splitinfo, &splitinfo,
true); true);

View File

@ -85,7 +85,7 @@ static void gistBufferingBuildInsert(GISTBuildState *buildstate,
IndexTuple itup); IndexTuple itup);
static bool gistProcessItup(GISTBuildState *buildstate, IndexTuple itup, static bool gistProcessItup(GISTBuildState *buildstate, IndexTuple itup,
BlockNumber startblkno, int startlevel); BlockNumber startblkno, int startlevel);
static void gistbufferinginserttuples(GISTBuildState *buildstate, static BlockNumber gistbufferinginserttuples(GISTBuildState *buildstate,
Buffer buffer, int level, Buffer buffer, int level,
IndexTuple *itup, int ntup, OffsetNumber oldoffnum, IndexTuple *itup, int ntup, OffsetNumber oldoffnum,
BlockNumber parentblk, OffsetNumber downlinkoffnum); BlockNumber parentblk, OffsetNumber downlinkoffnum);
@ -621,9 +621,9 @@ gistProcessItup(GISTBuildState *buildstate, IndexTuple itup,
newtup = gistgetadjusted(indexrel, idxtuple, itup, giststate); newtup = gistgetadjusted(indexrel, idxtuple, itup, giststate);
if (newtup) if (newtup)
{ {
gistbufferinginserttuples(buildstate, buffer, level, blkno = gistbufferinginserttuples(buildstate, buffer, level,
&newtup, 1, childoffnum, &newtup, 1, childoffnum,
InvalidBlockNumber, InvalidOffsetNumber); InvalidBlockNumber, InvalidOffsetNumber);
/* gistbufferinginserttuples() released the buffer */ /* gistbufferinginserttuples() released the buffer */
} }
else else
@ -676,10 +676,14 @@ gistProcessItup(GISTBuildState *buildstate, IndexTuple itup,
* *
* This is analogous with gistinserttuples() in the regular insertion code. * This is analogous with gistinserttuples() in the regular insertion code.
* *
* Returns the block number of the page where the (first) new or updated tuple
* was inserted. Usually that's the original page, but might be a sibling page
* if the original page was split.
*
* Caller should hold a lock on 'buffer' on entry. This function will unlock * Caller should hold a lock on 'buffer' on entry. This function will unlock
* and unpin it. * and unpin it.
*/ */
static void static BlockNumber
gistbufferinginserttuples(GISTBuildState *buildstate, Buffer buffer, int level, gistbufferinginserttuples(GISTBuildState *buildstate, Buffer buffer, int level,
IndexTuple *itup, int ntup, OffsetNumber oldoffnum, IndexTuple *itup, int ntup, OffsetNumber oldoffnum,
BlockNumber parentblk, OffsetNumber downlinkoffnum) BlockNumber parentblk, OffsetNumber downlinkoffnum)
@ -687,12 +691,13 @@ gistbufferinginserttuples(GISTBuildState *buildstate, Buffer buffer, int level,
GISTBuildBuffers *gfbb = buildstate->gfbb; GISTBuildBuffers *gfbb = buildstate->gfbb;
List *splitinfo; List *splitinfo;
bool is_split; bool is_split;
BlockNumber placed_to_blk = InvalidBlockNumber;
is_split = gistplacetopage(buildstate->indexrel, is_split = gistplacetopage(buildstate->indexrel,
buildstate->freespace, buildstate->freespace,
buildstate->giststate, buildstate->giststate,
buffer, buffer,
itup, ntup, oldoffnum, itup, ntup, oldoffnum, &placed_to_blk,
InvalidBuffer, InvalidBuffer,
&splitinfo, &splitinfo,
false); false);
@ -823,6 +828,8 @@ gistbufferinginserttuples(GISTBuildState *buildstate, Buffer buffer, int level,
} }
else else
UnlockReleaseBuffer(buffer); UnlockReleaseBuffer(buffer);
return placed_to_blk;
} }
/* /*

View File

@ -430,7 +430,8 @@ typedef struct
extern bool gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, extern bool gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate,
Buffer buffer, Buffer buffer,
IndexTuple *itup, int ntup, OffsetNumber oldoffnum, IndexTuple *itup, int ntup,
OffsetNumber oldoffnum, BlockNumber *newblkno,
Buffer leftchildbuf, Buffer leftchildbuf,
List **splitinfo, List **splitinfo,
bool markleftchild); bool markleftchild);