From 687708781e8f43a09a2a6bcd55b6d8cbb3b10d53 Mon Sep 17 00:00:00 2001
From: Rui Zhao <zhaorui126@gmail.com>
Date: Thu, 20 Aug 2026 00:10:23 +0800
Subject: [PATCH] Write stashed update-chain tuples with their own TOAST flags

rewrite_heap_tuple() stashes the prior version of an update pair in
rs_unresolved_tups and writes it out when the successor arrives, but
the write reused the successor's flags: a live successor resolves a
recently-dead prior version with flags = 0, and detoasting the prior
version's already-reclaimed chunks fails with "missing chunk".  The
same applied to the leftovers end_heap_rewrite() writes, which passed
a hardcoded 0.

Keep the flags a tuple was scanned with in its UnresolvedTupData and
use them at both write-out sites.  A failed insert of a stashed tuple
cannot be reported through rewrite_heap_tuple()'s return value -- the
caller's tuple has already been written, and the caller would adjust
the counters for the wrong tuple -- so drop the tuple on the spot and
have end_heap_rewrite() return the number of such drops, which the
caller folds into tups_vacuumed / num_tuples / tups_recently_dead.

Add UPDATE permutations for VACUUM FULL and REPACK to the
rewrite_stale_xmin isolation spec; they fail with "missing chunk"
without this fix.
---
 .../dblink/expected/rewrite_stale_xmin.out    | 118 ++++++++++++++++++
 contrib/dblink/specs/rewrite_stale_xmin.spec  |  27 ++++
 src/backend/access/heap/heapam_handler.c      |  15 ++-
 src/backend/access/heap/rewriteheap.c         |  44 ++++++-
 src/include/access/rewriteheap.h              |   2 +-
 5 files changed, 199 insertions(+), 7 deletions(-)

diff --git a/contrib/dblink/expected/rewrite_stale_xmin.out b/contrib/dblink/expected/rewrite_stale_xmin.out
index 2e5a7940ea2..ff600edd68d 100644
--- a/contrib/dblink/expected/rewrite_stale_xmin.out
+++ b/contrib/dblink/expected/rewrite_stale_xmin.out
@@ -235,3 +235,121 @@ dblink_disconnect
 OK               
 (1 row)
 
+
+starting permutation: s1_hold_xmin s2_begin s3_update s3_vacuum_main_only s2_commit s3_vacuum_toast s3_vacuum_full s1_release
+step s1_hold_xmin: 
+    SELECT dblink_connect('holder',
+        'dbname=regress_other_db port=' || current_setting('port'));
+    SELECT dblink_exec('holder', 'BEGIN');
+    SELECT dblink_exec('holder', 'CREATE TEMP TABLE xid_holder(x int)');
+
+dblink_connect
+--------------
+OK            
+(1 row)
+
+dblink_exec
+-----------
+BEGIN      
+(1 row)
+
+dblink_exec 
+------------
+CREATE TABLE
+(1 row)
+
+step s2_begin: 
+    BEGIN ISOLATION LEVEL REPEATABLE READ;
+    SELECT 1;
+
+?column?
+--------
+       1
+(1 row)
+
+step s3_update: 
+    UPDATE rewrite_test SET data = repeat('z', 2500) WHERE id = 1;
+
+step s3_vacuum_main_only: 
+    VACUUM (PROCESS_TOAST false) rewrite_test;
+
+step s2_commit: COMMIT;
+step s3_vacuum_toast: 
+    VACUUM pg_toast.rewrite_test_toast;
+
+step s3_vacuum_full: 
+    VACUUM FULL rewrite_test;
+
+step s1_release: 
+    SELECT dblink_exec('holder', 'COMMIT');
+    SELECT dblink_disconnect('holder');
+
+dblink_exec
+-----------
+COMMIT     
+(1 row)
+
+dblink_disconnect
+-----------------
+OK               
+(1 row)
+
+
+starting permutation: s1_hold_xmin s2_begin s3_update s3_vacuum_main_only s2_commit s3_vacuum_toast s3_repack s1_release
+step s1_hold_xmin: 
+    SELECT dblink_connect('holder',
+        'dbname=regress_other_db port=' || current_setting('port'));
+    SELECT dblink_exec('holder', 'BEGIN');
+    SELECT dblink_exec('holder', 'CREATE TEMP TABLE xid_holder(x int)');
+
+dblink_connect
+--------------
+OK            
+(1 row)
+
+dblink_exec
+-----------
+BEGIN      
+(1 row)
+
+dblink_exec 
+------------
+CREATE TABLE
+(1 row)
+
+step s2_begin: 
+    BEGIN ISOLATION LEVEL REPEATABLE READ;
+    SELECT 1;
+
+?column?
+--------
+       1
+(1 row)
+
+step s3_update: 
+    UPDATE rewrite_test SET data = repeat('z', 2500) WHERE id = 1;
+
+step s3_vacuum_main_only: 
+    VACUUM (PROCESS_TOAST false) rewrite_test;
+
+step s2_commit: COMMIT;
+step s3_vacuum_toast: 
+    VACUUM pg_toast.rewrite_test_toast;
+
+step s3_repack: 
+    REPACK rewrite_test;
+
+step s1_release: 
+    SELECT dblink_exec('holder', 'COMMIT');
+    SELECT dblink_disconnect('holder');
+
+dblink_exec
+-----------
+COMMIT     
+(1 row)
+
+dblink_disconnect
+-----------------
+OK               
+(1 row)
+
diff --git a/contrib/dblink/specs/rewrite_stale_xmin.spec b/contrib/dblink/specs/rewrite_stale_xmin.spec
index fe88e789f85..b670c6b4435 100644
--- a/contrib/dblink/specs/rewrite_stale_xmin.spec
+++ b/contrib/dblink/specs/rewrite_stale_xmin.spec
@@ -65,6 +65,10 @@ step s3_delete
 {
     DELETE FROM rewrite_test WHERE id = 1;
 }
+step s3_update
+{
+    UPDATE rewrite_test SET data = repeat('z', 2500) WHERE id = 1;
+}
 step s3_vacuum_main_only
 {
     VACUUM (PROCESS_TOAST false) rewrite_test;
@@ -135,3 +139,26 @@ permutation
     s3_vacuum_toast
     s3_create_index
     s1_release
+
+# VACUUM FULL after an UPDATE: the prior version of the update pair reaches
+# the new heap through rs_unresolved_tups
+permutation
+    s1_hold_xmin
+    s2_begin
+    s3_update
+    s3_vacuum_main_only
+    s2_commit
+    s3_vacuum_toast
+    s3_vacuum_full
+    s1_release
+
+# REPACK after an UPDATE
+permutation
+    s1_hold_xmin
+    s2_begin
+    s3_update
+    s3_vacuum_main_only
+    s2_commit
+    s3_vacuum_toast
+    s3_repack
+    s1_release
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 8c7a67ec3e1..0444bb53ace 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -1009,7 +1009,20 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap,
 
 	/* Write out any remaining tuples, and fsync if needed */
 	if (rwstate)
-		end_heap_rewrite(rwstate);
+	{
+		double		tuples_dropped;
+
+		tuples_dropped = end_heap_rewrite(rwstate);
+
+		/*
+		 * Tuples the rewrite dropped because their TOAST data was already
+		 * reclaimed were all scanned as recently dead: count them as
+		 * vacuumed instead.
+		 */
+		*tups_vacuumed += tuples_dropped;
+		*num_tuples -= tuples_dropped;
+		*tups_recently_dead -= tuples_dropped;
+	}
 	if (bistate)
 		FreeBulkInsertState(bistate);
 
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index c68333ed03f..467a9f8d089 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -149,6 +149,8 @@ typedef struct RewriteStateData
 	XLogRecPtr	rs_begin_lsn;	/* XLogInsertLsn when starting the rewrite */
 	HTAB	   *rs_unresolved_tups; /* unmatched A tuples */
 	HTAB	   *rs_old_new_tid_map; /* unmatched B tuples */
+	double		rs_tuples_dropped;	/* stashed tuples dropped because their
+									 * TOAST data was already reclaimed */
 	HTAB	   *rs_logical_mappings;	/* logical remapping files */
 	uint32		rs_num_rewrite_mappings;	/* # in memory mappings */
 } RewriteStateData;
@@ -173,6 +175,7 @@ typedef struct
 	TidHashKey	key;			/* expected xmin/old location of B tuple */
 	ItemPointerData old_tid;	/* A's location in the old heap */
 	HeapTuple	tuple;			/* A's tuple contents */
+	uint32		flags;			/* raw_heap_insert flags A was scanned with */
 } UnresolvedTupData;
 
 typedef UnresolvedTupData *UnresolvedTup;
@@ -294,23 +297,31 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm
  * End a rewrite.
  *
  * state and any other resources are freed.
+ *
+ * Returns the number of tuples the rewrite dropped because their TOAST
+ * data had already been reclaimed.  They were all scanned as recently
+ * dead, so the caller should count them as vacuumed rather than kept.
  */
-void
+double
 end_heap_rewrite(RewriteState state)
 {
 	HASH_SEQ_STATUS seq_status;
 	UnresolvedTup unresolved;
+	double		tuples_dropped;
 
 	/*
 	 * Write any remaining tuples in the UnresolvedTups table. If we have any
 	 * left, they should in fact be dead, but let's err on the safe side.
+	 * Each of them was scanned as recently dead, so write it with the flags
+	 * it was scanned with and drop it if its TOAST data is gone.
 	 */
 	hash_seq_init(&seq_status, state->rs_unresolved_tups);
 
 	while ((unresolved = hash_seq_search(&seq_status)) != NULL)
 	{
 		ItemPointerSetInvalid(&unresolved->tuple->t_data->t_ctid);
-		raw_heap_insert(state, unresolved->tuple, 0);
+		if (!raw_heap_insert(state, unresolved->tuple, unresolved->flags))
+			state->rs_tuples_dropped += 1;
 	}
 
 	/* Write the last page, if any */
@@ -324,8 +335,12 @@ end_heap_rewrite(RewriteState state)
 
 	logical_end_heap_rewrite(state);
 
+	tuples_dropped = state->rs_tuples_dropped;
+
 	/* Deleting the context frees everything */
 	MemoryContextDelete(state->rs_cxt);
+
+	return tuples_dropped;
 }
 
 /*
@@ -433,6 +448,7 @@ rewrite_heap_tuple(RewriteState state,
 
 			unresolved->old_tid = old_tuple->t_self;
 			unresolved->tuple = heap_copytuple(new_tuple);
+			unresolved->flags = flags;
 
 			/*
 			 * We can't do anything more now, since we don't know where the
@@ -459,10 +475,27 @@ rewrite_heap_tuple(RewriteState state,
 		/* Insert the tuple and find out where it's put in new_heap */
 		if (!raw_heap_insert(state, new_tuple, flags))
 		{
-			if (free_new)
-				heap_freetuple(new_tuple);
+			if (!free_new)
+			{
+				/* The caller's own tuple was skipped: report it */
+				MemoryContextSwitchTo(old_cxt);
+				return false;
+			}
+
+			/*
+			 * A stashed prior version.  The tuple this call was made for has
+			 * already been written, so the failure cannot be reported to the
+			 * caller; drop the stashed tuple here instead.  Only tuples
+			 * scanned as recently dead are stashed, so this is the same
+			 * treatment the caller gives its own tuple, and the drop is
+			 * counted for end_heap_rewrite() to report.  If a still older
+			 * version is waiting for this one in rs_unresolved_tups, it will
+			 * be written out by end_heap_rewrite().
+			 */
+			state->rs_tuples_dropped += 1;
+			heap_freetuple(new_tuple);
 			MemoryContextSwitchTo(old_cxt);
-			return false;
+			return true;
 		}
 		new_tid = new_tuple->t_self;
 
@@ -502,6 +535,7 @@ rewrite_heap_tuple(RewriteState state,
 				if (free_new)
 					heap_freetuple(new_tuple);
 				new_tuple = unresolved->tuple;
+				flags = unresolved->flags;
 				free_new = true;
 				old_tid = unresolved->old_tid;
 				new_tuple->t_data->t_ctid = new_tid;
diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h
index 09110f6337a..aba8dd462df 100644
--- a/src/include/access/rewriteheap.h
+++ b/src/include/access/rewriteheap.h
@@ -24,7 +24,7 @@ typedef struct RewriteStateData *RewriteState;
 extern RewriteState begin_heap_rewrite(Relation old_heap, Relation new_heap,
 									   TransactionId oldest_xmin, TransactionId freeze_xid,
 									   MultiXactId cutoff_multi);
-extern void end_heap_rewrite(RewriteState state);
+extern double end_heap_rewrite(RewriteState state);
 
 /* See toast_helper.h for the values of "flags" */
 extern bool rewrite_heap_tuple(RewriteState state, HeapTuple old_tuple,
-- 
2.43.7

