Re: Avoid using raw xmin of frozen tuples in logicalrep conflict detection

Hi Andres,

On Wed, Nov 4, 2026 at 09:33 PM Andres Freund wrote:
> On 2026-11-04 21:04:42 +0800, Zhijie Hou wrote:
>> The reason is that, GetTupleTransactionInfo() used slot_getsysattr() with
>> MinTransactionIdAttributeNumber to fetch the local tuple's xmin.
>> However, heap_getsysattr() calls HeapTupleHeaderGetRawXmin(), which
>> returns the raw 32-bit t_xmin from the tuple header even when the tuple
>> is marked frozen (HEAP_XMIN_FROZEN).
>>
>> Once a tuple is frozen, its raw t_xmin is retained only for debugging
>> purposes; its commit timestamp entry may no longer be valid, or the
>> 32-bit XID may have wrapped around and been reassigned in a later epoch.
>> Using the raw xmin of a frozen tuple could therefore trigger spurious
>> update_origin_differs or delete_origin_differs conflicts, or report
>> bogus transaction ID, origin, and commit timestamp details for unique
>> constraint conflicts.
>>
>> Attaching a patch fixing this by fetching the tuple via
>> ExecFetchSlotHeapTuple() and calling HeapTupleHeaderGetXmin(), returning
>> InvalidTransactionId and skipping the commit timestamp lookup when the tuple's
>> xmin is not a normal transaction ID. Also update errdetail_apply_conflict() to
>> omit the transaction ID from unique-conflict detail messages when localxmin is
>> invalid.
>
> That surely is not the right fix, it breaks any sense of abstraction across
> table AMs. There's afaict no guarantee that the target table is heapam. And
> even if there currently is, we shouldn't bake that in further.

Thanks for pointing it out. I agree that conflict.c shouldn't be
reaching into BufferHeapTupleTableSlot or HeapTupleHeaderGetXmin() directly.

To avoid that, on master, it seems cleanest to handle this at the
TupleTableSlotOps level (alongside getsysattr and is_current_xact_tuple
in execTuples.c), for example, via an optional callback that returns the
effective xmin (HeapTupleHeaderGetXmin() for heap/buffer-heap slots, and
InvalidTransactionId when NULL for virtual/minimal slots or AMs that
don't store tuple xmin).

TransactionId (*get_effective_xmin) (TupleTableSlot *slot);

I also considered adding a callback to TableAmRoutine instead, but
wasn't sure if it's worth adding a TableAmRoutine entry when the tuple
is already in the slot and getsysattr / is_current_xact_tuple are on
TupleTableSlotOps.

For back-patching to 18/19 where we can't change TupleTableSlotOps
without breaking ABI, GetTupleTransactionInfo() can branch on
TTS_IS_BUFFERTUPLE(localslot) || TTS_IS_HEAPTUPLE(localslot) to use
HeapTupleHeaderGetXmin() and fall back to slot_getsysattr() otherwise,
while still filtering out !TransactionIdIsNormal(xmin).

What do you think ?

--
Best Regards,
Zhijie Hou