BUG: Postgres 14 + vacuum_defer_cleanup_age + FOR UPDATE + UPDATE

From ea37a15e424385d76a5976ef14fc24174e738bee Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Mon, 9 Jan 2023 17:34:01 +0100
Subject: [PATCH v1] Prevent underflow of xid8 epoch

In some cases where we use xid offsets, it is possible to create a
xid4 that is logically "before" the first transaction ID. With
xid4, that is not much of an issue because we allow them to over-
and underflow as long as the values stay within 2^31 of any active
transaction, but this is not the case for xid8: xid8 assumes a
strictly increasing counter, which disallows under- and overflows.

To still correctly convert xid to xid8, an underflow of the epoch is
handled by (depending on context) truncating the value to either
FirstNormalFullTransactionId or InvalidFullTransactionId.
---
 src/backend/replication/walreceiver.c | 21 +++++++++++++++++++--
 src/backend/storage/ipc/procarray.c   | 11 +++++++++++
 src/backend/utils/adt/xid8funcs.c     | 12 ++++++++++++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 3876c0188d..5710a42ad6 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -1233,10 +1233,27 @@ XLogWalRcvSendHSFeedback(bool immed)
 	nextXid = XidFromFullTransactionId(nextFullXid);
 	xmin_epoch = EpochFromFullTransactionId(nextFullXid);
 	catalog_xmin_epoch = xmin_epoch;
+
+	/*
+	 * If we aren't careful, we can allow the epoch to underflow, resulting
+	 * in all kinds of bad behaviour due to breaking the xid8 sequentiality
+	 * guarantee. So, instead of letting the epoch wrap around, we set the
+	 * xmin to the lowest possible value.
+	 */
 	if (nextXid < xmin)
-		xmin_epoch--;
+	{
+		if (xmin_epoch == 0)
+			xmin = InvalidTransactionId;
+		else
+			--xmin_epoch;
+	}
 	if (nextXid < catalog_xmin)
-		catalog_xmin_epoch--;
+	{
+		if (catalog_xmin_epoch == 0)
+			catalog_xmin = InvalidTransactionId;
+		else
+			catalog_xmin_epoch--;
+	}
 
 	elog(DEBUG2, "sending hot standby feedback xmin %u epoch %u catalog_xmin %u catalog_xmin_epoch %u",
 		 xmin, xmin_epoch, catalog_xmin, catalog_xmin_epoch);
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 4340bf9641..f83ccacf03 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -4305,6 +4305,9 @@ GlobalVisCheckRemovableXid(Relation rel, TransactionId xid)
  * held by the backend and xid is from a table (where vacuum/freezing ensures
  * the xid has to be within that range), or if xid is from the procarray and
  * prevents xid wraparound that way.
+ *
+ * The function returns FirstNormalTransactionId if provided with a xid
+ * that would make the epoch underflow and wrap around.
  */
 static inline FullTransactionId
 FullXidRelativeTo(FullTransactionId rel, TransactionId xid)
@@ -4317,6 +4320,14 @@ FullXidRelativeTo(FullTransactionId rel, TransactionId xid)
 	/* not guaranteed to find issues, but likely to catch mistakes */
 	AssertTransactionIdInAllowableRange(xid);
 
+	/*
+	 * An epoch of 0 would underflow on this condition and break the
+	 * 'xid8 does not wrap around' guarantee.
+	 */
+	if (unlikely(EpochFromFullTransactionId(rel) == 0 &&
+				 ((int32) (xid - rel_xid)) < 0))
+		return FirstNormalFullTransactionId;
+
 	return FullTransactionIdFromU64(U64FromFullTransactionId(rel)
 									+ (int32) (xid - rel_xid));
 }
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index e616303a29..55fb838330 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -153,6 +153,9 @@ TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
  * FullTransactionId.  Use next_fxid as a reference FullTransactionId, so that
  * we can compute the high order bits.  It must have been obtained by the
  * caller with ReadNextFullTransactionId() after the snapshot was created.
+ *
+ * The function returns FirstNormalTransactionId if provided with a xid
+ * that would make the epoch underflow and wrap around.
  */
 static FullTransactionId
 widen_snapshot_xid(TransactionId xid, FullTransactionId next_fxid)
@@ -172,7 +175,16 @@ widen_snapshot_xid(TransactionId xid, FullTransactionId next_fxid)
 	 * more than one epoch ahead of the TransactionIds in any snapshot.
 	 */
 	if (xid > next_xid)
+	{
+		/*
+		 * We may not allow the epoch to underflow, so instead we'll output
+		 * FirstNormalFullTransactionId as a substitute.
+		 */
+		if (epoch == 0)
+			return FirstNormalFullTransactionId;
+
 		epoch--;
+	}
 
 	return FullTransactionIdFromEpochAndXid(epoch, xid);
 }
-- 
2.30.2

From 13450d14a1e064dd958b516ffa36b7a24bb49d7b Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 9 Jan 2023 10:23:10 -0800
Subject: [PATCH v2 1/3] Fix corruption due to vacuum_defer_cleanup_age
 underflowing 64bit xids

Author:
Reviewed-by:
Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
Backpatch:
---
 src/backend/storage/ipc/procarray.c | 73 ++++++++++++++++++++++++-----
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 4340bf96416..d7d18ba8c12 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -367,6 +367,7 @@ static inline void ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId l
 static void ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid);
 static void MaintainLatestCompletedXid(TransactionId latestXid);
 static void MaintainLatestCompletedXidRecovery(TransactionId latestXid);
+static int ClampVacuumDeferCleanupAge(FullTransactionId rel, TransactionId xid);
 
 static inline FullTransactionId FullXidRelativeTo(FullTransactionId rel,
 												  TransactionId xid);
@@ -1888,17 +1889,32 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
 		 * so guc.c should limit it to no more than the xidStopLimit threshold
 		 * in varsup.c.  Also note that we intentionally don't apply
 		 * vacuum_defer_cleanup_age on standby servers.
+		 *
+		 * Be careful to clamp vacuum_defer_cleanup age to prevent it from
+		 * creating an xid before FirstNormalTransactionId.
 		 */
-		h->oldest_considered_running =
-			TransactionIdRetreatedBy(h->oldest_considered_running,
-									 vacuum_defer_cleanup_age);
-		h->shared_oldest_nonremovable =
-			TransactionIdRetreatedBy(h->shared_oldest_nonremovable,
-									 vacuum_defer_cleanup_age);
-		h->data_oldest_nonremovable =
-			TransactionIdRetreatedBy(h->data_oldest_nonremovable,
-									 vacuum_defer_cleanup_age);
-		/* defer doesn't apply to temp relations */
+		Assert(TransactionIdPrecedesOrEquals(h->oldest_considered_running,
+											 h->shared_oldest_nonremovable));
+		Assert(TransactionIdPrecedesOrEquals(h->shared_oldest_nonremovable,
+											 h->data_oldest_nonremovable));
+
+		if (vacuum_defer_cleanup_age > 0)
+		{
+			int clamped_cleanup_age =
+				ClampVacuumDeferCleanupAge(h->latest_completed,
+										   h->oldest_considered_running);
+
+			h->oldest_considered_running =
+				TransactionIdRetreatedBy(h->oldest_considered_running,
+										 clamped_cleanup_age);
+			h->shared_oldest_nonremovable =
+				TransactionIdRetreatedBy(h->shared_oldest_nonremovable,
+										 clamped_cleanup_age);
+			h->data_oldest_nonremovable =
+				TransactionIdRetreatedBy(h->data_oldest_nonremovable,
+										 clamped_cleanup_age);
+			/* defer doesn't apply to temp relations */
+		}
 	}
 
 	/*
@@ -2469,9 +2485,17 @@ GetSnapshotData(Snapshot snapshot)
 		 */
 		oldestfxid = FullXidRelativeTo(latest_completed, oldestxid);
 
+		def_vis_xid_data = xmin;
+
 		/* apply vacuum_defer_cleanup_age */
-		def_vis_xid_data =
-			TransactionIdRetreatedBy(xmin, vacuum_defer_cleanup_age);
+		if (vacuum_defer_cleanup_age > 0)
+		{
+			int clamped_cleanup_age =
+				ClampVacuumDeferCleanupAge(oldestfxid, oldestxid);
+
+			def_vis_xid_data =
+				TransactionIdRetreatedBy(xmin, clamped_cleanup_age);
+		}
 
 		/* Check whether there's a replication slot requiring an older xmin. */
 		def_vis_xid_data =
@@ -4295,6 +4319,31 @@ GlobalVisCheckRemovableXid(Relation rel, TransactionId xid)
 	return GlobalVisTestIsRemovableXid(state, xid);
 }
 
+/*
+ * Clamp vacuum_defer_cleanup_age, to prevent it from retreating below
+ * FirstNormalTransactionId during epoch 0. This is important to prevent
+ * generating xids that cannot be converted to a FullTransactionId without
+ * wrapping around.
+ */
+static int
+ClampVacuumDeferCleanupAge(FullTransactionId rel, TransactionId oldest_xid)
+{
+	FullTransactionId oldest_fxid;
+	uint64 oldest_fxid_i;
+
+	if (vacuum_defer_cleanup_age == 0)
+		return 0;
+
+	oldest_fxid = FullXidRelativeTo(rel, oldest_xid);
+	oldest_fxid_i = U64FromFullTransactionId(oldest_fxid);
+
+	if (oldest_fxid_i < vacuum_defer_cleanup_age ||
+		(oldest_fxid_i - vacuum_defer_cleanup_age) < FirstNormalTransactionId)
+		return (int) (oldest_fxid_i - FirstNormalTransactionId);
+
+	return vacuum_defer_cleanup_age;
+}
+
 /*
  * Convert a 32 bit transaction id into 64 bit transaction id, by assuming it
  * is within MaxTransactionId / 2 of XidFromFullTransactionId(rel).
-- 
2.38.0

From 1b85be918858172cec0c2884169e79576608ed3f Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 9 Jan 2023 11:24:37 -0800
Subject: [PATCH v2 2/3] wip: amcheck: Fix bugs relating to 64bit xids

Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
---
 contrib/amcheck/verify_heapam.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 4fcfd6df72d..56a3b3e9974 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -1576,11 +1576,25 @@ FullTransactionIdFromXidAndCtx(TransactionId xid, const HeapCheckContext *ctx)
 {
 	uint32		epoch;
 
+	Assert(TransactionIdIsValid(ctx->next_xid));
+	Assert(FullTransactionIdIsValid(ctx->next_fxid));
+
 	if (!TransactionIdIsNormal(xid))
 		return FullTransactionIdFromEpochAndXid(0, xid);
 	epoch = EpochFromFullTransactionId(ctx->next_fxid);
 	if (xid > ctx->next_xid)
+	{
+		/*
+		 * FIXME, doubtful this is the best fix.
+		 *
+		 * Can't represent the 32bit xid as a 64bit xid, as it's before fxid
+		 * 0. Represent it as an xid from the future instead.
+		 */
+		if (epoch == 0)
+			return FullTransactionIdFromEpochAndXid(0, xid);
+
 		epoch--;
+	}
 	return FullTransactionIdFromEpochAndXid(epoch, xid);
 }
 
@@ -1597,8 +1611,8 @@ update_cached_xid_range(HeapCheckContext *ctx)
 	LWLockRelease(XidGenLock);
 
 	/* And compute alternate versions of the same */
-	ctx->oldest_fxid = FullTransactionIdFromXidAndCtx(ctx->oldest_xid, ctx);
 	ctx->next_xid = XidFromFullTransactionId(ctx->next_fxid);
+	ctx->oldest_fxid = FullTransactionIdFromXidAndCtx(ctx->oldest_xid, ctx);
 }
 
 /*
-- 
2.38.0

From 2fccf780a517416283ad739acb6df4cbcf2f2b28 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 9 Jan 2023 11:25:43 -0800
Subject: [PATCH v2 3/3] wip: stricter validation for 64bit xids

64bit xids can't represent xids before xid 0, but 32bit xids can. This has
lead to a number of bugs, some data corrupting. To make it easier to catch
such bugs, disallow 64bit where the upper 32bit are all set, which is what one
gets when naively trying to convert such 32bit xids. Additionally explicitly
disallow 64bit xids that are already aren't allowed because they'd map to
special 32bit xids.

This changes the input routines for 64bit xids to error out in more cases.

Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
---
 src/include/access/transam.h      | 136 ++++++++++++++++++++++++++++--
 src/backend/utils/adt/xid.c       |   7 ++
 src/backend/utils/adt/xid8funcs.c |  17 ++--
 src/test/regress/expected/xid.out |  44 ++++++++--
 src/test/regress/sql/xid.sql      |  12 ++-
 5 files changed, 192 insertions(+), 24 deletions(-)

diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index f5af6d30556..0d4bf57e63a 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -44,15 +44,6 @@
 #define TransactionIdStore(xid, dest)	(*(dest) = (xid))
 #define StoreInvalidTransactionId(dest) (*(dest) = InvalidTransactionId)
 
-#define EpochFromFullTransactionId(x)	((uint32) ((x).value >> 32))
-#define XidFromFullTransactionId(x)		((uint32) (x).value)
-#define U64FromFullTransactionId(x)		((x).value)
-#define FullTransactionIdEquals(a, b)	((a).value == (b).value)
-#define FullTransactionIdPrecedes(a, b)	((a).value < (b).value)
-#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <= (b).value)
-#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
-#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >= (b).value)
-#define FullTransactionIdIsValid(x)		TransactionIdIsValid(XidFromFullTransactionId(x))
 #define InvalidFullTransactionId		FullTransactionIdFromEpochAndXid(0, InvalidTransactionId)
 #define FirstNormalFullTransactionId	FullTransactionIdFromEpochAndXid(0, FirstNormalTransactionId)
 #define FullTransactionIdIsNormal(x)	FullTransactionIdFollowsOrEquals(x, FirstNormalFullTransactionId)
@@ -61,12 +52,127 @@
  * A 64 bit value that contains an epoch and a TransactionId.  This is
  * wrapped in a struct to prevent implicit conversion to/from TransactionId.
  * Not all values represent valid normal XIDs.
+ *
+ * 64bit xids that would map to a non-normal 32bit xid are not allowed, for
+ * obvious reasons.
+
+ * In addition, no xids with all the upper 32bit sets are allowed to exist,
+ * this is to make it easier to catch conversion errors from 32bit xids (which
+ * can point to "before" xid 0).
  */
 typedef struct FullTransactionId
 {
 	uint64		value;
 } FullTransactionId;
 
+#define MAX_FULL_TRANSACTION_ID ((((uint64)~(uint32)0)<<32) - 1)
+
+/*
+ * This is separate from FullTransactionIdValidRange() so that input routines
+ * can check for invalid values without triggering an assert.
+ */
+static inline bool
+InFullTransactionIdRange(uint64 val)
+{
+	/* 64bit xid above the upper bound */
+	if (val > MAX_FULL_TRANSACTION_ID)
+		return false;
+
+	/*
+	 * normal 64bit xids that'd map to special 32 xids aren't allowed.
+	 */
+	if (val >= (uint64) FirstNormalTransactionId &&
+		(uint32) val < FirstNormalTransactionId)
+		return false;
+	return true;
+}
+
+/*
+ * Validation routine, typically used in asserts.
+ */
+static inline bool
+FullTransactionIdValidRange(FullTransactionId x)
+{
+	return InFullTransactionIdRange(x.value);
+}
+
+static inline uint64
+U64FromFullTransactionId(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return x.value;
+}
+
+static inline TransactionId
+XidFromFullTransactionId(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return (TransactionId) x.value;
+}
+
+static inline uint32
+EpochFromFullTransactionId(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return (uint32) (x.value >> 32);
+}
+
+static inline bool
+FullTransactionIdEquals(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value == b.value;
+}
+
+static inline bool
+FullTransactionIdPrecedes(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value < b.value;
+}
+
+static inline bool
+FullTransactionIdPrecedesOrEquals(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value <= b.value;
+}
+
+static inline bool
+FullTransactionIdFollows(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value > b.value;
+}
+
+static inline bool
+FullTransactionIdFollowsOrEquals(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value >= b.value;
+}
+
+static inline bool
+FullTransactionIdIsValid(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return x.value != (uint64) InvalidTransactionId;
+}
+
 static inline FullTransactionId
 FullTransactionIdFromEpochAndXid(uint32 epoch, TransactionId xid)
 {
@@ -74,6 +180,8 @@ FullTransactionIdFromEpochAndXid(uint32 epoch, TransactionId xid)
 
 	result.value = ((uint64) epoch) << 32 | xid;
 
+	Assert(FullTransactionIdValidRange(result));
+
 	return result;
 }
 
@@ -84,6 +192,8 @@ FullTransactionIdFromU64(uint64 value)
 
 	result.value = value;
 
+	Assert(FullTransactionIdValidRange(result));
+
 	return result;
 }
 
@@ -102,6 +212,8 @@ FullTransactionIdFromU64(uint64 value)
 static inline void
 FullTransactionIdRetreat(FullTransactionId *dest)
 {
+	Assert(FullTransactionIdValidRange(*dest));
+
 	dest->value--;
 
 	/*
@@ -118,6 +230,8 @@ FullTransactionIdRetreat(FullTransactionId *dest)
 	 */
 	while (XidFromFullTransactionId(*dest) < FirstNormalTransactionId)
 		dest->value--;
+
+	Assert(FullTransactionIdValidRange(*dest));
 }
 
 /*
@@ -127,6 +241,8 @@ FullTransactionIdRetreat(FullTransactionId *dest)
 static inline void
 FullTransactionIdAdvance(FullTransactionId *dest)
 {
+	Assert(FullTransactionIdValidRange(*dest));
+
 	dest->value++;
 
 	/* see FullTransactionIdAdvance() */
@@ -135,6 +251,8 @@ FullTransactionIdAdvance(FullTransactionId *dest)
 
 	while (XidFromFullTransactionId(*dest) < FirstNormalTransactionId)
 		dest->value++;
+
+	Assert(FullTransactionIdValidRange(*dest));
 }
 
 /* back up a transaction ID variable, handling wraparound correctly */
diff --git a/src/backend/utils/adt/xid.c b/src/backend/utils/adt/xid.c
index 8ac1679c381..21c0f4c67df 100644
--- a/src/backend/utils/adt/xid.c
+++ b/src/backend/utils/adt/xid.c
@@ -188,6 +188,13 @@ xid8in(PG_FUNCTION_ARGS)
 	uint64		result;
 
 	result = uint64in_subr(str, NULL, "xid8", fcinfo->context);
+
+	if (!InFullTransactionIdRange(result))
+		ereturn(fcinfo->context, 0,
+				(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+				 errmsg("value \"%s\" is out of range for type %s",
+						str, "xid8")));
+
 	PG_RETURN_FULLTRANSACTIONID(FullTransactionIdFromU64(result));
 }
 
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index e616303a292..b8d8d92f793 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -302,15 +302,18 @@ parse_snapshot(const char *str, Node *escontext)
 	const char *str_start = str;
 	char	   *endp;
 	StringInfo	buf;
+	uint64 raw_fxid;
 
-	xmin = FullTransactionIdFromU64(strtou64(str, &endp, 10));
-	if (*endp != ':')
+	raw_fxid = strtou64(str, &endp, 10);
+	if (*endp != ':' || !InFullTransactionIdRange(raw_fxid))
 		goto bad_format;
+	xmin = FullTransactionIdFromU64(raw_fxid);
 	str = endp + 1;
 
-	xmax = FullTransactionIdFromU64(strtou64(str, &endp, 10));
-	if (*endp != ':')
+	raw_fxid = strtou64(str, &endp, 10);
+	if (*endp != ':' || !InFullTransactionIdRange(raw_fxid))
 		goto bad_format;
+	xmax = FullTransactionIdFromU64(raw_fxid);
 	str = endp + 1;
 
 	/* it should look sane */
@@ -326,7 +329,11 @@ parse_snapshot(const char *str, Node *escontext)
 	while (*str != '\0')
 	{
 		/* read next value */
-		val = FullTransactionIdFromU64(strtou64(str, &endp, 10));
+		raw_fxid = strtou64(str, &endp, 10);
+
+		val = FullTransactionIdFromU64(raw_fxid);
+		if (!InFullTransactionIdRange(raw_fxid))
+			goto bad_format;
 		str = endp;
 
 		/* require the input to be in order */
diff --git a/src/test/regress/expected/xid.out b/src/test/regress/expected/xid.out
index e62f7019434..4348eb66bd5 100644
--- a/src/test/regress/expected/xid.out
+++ b/src/test/regress/expected/xid.out
@@ -6,11 +6,11 @@ select '010'::xid,
        '-1'::xid,
 	   '010'::xid8,
 	   '42'::xid8,
-	   '0xffffffffffffffff'::xid8,
-	   '-1'::xid8;
- xid | xid |    xid     |    xid     | xid8 | xid8 |         xid8         |         xid8         
------+-----+------------+------------+------+------+----------------------+----------------------
-   8 |  42 | 4294967295 | 4294967295 |    8 |   42 | 18446744073709551615 | 18446744073709551615
+	   '0xefffffffffffffff'::xid8,
+	   '0'::xid8;
+ xid | xid |    xid     |    xid     | xid8 | xid8 |         xid8         | xid8 
+-----+-----+------------+------------+------+------+----------------------+------
+   8 |  42 | 4294967295 | 4294967295 |    8 |   42 | 17293822569102704639 |    0
 (1 row)
 
 -- garbage values
@@ -30,6 +30,18 @@ select 'asdf'::xid8;
 ERROR:  invalid input syntax for type xid8: "asdf"
 LINE 1: select 'asdf'::xid8;
                ^
+select '-1'::xid8;
+ERROR:  value "-1" is out of range for type xid8
+LINE 1: select '-1'::xid8;
+               ^
+select '0xffffffffffffffff'::xid8;
+ERROR:  value "0xffffffffffffffff" is out of range for type xid8
+LINE 1: select '0xffffffffffffffff'::xid8;
+               ^
+select '0x0000300000000001'::xid8;
+ERROR:  value "0x0000300000000001" is out of range for type xid8
+LINE 1: select '0x0000300000000001'::xid8;
+               ^
 -- Also try it with non-error-throwing API
 SELECT pg_input_is_valid('42', 'xid');
  pg_input_is_valid 
@@ -67,6 +79,24 @@ SELECT pg_input_error_message('0xffffffffffffffffffff', 'xid8');
  value "0xffffffffffffffffffff" is out of range for type xid8
 (1 row)
 
+SELECT pg_input_is_valid('-1', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
+SELECT pg_input_is_valid('0xffffffffffffffff', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
+SELECT pg_input_is_valid('0x0000300000000001', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
 -- equality
 select '1'::xid = '1'::xid;
  ?column? 
@@ -160,11 +190,11 @@ select xid8cmp('1', '2'), xid8cmp('2', '2'), xid8cmp('2', '1');
 
 -- min() and max() for xid8
 create table xid8_t1 (x xid8);
-insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xffffffffffffffff'), ('-1');
+insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xefffffffffffffff');
 select min(x), max(x) from xid8_t1;
  min |         max          
 -----+----------------------
-   0 | 18446744073709551615
+   0 | 17293822569102704639
 (1 row)
 
 -- xid8 has btree and hash opclasses
diff --git a/src/test/regress/sql/xid.sql b/src/test/regress/sql/xid.sql
index b6996588ef6..b2104e4ce20 100644
--- a/src/test/regress/sql/xid.sql
+++ b/src/test/regress/sql/xid.sql
@@ -7,14 +7,17 @@ select '010'::xid,
        '-1'::xid,
 	   '010'::xid8,
 	   '42'::xid8,
-	   '0xffffffffffffffff'::xid8,
-	   '-1'::xid8;
+	   '0xefffffffffffffff'::xid8,
+	   '0'::xid8;
 
 -- garbage values
 select ''::xid;
 select 'asdf'::xid;
 select ''::xid8;
 select 'asdf'::xid8;
+select '-1'::xid8;
+select '0xffffffffffffffff'::xid8;
+select '0x0000300000000001'::xid8;
 
 -- Also try it with non-error-throwing API
 SELECT pg_input_is_valid('42', 'xid');
@@ -23,6 +26,9 @@ SELECT pg_input_error_message('0xffffffffff', 'xid');
 SELECT pg_input_is_valid('42', 'xid8');
 SELECT pg_input_is_valid('asdf', 'xid8');
 SELECT pg_input_error_message('0xffffffffffffffffffff', 'xid8');
+SELECT pg_input_is_valid('-1', 'xid8');
+SELECT pg_input_is_valid('0xffffffffffffffff', 'xid8');
+SELECT pg_input_is_valid('0x0000300000000001', 'xid8');
 
 -- equality
 select '1'::xid = '1'::xid;
@@ -51,7 +57,7 @@ select xid8cmp('1', '2'), xid8cmp('2', '2'), xid8cmp('2', '1');
 
 -- min() and max() for xid8
 create table xid8_t1 (x xid8);
-insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xffffffffffffffff'), ('-1');
+insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xefffffffffffffff');
 select min(x), max(x) from xid8_t1;
 
 -- xid8 has btree and hash opclasses
-- 
2.38.0

From 8be634900796630a772c0131925f38f136fed599 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 9 Jan 2023 10:23:10 -0800
Subject: [PATCH v3 1/6] WIP: Fix corruption due to vacuum_defer_cleanup_age
 underflowing 64bit xids

Author:
Reviewed-by:
Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
Backpatch:
---
 src/backend/storage/ipc/procarray.c | 85 +++++++++++++++++++++++++----
 1 file changed, 73 insertions(+), 12 deletions(-)

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 4340bf96416..64d0896b23b 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -367,6 +367,9 @@ static inline void ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId l
 static void ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid);
 static void MaintainLatestCompletedXid(TransactionId latestXid);
 static void MaintainLatestCompletedXidRecovery(TransactionId latestXid);
+static void TransactionIdRetreatSafely(TransactionId *xid,
+									   int retreat_by,
+									   FullTransactionId rel);
 
 static inline FullTransactionId FullXidRelativeTo(FullTransactionId rel,
 												  TransactionId xid);
@@ -1888,17 +1891,35 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
 		 * so guc.c should limit it to no more than the xidStopLimit threshold
 		 * in varsup.c.  Also note that we intentionally don't apply
 		 * vacuum_defer_cleanup_age on standby servers.
+		 *
+		 * Need to use TransactionIdRetreatSafely() instead of open-coding the
+		 * subtraction, to prevent creating an xid before
+		 * FirstNormalTransactionId.
 		 */
-		h->oldest_considered_running =
-			TransactionIdRetreatedBy(h->oldest_considered_running,
-									 vacuum_defer_cleanup_age);
-		h->shared_oldest_nonremovable =
-			TransactionIdRetreatedBy(h->shared_oldest_nonremovable,
-									 vacuum_defer_cleanup_age);
-		h->data_oldest_nonremovable =
-			TransactionIdRetreatedBy(h->data_oldest_nonremovable,
-									 vacuum_defer_cleanup_age);
-		/* defer doesn't apply to temp relations */
+		Assert(TransactionIdPrecedesOrEquals(h->oldest_considered_running,
+											 h->shared_oldest_nonremovable));
+		Assert(TransactionIdPrecedesOrEquals(h->shared_oldest_nonremovable,
+											 h->data_oldest_nonremovable));
+
+		if (vacuum_defer_cleanup_age > 0)
+		{
+			TransactionIdRetreatSafely(&h->oldest_considered_running,
+									   vacuum_defer_cleanup_age,
+									   h->latest_completed);
+			TransactionIdRetreatSafely(&h->shared_oldest_nonremovable,
+									   vacuum_defer_cleanup_age,
+									   h->latest_completed);
+			TransactionIdRetreatSafely(&h->data_oldest_nonremovable,
+									   vacuum_defer_cleanup_age,
+									   h->latest_completed);
+			/* defer doesn't apply to temp relations */
+
+
+			Assert(TransactionIdPrecedesOrEquals(h->oldest_considered_running,
+												 h->shared_oldest_nonremovable));
+			Assert(TransactionIdPrecedesOrEquals(h->shared_oldest_nonremovable,
+												 h->data_oldest_nonremovable));
+		}
 	}
 
 	/*
@@ -2470,8 +2491,10 @@ GetSnapshotData(Snapshot snapshot)
 		oldestfxid = FullXidRelativeTo(latest_completed, oldestxid);
 
 		/* apply vacuum_defer_cleanup_age */
-		def_vis_xid_data =
-			TransactionIdRetreatedBy(xmin, vacuum_defer_cleanup_age);
+		def_vis_xid_data = xmin;
+		TransactionIdRetreatSafely(&def_vis_xid_data,
+								   vacuum_defer_cleanup_age,
+								   oldestfxid);
 
 		/* Check whether there's a replication slot requiring an older xmin. */
 		def_vis_xid_data =
@@ -4295,6 +4318,44 @@ GlobalVisCheckRemovableXid(Relation rel, TransactionId xid)
 	return GlobalVisTestIsRemovableXid(state, xid);
 }
 
+/*
+ * Safely retract *xid by retreat_by, store the result in *xid.
+ *
+ * Need to be careful to prevent *xid from retreating below
+ * FirstNormalTransactionId during epoch 0. This is important to prevent
+ * generating xids that cannot be converted to a FullTransactionId without
+ * wrapping around.
+ *
+ * If retreat_by would lead to a too old xid, FirstNormalTransactionId is
+ * returned instead.
+ */
+static void
+TransactionIdRetreatSafely(TransactionId *xid, int retreat_by, FullTransactionId rel)
+{
+	TransactionId original_xid = *xid;
+	FullTransactionId fxid;
+	uint64		fxid_i;
+
+	Assert(TransactionIdIsNormal(original_xid));
+	Assert(retreat_by >= 0);	/* relevant GUCs are stored as ints */
+	AssertTransactionIdInAllowableRange(original_xid);
+
+	if (retreat_by == 0)
+		return;
+
+	fxid = FullXidRelativeTo(rel, original_xid);
+	fxid_i = U64FromFullTransactionId(fxid);
+
+	if ((fxid_i - FirstNormalTransactionId) <= retreat_by)
+		*xid = FirstNormalTransactionId;
+	else
+	{
+		*xid = TransactionIdRetreatedBy(original_xid, retreat_by);
+		Assert(TransactionIdIsNormal(*xid));
+		Assert(NormalTransactionIdPrecedes(*xid, original_xid));
+	}
+}
+
 /*
  * Convert a 32 bit transaction id into 64 bit transaction id, by assuming it
  * is within MaxTransactionId / 2 of XidFromFullTransactionId(rel).
-- 
2.38.0

From 356f709f3173da84c464b7aaedac5a8595619610 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 30 Jan 2023 11:50:44 -0800
Subject: [PATCH v3 2/6] WIP: very incomplete test for vacuum_defer_cleanup_age

But it's enough to notice the corruption without the bugfix
---
 src/test/recovery/meson.build            |   1 +
 src/test/recovery/t/034_defer_cleanup.pl | 173 +++++++++++++++++++++++
 2 files changed, 174 insertions(+)
 create mode 100644 src/test/recovery/t/034_defer_cleanup.pl

diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index edaaa1a3ce5..da355782b02 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -40,6 +40,7 @@ tests += {
       't/031_recovery_conflict.pl',
       't/032_relfilenode_reuse.pl',
       't/033_replay_tsp_drops.pl',
+      't/034_defer_cleanup.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/034_defer_cleanup.pl b/src/test/recovery/t/034_defer_cleanup.pl
new file mode 100644
index 00000000000..77ce6037db8
--- /dev/null
+++ b/src/test/recovery/t/034_defer_cleanup.pl
@@ -0,0 +1,173 @@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+# Check that vacuum_defer_cleanup_age works.
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $primary = PostgreSQL::Test::Cluster->new('primary');
+$primary->init(allows_streaming => 1);
+$primary->start;
+
+# to make things more predictable, we schedule all vacuums ourselves
+$primary->append_conf('postgresql.conf', qq[
+autovacuum = 0
+log_line_prefix='%m [%p][%b][%v:%x][%a] '
+]);
+
+$primary->backup('backup');
+my $standby = PostgreSQL::Test::Cluster->new('standby');
+$standby->init_from_backup($primary, 'backup',
+  has_streaming => 1);
+
+# vacuum_defer_cleanup_age only makes sense without feedback
+# We don't want to ever wait for the standby to apply, otherwise the test will
+# take forever.
+$standby->append_conf('postgresql.conf', qq[
+hot_standby_feedback = 0
+max_standby_streaming_delay = 0
+]);
+$standby->start;
+
+# to avoid constantly needing to wait manually, use syncrep with remote_apply
+$primary->append_conf('postgresql.conf', qq[
+synchronous_standby_names = '*'
+synchronous_commit = remote_apply
+]);
+
+$standby->reload;
+
+
+# Find original txid at start, we want to use a cleanup age bigger than that,
+# because we had bugs with horizons wrapping around to before the big bang
+my $xid_at_start = $primary->safe_psql('postgres', 'SELECT txid_current()');
+my $defer_age = $xid_at_start + 100;
+
+note "Initial xid is $xid_at_start, will set defer to $defer_age";
+
+$primary->safe_psql('postgres', qq[
+  ALTER SYSTEM SET vacuum_defer_cleanup_age = $defer_age;
+  SELECT pg_reload_conf();
+]);
+
+$primary->safe_psql('postgres', qq[
+  CREATE TABLE testdata(id int not null unique, data text);
+  INSERT INTO testdata(id, data) VALUES(1, '1');
+  INSERT INTO testdata(id, data) VALUES(2, '2');
+  INSERT INTO testdata(id, data) VALUES(3, '3');
+  INSERT INTO testdata(id, data) VALUES(4, '4');
+  INSERT INTO testdata(id, data) VALUES(5, '5');
+]);
+
+
+# start a background psql on both nodes, to test effects on running sessions
+my $psql_timeout = IPC::Run::timer($PostgreSQL::Test::Utils::timeout_default);
+my %psql_primary = ('stdin' => '', 'stdout' => '');
+$psql_primary{run} =
+  $primary->background_psql('postgres', \$psql_primary{stdin},
+	\$psql_primary{stdout},
+	$psql_timeout);
+
+my %psql_standby = ('stdin' => '', 'stdout' => '');
+$psql_standby{run} =
+  $standby->background_psql('postgres', \$psql_standby{stdin},
+	\$psql_standby{stdout},
+	$psql_timeout);
+
+$psql_primary{stdin} .= '\t\set QUIET off';
+$psql_standby{stdin} .= '\t\set QUIET off';
+
+# start transaction on the standby
+my $q = qq[
+SET application_name = 'background_psql';
+BEGIN TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+SELECT 1;
+];
+$psql_primary{stdin} .= $q;
+$psql_standby{stdin} .= $q;
+ok( pump_until_primary(qr/\n\(1 row\)\n$/s), 'started transaction');
+ok( pump_until_standby(qr/\n\(1 row\)\n$/s), 'started transaction');
+
+# update a row on the primary, select from the table to remove old row version
+my $res = $primary->safe_psql('postgres', qq[
+  SELECT txid_current();
+  UPDATE testdata SET data = data || '-updated' WHERE id = 1;
+  SELECT txid_current();
+  VACUUM testdata;
+  SELECT count(*) FROM testdata;
+  SELECT count(*) FROM testdata;
+  SELECT txid_current();
+  SET enable_seqscan = 0;
+  EXPLAIN SELECT * FROM testdata WHERE id = 1;
+  SELECT * FROM testdata WHERE id = 1;
+]);
+
+
+# check query result is as expected on primary and on a new snapshot on the standby
+$q = "SELECT data FROM testdata WHERE id = 1";
+is($primary->safe_psql('postgres', $q), '1-updated', "query result on primary as expected");
+is($standby->safe_psql('postgres', $q), '1-updated', "query result on standby as expected");
+
+$q = "SELECT data FROM testdata WHERE id = 1;\n";
+$psql_primary{stdin} .= $q;
+$psql_standby{stdin} .= $q;
+
+$res = pump_until_primary(qr/^.*\n\(1 row\)\n$/s);
+my $expect = "data\n1\n(1 row)\n";
+is( $res, $expect, 'version 1 still visible on primary after update on primary');
+$res = pump_until_standby(qr/.*\n\(1 row\)\n$/s);
+is( $res, $expect, 'row 1, version 1 still visible on standby after update on primary');
+
+$psql_primary{stdin} .= "COMMIT;\n";
+is(pump_until_primary(qr/^COMMIT\n/m), "COMMIT\n", "release on primary");
+
+
+done_testing();
+
+sub pump_until_node
+{
+	my $psql = shift;
+	my $match = shift;
+	my $ret;
+
+	pump_until($$psql{run}, $psql_timeout,
+		\$$psql{stdout}, $match);
+	$ret = $$psql{stdout};
+	$$psql{stdout} = '';
+	return $ret;
+}
+
+sub pump_until_primary
+{
+	my $match = shift;
+
+	return pump_until_node(\%psql_primary, $match);
+}
+
+sub pump_until_standby
+{
+	my $match = shift;
+
+	return pump_until_node(\%psql_standby, $match);
+}
+
+sub burn_xids
+{
+	my $cnt = shift;
+
+	my $out = $primary->safe_psql('postgres', qq[
+SELECT txid_current();
+DO $$
+  BEGIN FOR i IN 1..100 LOOP
+    PERFORM txid_current();
+    COMMIT AND CHAIN;
+  END LOOP;
+END; $$;
+SELECT txid_current();
+]);
+	note $out;
+}
-- 
2.38.0

From a3032b89ce8e8e325035e2858a2b58a92d6ede7c Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 9 Jan 2023 11:24:37 -0800
Subject: [PATCH v3 3/6] wip: amcheck: Fix bugs relating to 64bit xids

Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
---
 contrib/amcheck/verify_heapam.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 4fcfd6df72d..56a3b3e9974 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -1576,11 +1576,25 @@ FullTransactionIdFromXidAndCtx(TransactionId xid, const HeapCheckContext *ctx)
 {
 	uint32		epoch;
 
+	Assert(TransactionIdIsValid(ctx->next_xid));
+	Assert(FullTransactionIdIsValid(ctx->next_fxid));
+
 	if (!TransactionIdIsNormal(xid))
 		return FullTransactionIdFromEpochAndXid(0, xid);
 	epoch = EpochFromFullTransactionId(ctx->next_fxid);
 	if (xid > ctx->next_xid)
+	{
+		/*
+		 * FIXME, doubtful this is the best fix.
+		 *
+		 * Can't represent the 32bit xid as a 64bit xid, as it's before fxid
+		 * 0. Represent it as an xid from the future instead.
+		 */
+		if (epoch == 0)
+			return FullTransactionIdFromEpochAndXid(0, xid);
+
 		epoch--;
+	}
 	return FullTransactionIdFromEpochAndXid(epoch, xid);
 }
 
@@ -1597,8 +1611,8 @@ update_cached_xid_range(HeapCheckContext *ctx)
 	LWLockRelease(XidGenLock);
 
 	/* And compute alternate versions of the same */
-	ctx->oldest_fxid = FullTransactionIdFromXidAndCtx(ctx->oldest_xid, ctx);
 	ctx->next_xid = XidFromFullTransactionId(ctx->next_fxid);
+	ctx->oldest_fxid = FullTransactionIdFromXidAndCtx(ctx->oldest_xid, ctx);
 }
 
 /*
-- 
2.38.0

From 5f995d8f3893adf263942606f2e0511e9d21f8f4 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 9 Jan 2023 11:25:43 -0800
Subject: [PATCH v3 4/6] wip: stricter validation for 64bit xids

64bit xids can't represent xids before xid 0, but 32bit xids can. This has
lead to a number of bugs, some data corrupting. To make it easier to catch
such bugs, disallow 64bit where the upper 32bit are all set, which is what one
gets when naively trying to convert such 32bit xids. Additionally explicitly
disallow 64bit xids that are already aren't allowed because they'd map to
special 32bit xids.

This changes the input routines for 64bit xids to error out in more cases.

Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
---
 src/include/access/transam.h      | 136 ++++++++++++++++++++++++++++--
 src/backend/utils/adt/xid.c       |   7 ++
 src/backend/utils/adt/xid8funcs.c |  17 ++--
 src/test/regress/expected/xid.out |  44 ++++++++--
 src/test/regress/sql/xid.sql      |  12 ++-
 5 files changed, 192 insertions(+), 24 deletions(-)

diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index f5af6d30556..0d4bf57e63a 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -44,15 +44,6 @@
 #define TransactionIdStore(xid, dest)	(*(dest) = (xid))
 #define StoreInvalidTransactionId(dest) (*(dest) = InvalidTransactionId)
 
-#define EpochFromFullTransactionId(x)	((uint32) ((x).value >> 32))
-#define XidFromFullTransactionId(x)		((uint32) (x).value)
-#define U64FromFullTransactionId(x)		((x).value)
-#define FullTransactionIdEquals(a, b)	((a).value == (b).value)
-#define FullTransactionIdPrecedes(a, b)	((a).value < (b).value)
-#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <= (b).value)
-#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
-#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >= (b).value)
-#define FullTransactionIdIsValid(x)		TransactionIdIsValid(XidFromFullTransactionId(x))
 #define InvalidFullTransactionId		FullTransactionIdFromEpochAndXid(0, InvalidTransactionId)
 #define FirstNormalFullTransactionId	FullTransactionIdFromEpochAndXid(0, FirstNormalTransactionId)
 #define FullTransactionIdIsNormal(x)	FullTransactionIdFollowsOrEquals(x, FirstNormalFullTransactionId)
@@ -61,12 +52,127 @@
  * A 64 bit value that contains an epoch and a TransactionId.  This is
  * wrapped in a struct to prevent implicit conversion to/from TransactionId.
  * Not all values represent valid normal XIDs.
+ *
+ * 64bit xids that would map to a non-normal 32bit xid are not allowed, for
+ * obvious reasons.
+
+ * In addition, no xids with all the upper 32bit sets are allowed to exist,
+ * this is to make it easier to catch conversion errors from 32bit xids (which
+ * can point to "before" xid 0).
  */
 typedef struct FullTransactionId
 {
 	uint64		value;
 } FullTransactionId;
 
+#define MAX_FULL_TRANSACTION_ID ((((uint64)~(uint32)0)<<32) - 1)
+
+/*
+ * This is separate from FullTransactionIdValidRange() so that input routines
+ * can check for invalid values without triggering an assert.
+ */
+static inline bool
+InFullTransactionIdRange(uint64 val)
+{
+	/* 64bit xid above the upper bound */
+	if (val > MAX_FULL_TRANSACTION_ID)
+		return false;
+
+	/*
+	 * normal 64bit xids that'd map to special 32 xids aren't allowed.
+	 */
+	if (val >= (uint64) FirstNormalTransactionId &&
+		(uint32) val < FirstNormalTransactionId)
+		return false;
+	return true;
+}
+
+/*
+ * Validation routine, typically used in asserts.
+ */
+static inline bool
+FullTransactionIdValidRange(FullTransactionId x)
+{
+	return InFullTransactionIdRange(x.value);
+}
+
+static inline uint64
+U64FromFullTransactionId(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return x.value;
+}
+
+static inline TransactionId
+XidFromFullTransactionId(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return (TransactionId) x.value;
+}
+
+static inline uint32
+EpochFromFullTransactionId(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return (uint32) (x.value >> 32);
+}
+
+static inline bool
+FullTransactionIdEquals(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value == b.value;
+}
+
+static inline bool
+FullTransactionIdPrecedes(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value < b.value;
+}
+
+static inline bool
+FullTransactionIdPrecedesOrEquals(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value <= b.value;
+}
+
+static inline bool
+FullTransactionIdFollows(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value > b.value;
+}
+
+static inline bool
+FullTransactionIdFollowsOrEquals(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value >= b.value;
+}
+
+static inline bool
+FullTransactionIdIsValid(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return x.value != (uint64) InvalidTransactionId;
+}
+
 static inline FullTransactionId
 FullTransactionIdFromEpochAndXid(uint32 epoch, TransactionId xid)
 {
@@ -74,6 +180,8 @@ FullTransactionIdFromEpochAndXid(uint32 epoch, TransactionId xid)
 
 	result.value = ((uint64) epoch) << 32 | xid;
 
+	Assert(FullTransactionIdValidRange(result));
+
 	return result;
 }
 
@@ -84,6 +192,8 @@ FullTransactionIdFromU64(uint64 value)
 
 	result.value = value;
 
+	Assert(FullTransactionIdValidRange(result));
+
 	return result;
 }
 
@@ -102,6 +212,8 @@ FullTransactionIdFromU64(uint64 value)
 static inline void
 FullTransactionIdRetreat(FullTransactionId *dest)
 {
+	Assert(FullTransactionIdValidRange(*dest));
+
 	dest->value--;
 
 	/*
@@ -118,6 +230,8 @@ FullTransactionIdRetreat(FullTransactionId *dest)
 	 */
 	while (XidFromFullTransactionId(*dest) < FirstNormalTransactionId)
 		dest->value--;
+
+	Assert(FullTransactionIdValidRange(*dest));
 }
 
 /*
@@ -127,6 +241,8 @@ FullTransactionIdRetreat(FullTransactionId *dest)
 static inline void
 FullTransactionIdAdvance(FullTransactionId *dest)
 {
+	Assert(FullTransactionIdValidRange(*dest));
+
 	dest->value++;
 
 	/* see FullTransactionIdAdvance() */
@@ -135,6 +251,8 @@ FullTransactionIdAdvance(FullTransactionId *dest)
 
 	while (XidFromFullTransactionId(*dest) < FirstNormalTransactionId)
 		dest->value++;
+
+	Assert(FullTransactionIdValidRange(*dest));
 }
 
 /* back up a transaction ID variable, handling wraparound correctly */
diff --git a/src/backend/utils/adt/xid.c b/src/backend/utils/adt/xid.c
index 8ac1679c381..21c0f4c67df 100644
--- a/src/backend/utils/adt/xid.c
+++ b/src/backend/utils/adt/xid.c
@@ -188,6 +188,13 @@ xid8in(PG_FUNCTION_ARGS)
 	uint64		result;
 
 	result = uint64in_subr(str, NULL, "xid8", fcinfo->context);
+
+	if (!InFullTransactionIdRange(result))
+		ereturn(fcinfo->context, 0,
+				(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+				 errmsg("value \"%s\" is out of range for type %s",
+						str, "xid8")));
+
 	PG_RETURN_FULLTRANSACTIONID(FullTransactionIdFromU64(result));
 }
 
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index e616303a292..8b33d145a36 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -302,15 +302,18 @@ parse_snapshot(const char *str, Node *escontext)
 	const char *str_start = str;
 	char	   *endp;
 	StringInfo	buf;
+	uint64		raw_fxid;
 
-	xmin = FullTransactionIdFromU64(strtou64(str, &endp, 10));
-	if (*endp != ':')
+	raw_fxid = strtou64(str, &endp, 10);
+	if (*endp != ':' || !InFullTransactionIdRange(raw_fxid))
 		goto bad_format;
+	xmin = FullTransactionIdFromU64(raw_fxid);
 	str = endp + 1;
 
-	xmax = FullTransactionIdFromU64(strtou64(str, &endp, 10));
-	if (*endp != ':')
+	raw_fxid = strtou64(str, &endp, 10);
+	if (*endp != ':' || !InFullTransactionIdRange(raw_fxid))
 		goto bad_format;
+	xmax = FullTransactionIdFromU64(raw_fxid);
 	str = endp + 1;
 
 	/* it should look sane */
@@ -326,7 +329,11 @@ parse_snapshot(const char *str, Node *escontext)
 	while (*str != '\0')
 	{
 		/* read next value */
-		val = FullTransactionIdFromU64(strtou64(str, &endp, 10));
+		raw_fxid = strtou64(str, &endp, 10);
+
+		val = FullTransactionIdFromU64(raw_fxid);
+		if (!InFullTransactionIdRange(raw_fxid))
+			goto bad_format;
 		str = endp;
 
 		/* require the input to be in order */
diff --git a/src/test/regress/expected/xid.out b/src/test/regress/expected/xid.out
index e62f7019434..4348eb66bd5 100644
--- a/src/test/regress/expected/xid.out
+++ b/src/test/regress/expected/xid.out
@@ -6,11 +6,11 @@ select '010'::xid,
        '-1'::xid,
 	   '010'::xid8,
 	   '42'::xid8,
-	   '0xffffffffffffffff'::xid8,
-	   '-1'::xid8;
- xid | xid |    xid     |    xid     | xid8 | xid8 |         xid8         |         xid8         
------+-----+------------+------------+------+------+----------------------+----------------------
-   8 |  42 | 4294967295 | 4294967295 |    8 |   42 | 18446744073709551615 | 18446744073709551615
+	   '0xefffffffffffffff'::xid8,
+	   '0'::xid8;
+ xid | xid |    xid     |    xid     | xid8 | xid8 |         xid8         | xid8 
+-----+-----+------------+------------+------+------+----------------------+------
+   8 |  42 | 4294967295 | 4294967295 |    8 |   42 | 17293822569102704639 |    0
 (1 row)
 
 -- garbage values
@@ -30,6 +30,18 @@ select 'asdf'::xid8;
 ERROR:  invalid input syntax for type xid8: "asdf"
 LINE 1: select 'asdf'::xid8;
                ^
+select '-1'::xid8;
+ERROR:  value "-1" is out of range for type xid8
+LINE 1: select '-1'::xid8;
+               ^
+select '0xffffffffffffffff'::xid8;
+ERROR:  value "0xffffffffffffffff" is out of range for type xid8
+LINE 1: select '0xffffffffffffffff'::xid8;
+               ^
+select '0x0000300000000001'::xid8;
+ERROR:  value "0x0000300000000001" is out of range for type xid8
+LINE 1: select '0x0000300000000001'::xid8;
+               ^
 -- Also try it with non-error-throwing API
 SELECT pg_input_is_valid('42', 'xid');
  pg_input_is_valid 
@@ -67,6 +79,24 @@ SELECT pg_input_error_message('0xffffffffffffffffffff', 'xid8');
  value "0xffffffffffffffffffff" is out of range for type xid8
 (1 row)
 
+SELECT pg_input_is_valid('-1', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
+SELECT pg_input_is_valid('0xffffffffffffffff', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
+SELECT pg_input_is_valid('0x0000300000000001', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
 -- equality
 select '1'::xid = '1'::xid;
  ?column? 
@@ -160,11 +190,11 @@ select xid8cmp('1', '2'), xid8cmp('2', '2'), xid8cmp('2', '1');
 
 -- min() and max() for xid8
 create table xid8_t1 (x xid8);
-insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xffffffffffffffff'), ('-1');
+insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xefffffffffffffff');
 select min(x), max(x) from xid8_t1;
  min |         max          
 -----+----------------------
-   0 | 18446744073709551615
+   0 | 17293822569102704639
 (1 row)
 
 -- xid8 has btree and hash opclasses
diff --git a/src/test/regress/sql/xid.sql b/src/test/regress/sql/xid.sql
index b6996588ef6..b2104e4ce20 100644
--- a/src/test/regress/sql/xid.sql
+++ b/src/test/regress/sql/xid.sql
@@ -7,14 +7,17 @@ select '010'::xid,
        '-1'::xid,
 	   '010'::xid8,
 	   '42'::xid8,
-	   '0xffffffffffffffff'::xid8,
-	   '-1'::xid8;
+	   '0xefffffffffffffff'::xid8,
+	   '0'::xid8;
 
 -- garbage values
 select ''::xid;
 select 'asdf'::xid;
 select ''::xid8;
 select 'asdf'::xid8;
+select '-1'::xid8;
+select '0xffffffffffffffff'::xid8;
+select '0x0000300000000001'::xid8;
 
 -- Also try it with non-error-throwing API
 SELECT pg_input_is_valid('42', 'xid');
@@ -23,6 +26,9 @@ SELECT pg_input_error_message('0xffffffffff', 'xid');
 SELECT pg_input_is_valid('42', 'xid8');
 SELECT pg_input_is_valid('asdf', 'xid8');
 SELECT pg_input_error_message('0xffffffffffffffffffff', 'xid8');
+SELECT pg_input_is_valid('-1', 'xid8');
+SELECT pg_input_is_valid('0xffffffffffffffff', 'xid8');
+SELECT pg_input_is_valid('0x0000300000000001', 'xid8');
 
 -- equality
 select '1'::xid = '1'::xid;
@@ -51,7 +57,7 @@ select xid8cmp('1', '2'), xid8cmp('2', '2'), xid8cmp('2', '1');
 
 -- min() and max() for xid8
 create table xid8_t1 (x xid8);
-insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xffffffffffffffff'), ('-1');
+insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xefffffffffffffff');
 select min(x), max(x) from xid8_t1;
 
 -- xid8 has btree and hash opclasses
-- 
2.38.0

From ffa80fece3384e3a12a52de18102a0d169f52841 Mon Sep 17 00:00:00 2001
From: Nkey <michail.nikolaev@gmail.com>
Date: Sat, 4 Feb 2023 15:16:52 +0300
Subject: [PATCH] WIP: Fix corruption due to vacuum_defer_cleanup_age
 underflowing 64bit xids

---
 src/backend/storage/ipc/procarray.c | 56 ++++++++++++-----------------
 src/include/access/transam.h        | 71 +++++++++++++++++++++++++++++++++----
 2 files changed, 87 insertions(+), 40 deletions(-)

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 4340bf9641..5dd662cd75 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -368,8 +368,6 @@ static void ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid);
 static void MaintainLatestCompletedXid(TransactionId latestXid);
 static void MaintainLatestCompletedXidRecovery(TransactionId latestXid);
 
-static inline FullTransactionId FullXidRelativeTo(FullTransactionId rel,
-												  TransactionId xid);
 static void GlobalVisUpdateApply(ComputeXidHorizonsResult *horizons);
 
 /*
@@ -1889,16 +1887,32 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
 		 * in varsup.c.  Also note that we intentionally don't apply
 		 * vacuum_defer_cleanup_age on standby servers.
 		 */
+		Assert(TransactionIdPrecedesOrEquals(h->oldest_considered_running,
+											 h->shared_oldest_nonremovable));
+		Assert(TransactionIdPrecedesOrEquals(h->shared_oldest_nonremovable,
+											 h->data_oldest_nonremovable));
+
 		h->oldest_considered_running =
 			TransactionIdRetreatedBy(h->oldest_considered_running,
-									 vacuum_defer_cleanup_age);
-		h->shared_oldest_nonremovable =
+									 vacuum_defer_cleanup_age,
+									 h->latest_completed);
+
+		h->shared_oldest_nonremovable = 
 			TransactionIdRetreatedBy(h->shared_oldest_nonremovable,
-									 vacuum_defer_cleanup_age);
-		h->data_oldest_nonremovable =
+									 vacuum_defer_cleanup_age,
+									 h->latest_completed);
+
+		h->data_oldest_nonremovable = 
 			TransactionIdRetreatedBy(h->data_oldest_nonremovable,
-									 vacuum_defer_cleanup_age);
+									 vacuum_defer_cleanup_age,
+									h->latest_completed);
+
 		/* defer doesn't apply to temp relations */
+
+		Assert(TransactionIdPrecedesOrEquals(h->oldest_considered_running,
+											 h->shared_oldest_nonremovable));
+		Assert(TransactionIdPrecedesOrEquals(h->shared_oldest_nonremovable,
+											 h->data_oldest_nonremovable));
 	}
 
 	/*
@@ -2471,7 +2485,7 @@ GetSnapshotData(Snapshot snapshot)
 
 		/* apply vacuum_defer_cleanup_age */
 		def_vis_xid_data =
-			TransactionIdRetreatedBy(xmin, vacuum_defer_cleanup_age);
+			TransactionIdRetreatedBy(xmin, vacuum_defer_cleanup_age, oldestfxid);
 
 		/* Check whether there's a replication slot requiring an older xmin. */
 		def_vis_xid_data =
@@ -4295,32 +4309,6 @@ GlobalVisCheckRemovableXid(Relation rel, TransactionId xid)
 	return GlobalVisTestIsRemovableXid(state, xid);
 }
 
-/*
- * Convert a 32 bit transaction id into 64 bit transaction id, by assuming it
- * is within MaxTransactionId / 2 of XidFromFullTransactionId(rel).
- *
- * Be very careful about when to use this function. It can only safely be used
- * when there is a guarantee that xid is within MaxTransactionId / 2 xids of
- * rel. That e.g. can be guaranteed if the caller assures a snapshot is
- * held by the backend and xid is from a table (where vacuum/freezing ensures
- * the xid has to be within that range), or if xid is from the procarray and
- * prevents xid wraparound that way.
- */
-static inline FullTransactionId
-FullXidRelativeTo(FullTransactionId rel, TransactionId xid)
-{
-	TransactionId rel_xid = XidFromFullTransactionId(rel);
-
-	Assert(TransactionIdIsValid(xid));
-	Assert(TransactionIdIsValid(rel_xid));
-
-	/* not guaranteed to find issues, but likely to catch mistakes */
-	AssertTransactionIdInAllowableRange(xid);
-
-	return FullTransactionIdFromU64(U64FromFullTransactionId(rel)
-									+ (int32) (xid - rel_xid));
-}
-
 
 /* ----------------------------------------------
  *		KnownAssignedTransactionIds sub-module
diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index f5af6d3055..e094e80859 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -317,16 +317,75 @@ ReadNextTransactionId(void)
 	return XidFromFullTransactionId(ReadNextFullTransactionId());
 }
 
-/* return transaction ID backed up by amount, handling wraparound correctly */
+/*
+ * Convert a 32 bit transaction id into 64 bit transaction id, by assuming it
+ * is within MaxTransactionId / 2 of XidFromFullTransactionId(rel).
+ *
+ * Be very careful about when to use this function. It can only safely be used
+ * when there is a guarantee that xid is within MaxTransactionId / 2 xids of
+ * rel. That e.g. can be guaranteed if the caller assures a snapshot is
+ * held by the backend and xid is from a table (where vacuum/freezing ensures
+ * the xid has to be within that range), or if xid is from the procarray and
+ * prevents xid wraparound that way.
+ */
+static inline FullTransactionId
+FullXidRelativeTo(FullTransactionId rel, TransactionId xid)
+{
+	TransactionId rel_xid = XidFromFullTransactionId(rel);
+
+	Assert(TransactionIdIsValid(xid));
+	Assert(TransactionIdIsValid(rel_xid));
+
+	/* not guaranteed to find issues, but likely to catch mistakes */
+	AssertTransactionIdInAllowableRange(xid);
+
+	return FullTransactionIdFromU64(U64FromFullTransactionId(rel)
+		+ (int32)(xid - rel_xid));
+}
+
+/*
+ * return transaction ID backed up by amount, handling wraparound correctly
+ *
+ * Need to be careful to prevent xid from retreating below
+ * FirstNormalTransactionId during epoch 0. This is important to prevent
+ * generating xids that cannot be converted to a FullTransactionId without
+ * wrapping around.
+ *
+ * If amount would lead to a too old xid, FirstNormalTransactionId is
+ * returned instead.
+ */
+
 static inline TransactionId
-TransactionIdRetreatedBy(TransactionId xid, uint32 amount)
+TransactionIdRetreatedBy(TransactionId xid, int amount, FullTransactionId rel)
 {
-	xid -= amount;
+	FullTransactionId fxid;
+	uint64		fxid_i;
+	TransactionId r;
+
+	Assert(TransactionIdIsNormal(xid));
+	Assert(amount >= 0);	/* relevant GUCs are stored as ints */
+	AssertTransactionIdInAllowableRange(xid);
+
+	if (amount == 0)
+		return xid;
+
+	fxid = FullXidRelativeTo(rel, xid);
+	fxid_i = U64FromFullTransactionId(fxid);
+
+	if ((fxid_i - FirstNormalTransactionId) <= amount)
+		r = FirstNormalTransactionId;
+	else
+	{
+		r = xid - amount;
+
+		while (r < FirstNormalTransactionId)
+			r--;
 
-	while (xid < FirstNormalTransactionId)
-		xid--;
+		Assert(TransactionIdIsNormal(r));
+		Assert(NormalTransactionIdPrecedes(r, xid));
+	}
 
-	return xid;
+	return r;
 }
 
 /* return the older of the two IDs */
-- 
2.16.2.windows.1

From 3f67523bac084964c0e780fddd509f3464d32282 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 8 Mar 2023 11:37:05 -0800
Subject: [PATCH v5 1/8] amcheck: Fix ordering bug in update_cached_xid_range()

The initialization order in update_cached_xid_range() was wrong, calling
FullTransactionIdFromXidAndCtx() before setting
->next_xid. FullTransactionIdFromXidAndCtx() uses ->next_xid.

In most situations this will not cause visible issues, because the next call
to update_cached_xid_range() will use a less wrong ->next_xid. It's rare that
xids advance fast enough for this to be a problem.

Found while adding more asserts to the 64bit xid infrastructure.

Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
Backpatch: 14-, where heapam verification was introduced
---
 contrib/amcheck/verify_heapam.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 4fcfd6df72d..33c5b338959 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -1576,6 +1576,9 @@ FullTransactionIdFromXidAndCtx(TransactionId xid, const HeapCheckContext *ctx)
 {
 	uint32		epoch;
 
+	Assert(TransactionIdIsNormal(ctx->next_xid));
+	Assert(FullTransactionIdIsNormal(ctx->next_fxid));
+
 	if (!TransactionIdIsNormal(xid))
 		return FullTransactionIdFromEpochAndXid(0, xid);
 	epoch = EpochFromFullTransactionId(ctx->next_fxid);
@@ -1597,8 +1600,8 @@ update_cached_xid_range(HeapCheckContext *ctx)
 	LWLockRelease(XidGenLock);
 
 	/* And compute alternate versions of the same */
-	ctx->oldest_fxid = FullTransactionIdFromXidAndCtx(ctx->oldest_xid, ctx);
 	ctx->next_xid = XidFromFullTransactionId(ctx->next_fxid);
+	ctx->oldest_fxid = FullTransactionIdFromXidAndCtx(ctx->oldest_xid, ctx);
 }
 
 /*
-- 
2.38.0

From c376f9c698c2c9cf9ad5316ceb96160611225430 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 8 Mar 2023 11:57:34 -0800
Subject: [PATCH v5 2/8] amcheck: Fix FullTransactionIdFromXidAndCtx() for xids
 before epoch 0

64bit xids can't represent xids before epoch 0 (see also be504a3e974). When
FullTransactionIdFromXidAndCtx() was passed such an xid, it'd create a 64bit
xid far into the future. Noticed while adding assertions in the course of
investigating be504a3e974, as amcheck's test create such xids.

To fix the issue, just return FirstNormalFullTransactionId in this case. A
freshly initdb'd cluster already has a newer horizon. The most minimal version
of this would make the messages for some detected corruptions differently
inaccurate. To make those cases accurate, switch
FullTransactionIdFromXidAndCtx() to use the 32bit modulo difference between
xid and nextxid to compute the 64bit xid, yielding sensible "in the future" /
"in the past" answers.

Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
Backpatch: 14-, where heapam verification was introduced
---
 src/bin/pg_amcheck/t/004_verify_heapam.pl | 28 +++++++++++++------
 contrib/amcheck/verify_heapam.c           | 33 +++++++++++++++++++----
 2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/src/bin/pg_amcheck/t/004_verify_heapam.pl b/src/bin/pg_amcheck/t/004_verify_heapam.pl
index 215c30eaa8e..9984d0d9f87 100644
--- a/src/bin/pg_amcheck/t/004_verify_heapam.pl
+++ b/src/bin/pg_amcheck/t/004_verify_heapam.pl
@@ -217,7 +217,7 @@ my $rel = $node->safe_psql('postgres',
 my $relpath = "$pgdata/$rel";
 
 # Insert data and freeze public.test
-use constant ROWCOUNT => 16;
+use constant ROWCOUNT => 17;
 $node->safe_psql(
 	'postgres', qq(
 	INSERT INTO public.test (a, b, c)
@@ -378,23 +378,24 @@ for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++)
 	elsif ($offnum == 3)
 	{
 		# Corruptly set xmin < datfrozenxid, further back, noting circularity
-		# of xid comparison.  For a new cluster with epoch = 0, the corrupt
-		# xmin will be interpreted as in the future
-		$tup->{t_xmin} = 4026531839;
+		# of xid comparison.
+		my $xmin = 4026531839;
+		$tup->{t_xmin} = $xmin;
 		$tup->{t_infomask} &= ~HEAP_XMIN_COMMITTED;
 		$tup->{t_infomask} &= ~HEAP_XMIN_INVALID;
 
 		push @expected,
-		  qr/${$header}xmin 4026531839 equals or exceeds next valid transaction ID 0:\d+/;
+		  qr/${$header}xmin ${xmin} precedes oldest valid transaction ID 0:\d+/;
 	}
 	elsif ($offnum == 4)
 	{
 		# Corruptly set xmax < relminmxid;
-		$tup->{t_xmax} = 4026531839;
+		my $xmax = 4026531839;
+		$tup->{t_xmax} = $xmax;
 		$tup->{t_infomask} &= ~HEAP_XMAX_INVALID;
 
 		push @expected,
-		  qr/${$header}xmax 4026531839 equals or exceeds next valid transaction ID 0:\d+/;
+		  qr/${$header}xmax ${xmax} precedes oldest valid transaction ID 0:\d+/;
 	}
 	elsif ($offnum == 5)
 	{
@@ -502,7 +503,7 @@ for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++)
 		push @expected,
 		  qr/${header}multitransaction ID 4 equals or exceeds next valid multitransaction ID 1/;
 	}
-	elsif ($offnum == 15)    # Last offnum must equal ROWCOUNT
+	elsif ($offnum == 15)
 	{
 		# Set both HEAP_XMAX_COMMITTED and HEAP_XMAX_IS_MULTI
 		$tup->{t_infomask} |= HEAP_XMAX_COMMITTED;
@@ -512,6 +513,17 @@ for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++)
 		push @expected,
 		  qr/${header}multitransaction ID 4000000000 precedes relation minimum multitransaction ID threshold 1/;
 	}
+	elsif ($offnum == 16)    # Last offnum must equal ROWCOUNT
+	{
+		# Corruptly set xmin > next_xid to be in the future.
+		my $xmin = 123456;
+		$tup->{t_xmin} = $xmin;
+		$tup->{t_infomask} &= ~HEAP_XMIN_COMMITTED;
+		$tup->{t_infomask} &= ~HEAP_XMIN_INVALID;
+
+		push @expected,
+          qr/${$header}xmin ${xmin} equals or exceeds next valid transaction ID 0:\d+/;
+	}
 	write_tuple($file, $offset, $tup);
 }
 close($file)
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 33c5b338959..94ddccd23a8 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -1574,17 +1574,40 @@ check_tuple(HeapCheckContext *ctx)
 static FullTransactionId
 FullTransactionIdFromXidAndCtx(TransactionId xid, const HeapCheckContext *ctx)
 {
-	uint32		epoch;
+	uint64		nextfxid_i;
+	int32		diff;
+	FullTransactionId fxid;
 
 	Assert(TransactionIdIsNormal(ctx->next_xid));
 	Assert(FullTransactionIdIsNormal(ctx->next_fxid));
+	Assert(XidFromFullTransactionId(ctx->next_fxid) == ctx->next_xid);
 
 	if (!TransactionIdIsNormal(xid))
 		return FullTransactionIdFromEpochAndXid(0, xid);
-	epoch = EpochFromFullTransactionId(ctx->next_fxid);
-	if (xid > ctx->next_xid)
-		epoch--;
-	return FullTransactionIdFromEpochAndXid(epoch, xid);
+
+	nextfxid_i = U64FromFullTransactionId(ctx->next_fxid);
+
+	/* compute the 32bit modulo difference */
+	diff = (int32) (ctx->next_xid - xid);
+
+	/*
+	 * In cases of corruption we might see a 32bit xid that is before epoch
+	 * 0. We can't represent that as a 64bit xid, due to 64bit xids being
+	 * unsigned integers, without the modulo arithmetic of 32bit xid. There's
+	 * no really nice way to deal with that, but it works ok enough to use
+	 * FirstNormalFullTransactionId in that case, as a freshly initdb'd
+	 * cluster already has a newer horizon.
+	 */
+	if (diff > 0 && (nextfxid_i - FirstNormalTransactionId) < (int64) diff)
+	{
+		Assert(EpochFromFullTransactionId(ctx->next_fxid) == 0);
+		fxid = FirstNormalFullTransactionId;
+	}
+	else
+		fxid = FullTransactionIdFromU64(nextfxid_i - diff);
+
+	Assert(FullTransactionIdIsNormal(fxid));
+	return fxid;
 }
 
 /*
-- 
2.38.0

From 327d170e25a90e7ea38bad37b32d207972edcc7b Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 27 Feb 2023 17:09:21 -0800
Subject: [PATCH v5 3/8] wip: stricter validation for 64bit xids

64bit xids can't represent xids before xid 0, but 32bit xids can. This has
lead to a number of bugs, some data corrupting. To make it easier to catch
such bugs, disallow 64bit where the upper 32bit are all set, which is what one
gets when naively trying to convert such 32bit xids. Additionally explicitly
disallow 64bit xids that are already aren't allowed because they'd map to
special 32bit xids.

This changes the input routines for 64bit xids to error out in more cases.

Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de
---
 src/include/access/transam.h      | 136 ++++++++++++++++++++++++++++--
 src/backend/utils/adt/xid.c       |   7 ++
 src/backend/utils/adt/xid8funcs.c |  17 ++--
 src/test/regress/expected/xid.out |  44 ++++++++--
 src/test/regress/sql/xid.sql      |  12 ++-
 5 files changed, 192 insertions(+), 24 deletions(-)

diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index f5af6d30556..0cf112e5bf2 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -44,15 +44,6 @@
 #define TransactionIdStore(xid, dest)	(*(dest) = (xid))
 #define StoreInvalidTransactionId(dest) (*(dest) = InvalidTransactionId)
 
-#define EpochFromFullTransactionId(x)	((uint32) ((x).value >> 32))
-#define XidFromFullTransactionId(x)		((uint32) (x).value)
-#define U64FromFullTransactionId(x)		((x).value)
-#define FullTransactionIdEquals(a, b)	((a).value == (b).value)
-#define FullTransactionIdPrecedes(a, b)	((a).value < (b).value)
-#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <= (b).value)
-#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
-#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >= (b).value)
-#define FullTransactionIdIsValid(x)		TransactionIdIsValid(XidFromFullTransactionId(x))
 #define InvalidFullTransactionId		FullTransactionIdFromEpochAndXid(0, InvalidTransactionId)
 #define FirstNormalFullTransactionId	FullTransactionIdFromEpochAndXid(0, FirstNormalTransactionId)
 #define FullTransactionIdIsNormal(x)	FullTransactionIdFollowsOrEquals(x, FirstNormalFullTransactionId)
@@ -61,12 +52,127 @@
  * A 64 bit value that contains an epoch and a TransactionId.  This is
  * wrapped in a struct to prevent implicit conversion to/from TransactionId.
  * Not all values represent valid normal XIDs.
+ *
+ * 64bit xids that would map to a non-normal 32bit xid are not allowed, for
+ * obvious reasons.
+
+ * In addition, no xids with all the upper 32bit sets are allowed to exist,
+ * this is to make it easier to catch conversion errors from 32bit xids (which
+ * can point to "before" xid 0).
  */
 typedef struct FullTransactionId
 {
 	uint64		value;
 } FullTransactionId;
 
+#define MAX_FULL_TRANSACTION_ID ((((uint64) PG_UINT32_MAX) << 32) - 1)
+
+/*
+ * This is separate from FullTransactionIdValidRange() so that input routines
+ * can check for invalid values without triggering an assert.
+ */
+static inline bool
+InFullTransactionIdRange(uint64 val)
+{
+	/* 64bit xid above the upper bound */
+	if (val > MAX_FULL_TRANSACTION_ID)
+		return false;
+
+	/*
+	 * normal 64bit xids that'd map to special 32 xids aren't allowed.
+	 */
+	if (val >= (uint64) FirstNormalTransactionId &&
+		(uint32) val < FirstNormalTransactionId)
+		return false;
+	return true;
+}
+
+/*
+ * Validation routine, typically used in asserts.
+ */
+static inline bool
+FullTransactionIdValidRange(FullTransactionId x)
+{
+	return InFullTransactionIdRange(x.value);
+}
+
+static inline uint64
+U64FromFullTransactionId(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return x.value;
+}
+
+static inline TransactionId
+XidFromFullTransactionId(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return (TransactionId) x.value;
+}
+
+static inline uint32
+EpochFromFullTransactionId(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return (uint32) (x.value >> 32);
+}
+
+static inline bool
+FullTransactionIdEquals(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value == b.value;
+}
+
+static inline bool
+FullTransactionIdPrecedes(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value < b.value;
+}
+
+static inline bool
+FullTransactionIdPrecedesOrEquals(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value <= b.value;
+}
+
+static inline bool
+FullTransactionIdFollows(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value > b.value;
+}
+
+static inline bool
+FullTransactionIdFollowsOrEquals(FullTransactionId a, FullTransactionId b)
+{
+	Assert(FullTransactionIdValidRange(a));
+	Assert(FullTransactionIdValidRange(b));
+
+	return a.value >= b.value;
+}
+
+static inline bool
+FullTransactionIdIsValid(FullTransactionId x)
+{
+	Assert(FullTransactionIdValidRange(x));
+
+	return x.value != (uint64) InvalidTransactionId;
+}
+
 static inline FullTransactionId
 FullTransactionIdFromEpochAndXid(uint32 epoch, TransactionId xid)
 {
@@ -74,6 +180,8 @@ FullTransactionIdFromEpochAndXid(uint32 epoch, TransactionId xid)
 
 	result.value = ((uint64) epoch) << 32 | xid;
 
+	Assert(FullTransactionIdValidRange(result));
+
 	return result;
 }
 
@@ -84,6 +192,8 @@ FullTransactionIdFromU64(uint64 value)
 
 	result.value = value;
 
+	Assert(FullTransactionIdValidRange(result));
+
 	return result;
 }
 
@@ -102,6 +212,8 @@ FullTransactionIdFromU64(uint64 value)
 static inline void
 FullTransactionIdRetreat(FullTransactionId *dest)
 {
+	Assert(FullTransactionIdValidRange(*dest));
+
 	dest->value--;
 
 	/*
@@ -118,6 +230,8 @@ FullTransactionIdRetreat(FullTransactionId *dest)
 	 */
 	while (XidFromFullTransactionId(*dest) < FirstNormalTransactionId)
 		dest->value--;
+
+	Assert(FullTransactionIdValidRange(*dest));
 }
 
 /*
@@ -127,6 +241,8 @@ FullTransactionIdRetreat(FullTransactionId *dest)
 static inline void
 FullTransactionIdAdvance(FullTransactionId *dest)
 {
+	Assert(FullTransactionIdValidRange(*dest));
+
 	dest->value++;
 
 	/* see FullTransactionIdAdvance() */
@@ -135,6 +251,8 @@ FullTransactionIdAdvance(FullTransactionId *dest)
 
 	while (XidFromFullTransactionId(*dest) < FirstNormalTransactionId)
 		dest->value++;
+
+	Assert(FullTransactionIdValidRange(*dest));
 }
 
 /* back up a transaction ID variable, handling wraparound correctly */
diff --git a/src/backend/utils/adt/xid.c b/src/backend/utils/adt/xid.c
index 8ac1679c381..21c0f4c67df 100644
--- a/src/backend/utils/adt/xid.c
+++ b/src/backend/utils/adt/xid.c
@@ -188,6 +188,13 @@ xid8in(PG_FUNCTION_ARGS)
 	uint64		result;
 
 	result = uint64in_subr(str, NULL, "xid8", fcinfo->context);
+
+	if (!InFullTransactionIdRange(result))
+		ereturn(fcinfo->context, 0,
+				(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+				 errmsg("value \"%s\" is out of range for type %s",
+						str, "xid8")));
+
 	PG_RETURN_FULLTRANSACTIONID(FullTransactionIdFromU64(result));
 }
 
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index e616303a292..8b33d145a36 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -302,15 +302,18 @@ parse_snapshot(const char *str, Node *escontext)
 	const char *str_start = str;
 	char	   *endp;
 	StringInfo	buf;
+	uint64		raw_fxid;
 
-	xmin = FullTransactionIdFromU64(strtou64(str, &endp, 10));
-	if (*endp != ':')
+	raw_fxid = strtou64(str, &endp, 10);
+	if (*endp != ':' || !InFullTransactionIdRange(raw_fxid))
 		goto bad_format;
+	xmin = FullTransactionIdFromU64(raw_fxid);
 	str = endp + 1;
 
-	xmax = FullTransactionIdFromU64(strtou64(str, &endp, 10));
-	if (*endp != ':')
+	raw_fxid = strtou64(str, &endp, 10);
+	if (*endp != ':' || !InFullTransactionIdRange(raw_fxid))
 		goto bad_format;
+	xmax = FullTransactionIdFromU64(raw_fxid);
 	str = endp + 1;
 
 	/* it should look sane */
@@ -326,7 +329,11 @@ parse_snapshot(const char *str, Node *escontext)
 	while (*str != '\0')
 	{
 		/* read next value */
-		val = FullTransactionIdFromU64(strtou64(str, &endp, 10));
+		raw_fxid = strtou64(str, &endp, 10);
+
+		val = FullTransactionIdFromU64(raw_fxid);
+		if (!InFullTransactionIdRange(raw_fxid))
+			goto bad_format;
 		str = endp;
 
 		/* require the input to be in order */
diff --git a/src/test/regress/expected/xid.out b/src/test/regress/expected/xid.out
index 835077e9d57..9353a9f1c2d 100644
--- a/src/test/regress/expected/xid.out
+++ b/src/test/regress/expected/xid.out
@@ -6,11 +6,11 @@ select '010'::xid,
        '-1'::xid,
 	   '010'::xid8,
 	   '42'::xid8,
-	   '0xffffffffffffffff'::xid8,
-	   '-1'::xid8;
- xid | xid |    xid     |    xid     | xid8 | xid8 |         xid8         |         xid8         
------+-----+------------+------------+------+------+----------------------+----------------------
-   8 |  42 | 4294967295 | 4294967295 |    8 |   42 | 18446744073709551615 | 18446744073709551615
+	   '0xfffffffeffffffff'::xid8,
+	   '0'::xid8;
+ xid | xid |    xid     |    xid     | xid8 | xid8 |         xid8         | xid8 
+-----+-----+------------+------------+------+------+----------------------+------
+   8 |  42 | 4294967295 | 4294967295 |    8 |   42 | 18446744069414584319 |    0
 (1 row)
 
 -- garbage values
@@ -30,6 +30,18 @@ select 'asdf'::xid8;
 ERROR:  invalid input syntax for type xid8: "asdf"
 LINE 1: select 'asdf'::xid8;
                ^
+select '-1'::xid8;
+ERROR:  value "-1" is out of range for type xid8
+LINE 1: select '-1'::xid8;
+               ^
+select '0xffffffffffffffff'::xid8;
+ERROR:  value "0xffffffffffffffff" is out of range for type xid8
+LINE 1: select '0xffffffffffffffff'::xid8;
+               ^
+select '0x0000300000000001'::xid8;
+ERROR:  value "0x0000300000000001" is out of range for type xid8
+LINE 1: select '0x0000300000000001'::xid8;
+               ^
 -- Also try it with non-error-throwing API
 SELECT pg_input_is_valid('42', 'xid');
  pg_input_is_valid 
@@ -67,6 +79,24 @@ SELECT * FROM pg_input_error_info('0xffffffffffffffffffff', 'xid8');
  value "0xffffffffffffffffffff" is out of range for type xid8 |        |      | 22003
 (1 row)
 
+SELECT pg_input_is_valid('-1', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
+SELECT pg_input_is_valid('0xffffffffffffffff', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
+SELECT pg_input_is_valid('0x0000300000000001', 'xid8');
+ pg_input_is_valid 
+-------------------
+ f
+(1 row)
+
 -- equality
 select '1'::xid = '1'::xid;
  ?column? 
@@ -160,11 +190,11 @@ select xid8cmp('1', '2'), xid8cmp('2', '2'), xid8cmp('2', '1');
 
 -- min() and max() for xid8
 create table xid8_t1 (x xid8);
-insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xffffffffffffffff'), ('-1');
+insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xfffffffeffffffff');
 select min(x), max(x) from xid8_t1;
  min |         max          
 -----+----------------------
-   0 | 18446744073709551615
+   0 | 18446744069414584319
 (1 row)
 
 -- xid8 has btree and hash opclasses
diff --git a/src/test/regress/sql/xid.sql b/src/test/regress/sql/xid.sql
index 9f716b3653a..ed94c6d7d08 100644
--- a/src/test/regress/sql/xid.sql
+++ b/src/test/regress/sql/xid.sql
@@ -7,14 +7,17 @@ select '010'::xid,
        '-1'::xid,
 	   '010'::xid8,
 	   '42'::xid8,
-	   '0xffffffffffffffff'::xid8,
-	   '-1'::xid8;
+	   '0xfffffffeffffffff'::xid8,
+	   '0'::xid8;
 
 -- garbage values
 select ''::xid;
 select 'asdf'::xid;
 select ''::xid8;
 select 'asdf'::xid8;
+select '-1'::xid8;
+select '0xffffffffffffffff'::xid8;
+select '0x0000300000000001'::xid8;
 
 -- Also try it with non-error-throwing API
 SELECT pg_input_is_valid('42', 'xid');
@@ -23,6 +26,9 @@ SELECT * FROM pg_input_error_info('0xffffffffff', 'xid');
 SELECT pg_input_is_valid('42', 'xid8');
 SELECT pg_input_is_valid('asdf', 'xid8');
 SELECT * FROM pg_input_error_info('0xffffffffffffffffffff', 'xid8');
+SELECT pg_input_is_valid('-1', 'xid8');
+SELECT pg_input_is_valid('0xffffffffffffffff', 'xid8');
+SELECT pg_input_is_valid('0x0000300000000001', 'xid8');
 
 -- equality
 select '1'::xid = '1'::xid;
@@ -51,7 +57,7 @@ select xid8cmp('1', '2'), xid8cmp('2', '2'), xid8cmp('2', '1');
 
 -- min() and max() for xid8
 create table xid8_t1 (x xid8);
-insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xffffffffffffffff'), ('-1');
+insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xfffffffeffffffff');
 select min(x), max(x) from xid8_t1;
 
 -- xid8 has btree and hash opclasses
-- 
2.38.0

From a01e1481505e74097112c0ea358e7e0eef6a5684 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 11 Mar 2023 15:15:35 -0800
Subject: [PATCH v1] pg_amcheck: Minor test speedups

Discussion: https://postgr.es/m/20230309001558.b7shzvio645ebdta@awork3.anarazel.de
---
 src/bin/pg_amcheck/t/004_verify_heapam.pl | 33 +++++++++++------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/src/bin/pg_amcheck/t/004_verify_heapam.pl b/src/bin/pg_amcheck/t/004_verify_heapam.pl
index 9984d0d9f87..e5ae7e6aada 100644
--- a/src/bin/pg_amcheck/t/004_verify_heapam.pl
+++ b/src/bin/pg_amcheck/t/004_verify_heapam.pl
@@ -217,17 +217,17 @@ my $rel = $node->safe_psql('postgres',
 my $relpath = "$pgdata/$rel";
 
 # Insert data and freeze public.test
-use constant ROWCOUNT => 17;
+my $ROWCOUNT = 17;
 $node->safe_psql(
 	'postgres', qq(
 	INSERT INTO public.test (a, b, c)
-		VALUES (
+		SELECT
 			x'DEADF9F9DEADF9F9'::bigint,
 			'abcdefg',
 			repeat('w', 10000)
-		);
-	VACUUM FREEZE public.test
-	)) for (1 .. ROWCOUNT);
+        FROM generate_series(1, $ROWCOUNT);
+	VACUUM FREEZE public.test;)
+);
 
 my $relfrozenxid = $node->safe_psql('postgres',
 	q(select relfrozenxid from pg_class where relname = 'test'));
@@ -246,16 +246,13 @@ if ($datfrozenxid <= 3 || $datfrozenxid >= $relfrozenxid)
 }
 
 # Find where each of the tuples is located on the page.
-my @lp_off;
-for my $tup (0 .. ROWCOUNT - 1)
-{
-	push(
-		@lp_off,
-		$node->safe_psql(
-			'postgres', qq(
-select lp_off from heap_page_items(get_raw_page('test', 'main', 0))
-	offset $tup limit 1)));
-}
+my @lp_off = split '\n', $node->safe_psql(
+	'postgres', qq(
+	    select lp_off from heap_page_items(get_raw_page('test', 'main', 0))
+		where lp <= $ROWCOUNT
+    )
+);
+is(scalar @lp_off, $ROWCOUNT, "acquired row offsets");
 
 # Sanity check that our 'test' table on disk layout matches expectations.  If
 # this is not so, we will have to skip the test until somebody updates the test
@@ -267,7 +264,7 @@ open($file, '+<', $relpath)
 binmode $file;
 
 my $ENDIANNESS;
-for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++)
+for (my $tupidx = 0; $tupidx < $ROWCOUNT; $tupidx++)
 {
 	my $offnum = $tupidx + 1;        # offnum is 1-based, not zero-based
 	my $offset = $lp_off[$tupidx];
@@ -345,7 +342,7 @@ open($file, '+<', $relpath)
   or BAIL_OUT("open failed: $!");
 binmode $file;
 
-for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++)
+for (my $tupidx = 0; $tupidx < $ROWCOUNT; $tupidx++)
 {
 	my $offnum = $tupidx + 1;        # offnum is 1-based, not zero-based
 	my $offset = $lp_off[$tupidx];
@@ -522,7 +519,7 @@ for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++)
 		$tup->{t_infomask} &= ~HEAP_XMIN_INVALID;
 
 		push @expected,
-          qr/${$header}xmin ${xmin} equals or exceeds next valid transaction ID 0:\d+/;
+		  qr/${$header}xmin ${xmin} equals or exceeds next valid transaction ID 0:\d+/;
 	}
 	write_tuple($file, $offset, $tup);
 }
-- 
2.38.0