From 2bf372a4ae26337ee61cf7a82081cf4bed05182d Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 2 May 2019 16:13:48 -0400 Subject: [PATCH] heap_prepare_freeze_tuple: Simplify coding Commit d2599ecfcc74 introduced some contorted, confused code around: readers would think that it's possible for HeapTupleHeaderGetXmin return a non-frozen value for some frozen tuples, which would be disastrous. There's no actual bug, but it seems better to make it clearer. Per gripe from Tom Lane and Andres Freund. Discussion: https://postgr.es/m/30116.1555430496@sss.pgh.pa.us --- src/backend/access/heap/heapam.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index a05b6a07ad..6f26ddac5f 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -6106,11 +6106,18 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, frz->t_infomask = tuple->t_infomask; frz->xmax = HeapTupleHeaderGetRawXmax(tuple); - /* Process xmin */ + /* + * Process xmin. xmin_frozen has two slightly different meanings: in the + * !XidIsNormal case, it means "the xmin doesn't need any freezing" (it's + * already a permanent value), while in the block below it is set true to + * mean "xmin won't need freezing after what we do to it here" (false + * otherwise). In both cases we're allowed to set totally_frozen, as far + * as xmin is concerned. + */ xid = HeapTupleHeaderGetXmin(tuple); - xmin_frozen = ((xid == FrozenTransactionId) || - HeapTupleHeaderXminFrozen(tuple)); - if (TransactionIdIsNormal(xid)) + if (!TransactionIdIsNormal(xid)) + xmin_frozen = true; + else { if (TransactionIdPrecedes(xid, relfrozenxid)) ereport(ERROR, @@ -6118,7 +6125,8 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, errmsg_internal("found xmin %u from before relfrozenxid %u", xid, relfrozenxid))); - if (TransactionIdPrecedes(xid, cutoff_xid)) + xmin_frozen = TransactionIdPrecedes(xid, cutoff_xid); + if (xmin_frozen) { if (!TransactionIdDidCommit(xid)) ereport(ERROR, @@ -6128,7 +6136,6 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, frz->t_infomask |= HEAP_XMIN_FROZEN; changed = true; - xmin_frozen = true; } }