From 02d647bbf0576ebb87f9dc24e1db4dd034f04048 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Wed, 23 Nov 2022 10:49:39 -0800 Subject: [PATCH] Don't test HEAP_XMAX_INVALID when freezing xmax. We shouldn't ever need to rely on whether HEAP_XMAX_INVALID is set in t_infomask when considering whether or not an xmax should be deemed already frozen, since that status flag is just a hint. The only acceptable representation for an "xmax_already_frozen" raw xmax field is the transaction ID value zero (also known as InvalidTransactionId). Adjust code that superficially appeared to rely on HEAP_XMAX_INVALID to make the rule about xmax_already_frozen clear. Also avoid needlessly rereading the tuple's raw xmax. Oversight in bugfix commit d2599ecf. There is no evidence that this ever led to incorrect behavior, so no backpatch. The worst consequence of this bug was that VACUUM could hypothetically fail to notice and report on certain kinds of corruption, which seems fairly benign. Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-Wzkh3DMCDRPfhZxj9xCq9v3WmzvmbiCpf1dNKUBPadhCbQ@mail.gmail.com --- src/backend/access/heap/heapam.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index d18c5ca6f5..747db50376 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -6542,6 +6542,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, if (tuple->t_infomask & HEAP_XMAX_IS_MULTI) { + /* Raw xmax is a MultiXactId */ TransactionId newxmax; uint16 flags; TransactionId mxid_oldest_xid_out = *relfrozenxid_out; @@ -6640,6 +6641,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, } else if (TransactionIdIsNormal(xid)) { + /* Raw xmax is normal XID */ if (TransactionIdPrecedes(xid, relfrozenxid)) ereport(ERROR, (errcode(ERRCODE_DATA_CORRUPTED), @@ -6670,9 +6672,10 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, *relfrozenxid_out = xid; } } - else if ((tuple->t_infomask & HEAP_XMAX_INVALID) || - !TransactionIdIsValid(HeapTupleHeaderGetRawXmax(tuple))) + else if (!TransactionIdIsValid(xid)) { + /* Raw xmax is InvalidTransactionId XID */ + Assert((tuple->t_infomask & HEAP_XMAX_IS_MULTI) == 0); freeze_xmax = false; xmax_already_frozen = true; /* No need for relfrozenxid_out handling for already-frozen xmax */