0001-Fix-wrong-logic-in-TransactionIdInRecentPast-v3.patch

application/octet-stream

Filename: 0001-Fix-wrong-logic-in-TransactionIdInRecentPast-v3.patch
Type: application/octet-stream
Part: 0
Message: Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction

Patch

Format: format-patch
Series: patch v3-0001
Subject: Fix wrong logic in TransactionIdInRecentPast()
File+
src/backend/utils/adt/xid8funcs.c 19 11
From d682ecc4f67b43c30028827f2143ca1aa47132b1 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Mon, 5 Feb 2024 15:40:49 +0200
Subject: [PATCH] Fix wrong logic in TransactionIdInRecentPast()

The TransactionIdInRecentPast() should return false for all the transactions
older than TransamVariables->oldestClogXid.  However, the function contains
a bug in comparison FullTransactionId to TransactionID allowing full
transactions between nextXid - 2^32 and oldestClogXid - 2^31.

This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into
FullTransactionId first, then performing the comparison.

Backpatch to all supported versions.

Reported-by: Egor Chindyaskin
Bug: 18212
Discussion: https://postgr.es/m/18212-547307f8adf57262%40postgresql.org
Author: Karina Litskevich
Reviewed-by: Kyotaro Horiguchi
Backpatch-through: 12
---
 src/backend/utils/adt/xid8funcs.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index be5e28c93ab..827392b2a04 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -98,11 +98,12 @@ StaticAssertDecl(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP,
 static bool
 TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
 {
-	uint32		xid_epoch = EpochFromFullTransactionId(fxid);
 	TransactionId xid = XidFromFullTransactionId(fxid);
 	uint32		now_epoch;
 	TransactionId now_epoch_next_xid;
 	FullTransactionId now_fullxid;
+	TransactionId oldest_xid;
+	FullTransactionId oldest_fxid;
 
 	now_fullxid = ReadNextFullTransactionId();
 	now_epoch_next_xid = XidFromFullTransactionId(now_fullxid);
@@ -135,17 +136,24 @@ TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
 	Assert(LWLockHeldByMe(XactTruncationLock));
 
 	/*
-	 * If the transaction ID has wrapped around, it's definitely too old to
-	 * determine the commit status.  Otherwise, we can compare it to
-	 * TransamVariables->oldestClogXid to determine whether the relevant CLOG
-	 * entry is guaranteed to still exist.
+	 * If fxid is not older than TransamVariables->oldestClogXid, the relevant
+	 * CLOG entry is guaranteed to still exist.  To compare them correctly
+	 * make FullTransactionId from TransamVariables->oldestClogXid.  Determine
+	 * the right epoch knowing that oldest_fxid shouldn't be more than 2^31
+	 * older than now_fullxid.
 	 */
-	if (xid_epoch + 1 < now_epoch
-		|| (xid_epoch + 1 == now_epoch && xid < now_epoch_next_xid)
-		|| TransactionIdPrecedes(xid, TransamVariables->oldestClogXid))
-		return false;
-
-	return true;
+	oldest_xid = TransamVariables->oldestClogXid;
+	Assert(TransactionIdPrecedes(oldest_xid, now_epoch_next_xid));
+	if (oldest_xid < now_epoch_next_xid)
+	{
+		oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch, oldest_xid);
+	}
+	else
+	{
+		Assert(now_epoch > 0);
+		oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch - 1, oldest_xid);
+	}
+	return !FullTransactionIdPrecedes(fxid, oldest_fxid);
 }
 
 /*
-- 
2.39.3 (Apple Git-145)