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

On Tue, Oct 20, 2026 at 09:20:18PM -0400, shihao zhong wrote:
> Agreed, and thanks for pointing at both places. v2 does just that and
> leaves rewriteheap.c alone.
>
> With the hint set, the old version no longer waits in unresolved_tups,
> so the logical mapping change from v1 is not needed anymore.

I can see that you are testing both code paths, at least. The
coverage part looks good, but I'd suggest to make the test more
readable at least especially in the s1_alter and s1_plain that include
a bunch of queries. There two are hard to parse.

+ if (!TransactionIdIsInProgress(xmax) &&
+ !TransactionIdDidCommit(xmax))
+ SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
+ InvalidTransactionId)

It's also worth noting that these may make the code less efficient,
with more SLRU traffic. What could be the class of worst case
possible? A combination of an in-progress transaction with some
VACUUM activity while the in-progress transaction is still opened?
VACUUM would need to do more work with the patch in place, where I
guess it could be slower if we have a bunch of tuples that need to
have this hint bit set.

Or perhaps this is too narrow to worry about? Anyway, we should be
super careful with such changes. That can bite hard.

If others have any ideas and/or comments, please feel free..

Thanks,
--
Michael