From 239d268615a94b18febd82a42cfe019072ff69b8 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
Date: Tue, 6 Oct 2026 14:54:51 +0500
Subject: [PATCH v2 2/2] Detect incorrect committed transaction hints in
 verify_heapam()

Incorrect committed hints can expose aborted inserts or hide tuples
after aborted updates.

Cross-check committed hints against aborted transactions, including
lockers.  Do not confuse a stale in-progress status with an abort.
Add coverage for contradictory committed hints.

Discussion: https://postgr.es/m/B993F6CC-03D0-440F-9D7C-FEF65D19C082@yandex-team.ru
---
 contrib/amcheck/verify_heapam.c           | 27 +++++++++++++++++++
 src/bin/pg_amcheck/t/004_verify_heapam.pl | 33 ++++++++++++++++++++---
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 89a4d80935f..ef0dae151c1 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -1357,6 +1357,17 @@ check_tuple_visibility(HeapCheckContext *ctx, bool *xmin_commit_status_ok,
 			return false;
 		}
 	}
+	else if (xmin_status == XID_ABORTED)
+	{
+		/*
+		 * Only a final aborted status contradicts a committed hint.  An
+		 * in-progress status can be stale by the time we read the hint.
+		 */
+		report_corruption(ctx,
+						  psprintf("xmin %u is aborted, but marked committed",
+								   xmin));
+		return false;			/* don't check the tuple's contents */
+	}
 
 	/*
 	 * Okay, the inserter committed, so it was good at some point.  Now what
@@ -1409,6 +1420,22 @@ check_tuple_visibility(HeapCheckContext *ctx, bool *xmin_commit_status_ok,
 		}
 	}
 
+	/*
+	 * A committed hint must agree with the transaction's outcome, even if
+	 * xmax only locked the tuple.  A multixact cannot have a committed hint;
+	 * check_tuple_header() already reports that case.
+	 */
+	if ((tuphdr->t_infomask & (HEAP_XMAX_COMMITTED | HEAP_XMAX_IS_MULTI)) ==
+		HEAP_XMAX_COMMITTED)
+	{
+		xmax = HeapTupleHeaderGetRawXmax(tuphdr);
+		if (get_xid_status(xmax, ctx, &xmax_status, NULL) == XID_BOUNDS_OK &&
+			xmax_status == XID_ABORTED)
+			report_corruption(ctx,
+							  psprintf("xmax %u is aborted, but marked committed",
+									   xmax));
+	}
+
 	if (tuphdr->t_infomask & HEAP_XMAX_INVALID)
 	{
 		/*
diff --git a/src/bin/pg_amcheck/t/004_verify_heapam.pl b/src/bin/pg_amcheck/t/004_verify_heapam.pl
index 9c282cd9603..3c196138c55 100644
--- a/src/bin/pg_amcheck/t/004_verify_heapam.pl
+++ b/src/bin/pg_amcheck/t/004_verify_heapam.pl
@@ -224,7 +224,7 @@ my $relpath = "$pgdata/$rel";
 # $ROWCOUNT is the total number of rows that we expect to insert into the page.
 # $ROWCOUNT_BASIC is the number of those rows that are related to basic
 # tuple validation, rather than update chain validation.
-my $ROWCOUNT = 57;
+my $ROWCOUNT = 60;
 my $ROWCOUNT_BASIC = 16;
 
 # First insert data needed for tests unrelated to update chain validation.
@@ -316,13 +316,13 @@ my $in_progress_xid = $node->safe_psql(
 		SELECT transaction FROM pg_prepared_xacts;
 	));
 
-# Tuples for checking hint bits, at offset numbers 45 through 57.
+# Tuples for checking hint bits, at offset numbers 45 through 60.
 $node->safe_psql(
 	'postgres', qq(
 		INSERT INTO public.test (a, b, c)
 			SELECT x'DEADF9F9DEADF9F9'::bigint, 'abcdefg',
 				repeat('w', 10000)
-			FROM generate_series(1, 13);
+			FROM generate_series(1, 16);
 	));
 
 # Make real multixacts with a committed updater, an aborted updater, and
@@ -864,6 +864,26 @@ for (my $tupidx = 0; $tupidx < $ROWCOUNT; $tupidx++)
 			}
 		}
 	}
+	elsif ($offnum == 58)
+	{
+		# An aborted inserter must not be hinted committed.
+		$tup->{t_xmin} = $aborted_xid;
+		$tup->{t_infomask} &= ~HEAP_XMIN_INVALID;
+		$tup->{t_infomask} |= HEAP_XMIN_COMMITTED;
+		push @expected,
+		  qr/${header}xmin $aborted_xid is aborted, but marked committed/;
+	}
+	elsif ($offnum == 59 || $offnum == 60)
+	{
+		# An aborted xmax cannot be hinted committed, even for a locker.
+		$tup->{t_xmax} = $aborted_xid;
+		$tup->{t_infomask} &= ~HEAP_XMAX_INVALID;
+		$tup->{t_infomask} |= HEAP_XMAX_COMMITTED;
+		$tup->{t_infomask} |= HEAP_XMAX_LOCK_ONLY | HEAP_XMAX_KEYSHR_LOCK
+		  if $offnum == 60;
+		push @expected,
+		  qr/${header}xmax $aborted_xid is aborted, but marked committed/;
+	}
 	else
 	{
 		# The tests for update chain validation end up creating a bunch of
@@ -902,6 +922,13 @@ is( $node->safe_psql(
 	"50\n54",
 	'only the incorrect xmax hint bits are reported');
 
+is( $node->safe_psql(
+		'postgres',
+		q(SELECT offnum FROM verify_heapam('test', check_toast => false)
+		  WHERE offnum >= 58 ORDER BY offnum)),
+	"58\n59\n60",
+	'only contradictory committed hints are reported');
+
 $node->safe_psql(
 	'postgres', qq(
                         COMMIT PREPARED 'in_progress_tx';
-- 
That's all, folks. May the source be with you.