1 files changed, 100 insertions, 80 deletions

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index b1f19f6a8e6..8ef5fa03290 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -1018,87 +1018,106 @@ gistFindPath(Relation r, BlockNumber child, OffsetNumber *downlinkoffnum)
  * remain so at exit, but it might not be the same page anymore.
  */
 static void
-gistFindCorrectParent(Relation r, GISTInsertStack *child)
+gistFindCorrectParent(Relation r, GISTInsertStack *child, bool is_build)
 {
 	GISTInsertStack *parent = child->parent;
+	ItemId		iid;
+	IndexTuple	idxtuple;
+	OffsetNumber maxoff;
+	GISTInsertStack *ptr;
 
 	gistcheckpage(r, parent->buffer);
 	parent->page = (Page) BufferGetPage(parent->buffer);
+	maxoff = PageGetMaxOffsetNumber(parent->page);
 
-	/* here we don't need to distinguish between split and page update */
-	if (child->downlinkoffnum == InvalidOffsetNumber ||
-		parent->lsn != PageGetLSN(parent->page))
+	/* Check if the downlink is still where it was before */
+	if (child->downlinkoffnum != InvalidOffsetNumber && child->downlinkoffnum <= maxoff)
 	{
-		/* parent is changed, look child in right links until found */
-		OffsetNumber i,
-					maxoff;
-		ItemId		iid;
-		IndexTuple	idxtuple;
-		GISTInsertStack *ptr;
+		iid = PageGetItemId(parent->page, child->downlinkoffnum);
+		idxtuple = (IndexTuple) PageGetItem(parent->page, iid);
+		if (ItemPointerGetBlockNumber(&(idxtuple->t_tid)) == child->blkno)
+			return;				/* still there */
+	}
 
-		while (true)
-		{
-			maxoff = PageGetMaxOffsetNumber(parent->page);
-			for (i = FirstOffsetNumber; i <= maxoff; i = OffsetNumberNext(i))
-			{
-				iid = PageGetItemId(parent->page, i);
-				idxtuple = (IndexTuple) PageGetItem(parent->page, iid);
-				if (ItemPointerGetBlockNumber(&(idxtuple->t_tid)) == child->blkno)
-				{
-					/* yes!!, found */
-					child->downlinkoffnum = i;
-					return;
-				}
-			}
+	/*
+	 * The page has changed since we looked. During normal operation, every
+	 * update of a page changes its LSN, so the LSN we memorized should have
+	 * changed too. During index build, however, we don't WAL-log the changes
+	 * until we have built the index, so the LSN doesn't change. There is no
+	 * concurrent activity during index build, but we might have changed the
+	 * parent ourselves.
+	 */
+	Assert(parent->lsn != PageGetLSN(parent->page) || is_build);
+
+	/*
+	 * Scan the page to re-find the downlink. If the page was split, it might
+	 * have moved to a different page, so follow the right links until we find
+	 * it.
+	 */
+	while (true)
+	{
+		OffsetNumber i;
 
-			parent->blkno = GistPageGetOpaque(parent->page)->rightlink;
-			UnlockReleaseBuffer(parent->buffer);
-			if (parent->blkno == InvalidBlockNumber)
+		maxoff = PageGetMaxOffsetNumber(parent->page);
+		for (i = FirstOffsetNumber; i <= maxoff; i = OffsetNumberNext(i))
+		{
+			iid = PageGetItemId(parent->page, i);
+			idxtuple = (IndexTuple) PageGetItem(parent->page, iid);
+			if (ItemPointerGetBlockNumber(&(idxtuple->t_tid)) == child->blkno)
 			{
-				/*
-				 * End of chain and still didn't find parent. It's a very-very
-				 * rare situation when root splitted.
-				 */
-				break;
+				/* yes!!, found */
+				child->downlinkoffnum = i;
+				return;
 			}
-			parent->buffer = ReadBuffer(r, parent->blkno);
-			LockBuffer(parent->buffer, GIST_EXCLUSIVE);
-			gistcheckpage(r, parent->buffer);
-			parent->page = (Page) BufferGetPage(parent->buffer);
 		}
 
-		/*
-		 * awful!!, we need search tree to find parent ... , but before we
-		 * should release all old parent
-		 */
-
-		ptr = child->parent->parent;	/* child->parent already released
-										 * above */
-		while (ptr)
+		parent->blkno = GistPageGetOpaque(parent->page)->rightlink;
+		parent->downlinkoffnum = InvalidOffsetNumber;
+		UnlockReleaseBuffer(parent->buffer);
+		if (parent->blkno == InvalidBlockNumber)
 		{
-			ReleaseBuffer(ptr->buffer);
-			ptr = ptr->parent;
+			/*
+			 * End of chain and still didn't find parent. It's a very-very
+			 * rare situation when root splitted.
+			 */
+			break;
 		}
+		parent->buffer = ReadBuffer(r, parent->blkno);
+		LockBuffer(parent->buffer, GIST_EXCLUSIVE);
+		gistcheckpage(r, parent->buffer);
+		parent->page = (Page) BufferGetPage(parent->buffer);
+	}
 
-		/* ok, find new path */
-		ptr = parent = gistFindPath(r, child->blkno, &child->downlinkoffnum);
+	/*
+	 * awful!!, we need search tree to find parent ... , but before we should
+	 * release all old parent
+	 */
 
-		/* read all buffers as expected by caller */
-		/* note we don't lock them or gistcheckpage them here! */
-		while (ptr)
-		{
-			ptr->buffer = ReadBuffer(r, ptr->blkno);
-			ptr->page = (Page) BufferGetPage(ptr->buffer);
-			ptr = ptr->parent;
-		}
+	ptr = child->parent->parent;	/* child->parent already released above */
+	while (ptr)
+	{
+		ReleaseBuffer(ptr->buffer);
+		ptr = ptr->parent;
+	}
 
-		/* install new chain of parents to stack */
-		child->parent = parent;
+	/* ok, find new path */
+	ptr = parent = gistFindPath(r, child->blkno, &child->downlinkoffnum);
 
-		/* make recursive call to normal processing */
-		LockBuffer(child->parent->buffer, GIST_EXCLUSIVE);
-		gistFindCorrectParent(r, child);
+	/* read all buffers as expected by caller */
+	/* note we don't lock them or gistcheckpage them here! */
+	while (ptr)
+	{
+		ptr->buffer = ReadBuffer(r, ptr->blkno);
+		ptr->page = (Page) BufferGetPage(ptr->buffer);
+		ptr = ptr->parent;
 	}
+
+	/* install new chain of parents to stack */
+	child->parent = parent;
+
+	/* make recursive call to normal processing */
+	LockBuffer(child->parent->buffer, GIST_EXCLUSIVE);
+	gistFindCorrectParent(r, child, is_build);
 }
 
 /*
@@ -1106,7 +1125,7 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child)
  */
 static IndexTuple
 gistformdownlink(Relation rel, Buffer buf, GISTSTATE *giststate,
-				 GISTInsertStack *stack)
+				 GISTInsertStack *stack, bool is_build)
 {
 	Page		page = BufferGetPage(buf);
 	OffsetNumber maxoff;
@@ -1147,7 +1166,7 @@ gistformdownlink(Relation rel, Buffer buf, GISTSTATE *giststate,
 		ItemId		iid;
 
 		LockBuffer(stack->parent->buffer, GIST_EXCLUSIVE);
-		gistFindCorrectParent(rel, stack);
+		gistFindCorrectParent(rel, stack, is_build);
 		iid = PageGetItemId(stack->parent->page, stack->downlinkoffnum);
 		downlink = (IndexTuple) PageGetItem(stack->parent->page, iid);
 		downlink = CopyIndexTuple(downlink);
@@ -1193,7 +1212,7 @@ gistfixsplit(GISTInsertState *state, GISTSTATE *giststate)
 		page = BufferGetPage(buf);
 
 		/* Form the new downlink tuples to insert to parent */
-		downlink = gistformdownlink(state->r, buf, giststate, stack);
+		downlink = gistformdownlink(state->r, buf, giststate, stack, state->is_build);
 
 		si->buf = buf;
 		si->downlink = downlink;
@@ -1347,7 +1366,7 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack,
 		right = (GISTPageSplitInfo *) list_nth(splitinfo, pos);
 		left = (GISTPageSplitInfo *) list_nth(splitinfo, pos - 1);
 
-		gistFindCorrectParent(state->r, stack);
+		gistFindCorrectParent(state->r, stack, state->is_build);
 		if (gistinserttuples(state, stack->parent, giststate,
 							 &right->downlink, 1,
 							 InvalidOffsetNumber,
@@ -1372,21 +1391,22 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack,
 	 */
 	tuples[0] = left->downlink;
 	tuples[1] = right->downlink;
-	gistFindCorrectParent(state->r, stack);
-	if (gistinserttuples(state, stack->parent, giststate,
-						 tuples, 2,
-						 stack->downlinkoffnum,
-						 left->buf, right->buf,
-						 true,	/* Unlock parent */
-						 unlockbuf	/* Unlock stack->buffer if caller wants
-									 * that */
-						 ))
-	{
-		/*
-		 * If the parent page was split, the downlink might have moved.
-		 */
-		stack->downlinkoffnum = InvalidOffsetNumber;
-	}
+	gistFindCorrectParent(state->r, stack, state->is_build);
+	(void) gistinserttuples(state, stack->parent, giststate,
+							tuples, 2,
+							stack->downlinkoffnum,
+							left->buf, right->buf,
+							true,	/* Unlock parent */
+							unlockbuf	/* Unlock stack->buffer if caller
+										 * wants that */
+		);
+
+	/*
+	 * The downlink might have moved when we updated it. Even if the page
+	 * wasn't split, because gistinserttuples() implements updating the old
+	 * tuple by removing and re-inserting it!
+	 */
+	stack->downlinkoffnum = InvalidOffsetNumber;
 
 	Assert(left->buf == stack->buffer);