Hi Antonin,
On Tue, Nov 10, 2026 at 11:54 AM Antonin Houska <[email protected]> wrote:
> For now I commented the assertion out and added a comment on possible
> solutions.
Leaving it out looks safe to me, and the check can stay. Every other
caller passes GetCurrentTransactionId(), and in all the runs I made
(UPDATE, DELETE, savepoints, TOAST, row lockers, 150 replayed calls)
the replay reached the function with xmax still zero: the copy clears
it and only REPACK writes to the new heap. So the attached assertion
keeps the old check and states what REPACK relies on; it passes
Shihao's spec and the existing REPACK tests. With assertions off, a
REPEATABLE READ snapshot taken while REPACK waited for the lock saw the
same rows as an untouched copy of the table in every one of those runs.
A correction: in September I blamed the transient heap's XID, but that
run had failed before the final lock. Shihao has it right.
Regards,
Manu
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 19be1a3995b..9086b79bc0c 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -5578,12 +5578,14 @@ compute_new_xmax_infomask(TransactionId xmax, uint16
old_infomask,
new_infomask2;
/*
- * TODO Move this statement to callers? REPACK (CONCURRENTLY) uses
- * add_to_xmax different from the current transaction when replaying
data
- * changes. Alternatively, we can add a boolean argument to the function
- * indicating the special case, but that does not look nice.
- */
- //Assert(TransactionIdIsCurrentTransactionId(add_to_xmax));
+ * REPACK (CONCURRENTLY) replays data changes with the XID of the
+ * transaction that made them (TABLE_REUSE_XID), so add_to_xmax need not
+ * be ours. It only replays them on tuples of the new heap that nobody
+ * else can lock or modify and whose xmax it has not set, so xmax is
+ * still invalid here and our own transaction does not matter below.
+ */
+ Assert(TransactionIdIsCurrentTransactionId(add_to_xmax) ||
+ !TransactionIdIsValid(xmax));
l5:
new_infomask = 0;
