Re: BUG #19760: CLUSTER loses a row the transaction inserted and then updated in a rolled-back subtransaction

On Mon, Oct 26, 2026 at 12:41:19PM -0300, Manu wrote:
> Thanks for taking care of it. The attached series adds the second
> spot on top of e7f4463e869, as shihao asked upthread.
>
> + /*
> + * If the updater of the tuple has already aborted (a rolled-back
> + * subtransaction of the inserter), remember that, as callers like
> + * rewriteheap.c rely on HEAP_XMAX_INVALID to tell a live tuple
> + * from an updated one.
> + */
> + if (!(tuple->t_infomask & HEAP_XMAX_INVALID) &&
> + !(tuple->t_infomask & HEAP_XMAX_IS_MULTI) &&
> + !HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask) &&
> + !HeapTupleHeaderIsOnlyLocked(tuple))
> + {
> + TransactionId xmax = HeapTupleHeaderGetUpdateXid(tuple);
> +
> + /*
> + * xmax == xmin: xmax is in progress too, nothing to do.
> + * The clog is only a cheap filter here: a running xmax is not
> + * aborted there. An abort reaches the clog before the xid
> + * leaves the proc array, and the new version of the tuple,
> + * whose xmin is this xmax, is in progress until then. So the
> + * proc array must confirm it, as the note at the top of this
> + * file asks.
> + */
> + if (!TransactionIdEquals(xmax, HeapTupleHeaderGetRawXmin(tuple)) &&
> + TransactionIdDidAbort(xmax) &&
> + !TransactionIdIsInProgress(xmax))
> + SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
> + InvalidTransactionId);
> + }

FWIW, this does not feel right. Your patch is a gathering of
everybody's suggestions that have been fed to an LLM, without thinking
through the problem, resulting of a pile of changes which was itself a
pile of changes. I'd suggest to take three deep breaths and not rely
on an LLM to try to think through all the potential holes that any of
the previous suggestions have. Piling workarounds on top of
workarounds is no good for these code paths. They just become more
complicated to think about long-term.
--
Michael