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

On Tue, Oct 20, 2026 at 04:41:49PM +0900, Michael Paquier wrote:
> Having a recheck of the old tuple's state within
> rewrite_heap_dead_tuple() feels very incorrect to me, based on the
> bunch of comments at the top of rewriteheap.c. It sounds to me that
> rewrite_heap_dead_tuple() should be called when we know that the tuple
> is dead, no? Isn't heapam_relation_copy_for_cluster() the bit actually
> incorrect here, marking the tuple with "isdead" to start with? Your
> solution of a recheck to make sure that an old tuple is actually dead
> in a code path where an old tuple is assumed to be absolutely and
> definitely dead looks like an API contract violation regarding
> rewriteheap.c.

Looking a bit more at that before the end of the day, I think that the
suggestion of touching HeapTupleSatisfiesVacuumHorizon() and setting
some hint bits would be the correct thing to do. One case that matters
is the "deleting subtransaction must have aborted" case. I am pretty
sure that we should also worry about the case where we check for
TransactionIdIsInProgress(HeapTupleHeaderGetRawXmin(tuple)), which
looks reachable, and set an extra hint bit there..
--
Michael