From f405db22b6ea2cb39a9b063e4759c0f97fa281a6 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Sun, 24 Sep 2023 00:22:30 +0300
Subject: [PATCH 2/5] Fix another bug in GiST index build.

Yet another bug in the ilk of commits a7ee7c851 and 741b88435. In
741b88435, we took care to clear the memorized location of the
downlink when we split the parent page, because splitting the parent
page can move the downlink. But we missed that even *updating* a tuple
on the parent can move it, because updating a tuple on a gist page is
implemented as a delete+insert, so the updated tuple gets moved to the
end of the page. To fix, clear the downlink when we updatie a tuple on
the parent page, even if it's not split.

Backpatch XXX

Reported-by: Alexander Lakhin
Discussion: https://www.postgresql.org/message-id/18129-caca016eaf0c3702@postgresql.org
---
 src/backend/access/gist/gist.c | 37 +++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 10405239027..d4469eb7cc4 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -1272,7 +1272,10 @@ gistinserttuple(GISTInsertState *state, GISTInsertStack *stack,
  *
  * Returns 'true' if the page had to be split. Note that if the page was
  * split, the inserted/updated tuples might've been inserted to a right
- * sibling of stack->buffer instead of stack->buffer itself.
+ * sibling of stack->buffer instead of stack->buffer itself. Even if the page
+ * was not split but an old tuple was updated, any existing tuples on the page
+ * including the one that's updated might move to a different offset on th
+ * epage.
  */
 static bool
 gistinserttuples(GISTInsertState *state, GISTInsertStack *stack,
@@ -1371,7 +1374,8 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack,
 		{
 			/*
 			 * If the parent page was split, the existing downlink might have
-			 * moved.
+			 * moved. This assumes that gistinsettuples() doesn't move the
+			 * tuples around unless the page is split.
 			 */
 			stack->downlinkoffnum = InvalidOffsetNumber;
 		}
@@ -1389,20 +1393,21 @@ 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;
-	}
+	(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);
 
-- 
2.39.2