From 9ac665638c86460f0b767628203f8bf28df35e49 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 6 Dec 2017 16:44:25 -0300
Subject: [PATCH 1/2] tweaks for 0002

---
 src/backend/access/heap/heapam.c  | 67 +++++++++++++++++++++++++++------------
 src/backend/commands/vacuumlazy.c |  6 ++--
 2 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index eb93718baa..5c284a4c32 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6387,17 +6387,23 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 	else if (MultiXactIdPrecedes(multi, cutoff_multi))
 	{
 		if (MultiXactIdPrecedes(multi, relminmxid))
-			elog(ERROR, "encountered multixact from before horizon");
+			ereport(ERROR,
+					(errcode(ERRCODE_DATA_CORRUPTED),
+					 errmsg_internal("found multixact %u from before relminmxid %u",
+									 multi, relminmxid)));
 
 		/*
-		 * This old multi cannot possibly have members still running.  If it
-		 * was a locker only, it can be removed without any further
-		 * consideration; but if it contained an update, we might need to
-		 * preserve it.
+		 * This old multi cannot possibly have members still running, but
+		 * verify just in case.  If it was a locker only, it can be removed
+		 * without any further consideration; but if it contained an update, we
+		 * might need to preserve it.
 		 */
 		if (MultiXactIdIsRunning(multi,
 								 HEAP_XMAX_IS_LOCKED_ONLY(t_infomask)))
-			elog(ERROR, "multixact from before cutoff found to be still running");
+			ereport(ERROR,
+					(errcode(ERRCODE_DATA_CORRUPTED),
+					 errmsg_internal("multixact %u from before cutoff %u found to be still running",
+									 multi, cutoff_multi)));
 
 		if (HEAP_XMAX_IS_LOCKED_ONLY(t_infomask))
 		{
@@ -6419,9 +6425,14 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 			if (TransactionIdPrecedes(xid, cutoff_xid))
 			{
 				if (TransactionIdPrecedes(xid, relfrozenxid))
-					elog(ERROR, "encountered xid from before horizon");
+					ereport(ERROR,
+							(errcode(ERRCODE_DATA_CORRUPTED),
+							 errmsg_internal("found xid %u from before relfrozenxid %u",
+											 xid, relfrozenxid)));
 				if (TransactionIdDidCommit(xid))
-					elog(ERROR, "can't freeze committed xmax");
+					ereport(ERROR,
+							(errcode(ERRCODE_DATA_CORRUPTED),
+							 errmsg_internal("can't freeze committed xmax %u", xid)));
 				*flags |= FRM_INVALIDATE_XMAX;
 				xid = InvalidTransactionId; /* not strictly necessary */
 			}
@@ -6495,7 +6506,10 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 
 			Assert(TransactionIdIsValid(xid));
 			if (TransactionIdPrecedes(xid, relfrozenxid))
-				elog(ERROR, "encountered xid from before horizon");
+				ereport(ERROR,
+						(errcode(ERRCODE_DATA_CORRUPTED),
+						 errmsg_internal("found update xid %u from before relfrozenxid %u",
+										 xid, relfrozenxid)));
 
 			/*
 			 * It's an update; should we keep it?  If the transaction is known
@@ -6533,7 +6547,6 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 				 */
 			}
 
-
 			/*
 			 * Since the tuple wasn't marked HEAPTUPLE_DEAD by vacuum, the
 			 * update Xid cannot possibly be older than the xid cutoff. The
@@ -6542,7 +6555,10 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 			 */
 			if (TransactionIdIsValid(update_xid) &&
 				TransactionIdPrecedes(update_xid, cutoff_xid))
-				elog(ERROR, "encountered xid from before xid cutoff");
+				ereport(ERROR,
+						(errcode(ERRCODE_DATA_CORRUPTED),
+						 errmsg_internal("found update xid %u from before xid cutoff %u",
+										 update_xid, cutoff_xid)));
 
 			/*
 			 * If we determined that it's an Xid corresponding to an update
@@ -6658,13 +6674,19 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 	xid = HeapTupleHeaderGetXmin(tuple);
 	if (TransactionIdIsNormal(xid))
 	{
+		if (TransactionIdPrecedes(xid, relfrozenxid))
+			ereport(ERROR,
+					(errcode(ERRCODE_DATA_CORRUPTED),
+					 errmsg_internal("found xid %u from before relfrozenxid %u",
+									 xid, relfrozenxid)));
+
 		if (TransactionIdPrecedes(xid, cutoff_xid))
 		{
-			if (TransactionIdPrecedes(xid, relfrozenxid))
-				elog(ERROR, "encountered xid from before xid cutoff");
-
 			if (!TransactionIdDidCommit(xid))
-				elog(ERROR, "uncommitted xmin needs to be frozen");
+				ereport(ERROR,
+						(errcode(ERRCODE_DATA_CORRUPTED),
+						 errmsg_internal("uncommitted Xmin %u from before xid cutoff %u needs to be frozen",
+										 xid, cutoff_xid)));
 
 			frz->t_infomask |= HEAP_XMIN_FROZEN;
 			changed = true;
@@ -6740,20 +6762,25 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 	}
 	else if (TransactionIdIsNormal(xid))
 	{
+		if (TransactionIdPrecedes(xid, relfrozenxid))
+			ereport(ERROR,
+					(errcode(ERRCODE_DATA_CORRUPTED),
+					 errmsg("found xid %u from before relfrozenxid %u",
+							xid, relfrozenxid)));
+
 		if (TransactionIdPrecedes(xid, cutoff_xid))
 		{
-			if (TransactionIdPrecedes(xid, relfrozenxid))
-				elog(ERROR, "encountered xid from before xid cutoff");
-
 			/*
 			 * If we freeze xmax, make absolutely sure that it's not an XID
-			 * that is important (Note, a lock-only xmax can be removed
+			 * that is important.  (Note, a lock-only xmax can be removed
 			 * independent of committedness, since a committed lock holder has
 			 * released the lock).
 			 */
 			if (!(tuple->t_infomask & HEAP_XMAX_LOCK_ONLY) &&
 				TransactionIdDidCommit(xid))
-				elog(ERROR, "can't freeze committed xmax");
+				ereport(ERROR,
+						(errcode(ERRCODE_DATA_CORRUPTED),
+						 errmsg("can't freeze committed xmax %u", xid)));
 			freeze_xmax = true;
 		}
 		else
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 7035a7a6f7..f95346acdb 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -1008,10 +1008,10 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats,
 					 * to treat it like an indexed tuple.
 					 *
 					 * If this were to happen for a tuple that actually needed
-					 * to be deleted , we'd be in trouble, because it'd
+					 * to be deleted, we'd be in trouble, because it'd
 					 * possibly leave a tuple below the relation's xmin
-					 * horizon alive. But checks in
-					 * heap_prepare_freeze_tuple() would trigger in that case,
+					 * horizon alive.  heap_prepare_freeze_tuple() is prepared
+					 * to detect that case and abort the transaction,
 					 * preventing corruption.
 					 */
 					if (HeapTupleIsHotUpdated(&tuple) ||
-- 
2.11.0

