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

diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index d8e40b3b969e4b940589c23a1149929fc482c69c..3af9ebadce6758b998a291f5083fc33941fd487d 100644 (file)
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -90,11 +90,12 @@ typedef struct
 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);
@@ -127,17 +128,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
-    * ShmemVariableCache->oldestClogXid to determine whether the relevant
-    * CLOG entry is guaranteed to still exist.
+    * If fxid is not older than ShmemVariableCache->oldestClogXid, the
+    * relevant CLOG entry is guaranteed to still exist.  Convert
+    * ShmemVariableCache->oldestClogXid into a FullTransactionId to compare
+    * it with fxid.  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, ShmemVariableCache->oldestClogXid))
-       return false;
-
-   return true;
+   oldest_xid = ShmemVariableCache->oldestClogXid;
+   Assert(TransactionIdPrecedesOrEquals(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);
 }
 
 /*