heap_prepare_freeze_tuple: Simplify coding

alvherre · alvherre · commit 2bf372a4ae26 · 2019-05-02T16:14:08.000-04:00
Commit d2599ec 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
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
@@ -6106,19 +6106,27 @@ 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,
 					(errcode(ERRCODE_DATA_CORRUPTED),
 					 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;
 		}
 	}
 
