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

Hi,

On Fri, Nov 6, 2026 at 12:13 PM Zhijie Hou wrote:
> Hi Andres,
>
> On Wed, Nov 4, 2026 at 09:33 PM Andres Freund wrote:
> > 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);

Following this, I am attaching two patches implementing the approaches
discussed above for reference:

v2-0001-Fix-logical-xx.patch adds an optional get_effective_xmin
callback to TupleTableSlotOps (alongside getsysattr and
is_current_xact_tuple). Heap and buffer-heap slots implement it via
HeapTupleHeaderGetXmin(), while slots that do not provide this
information leave the callback unset so slot_get_effective_xmin()
returns InvalidTransactionId.

nocfbot-v2-0001-Fix-logical-xx.patch avoids adding a new
TupleTableSlotOps callback (which is also needed for back-patching to
18) by checking TTS_IS_BUFFERTUPLE(localslot) ||
TTS_IS_HEAPTUPLE(localslot) in GetTupleTransactionInfo() to use
HeapTupleHeaderGetXmin(), and falling back to slot_getsysattr() for
other slot types.

What do you think ?

--
Best Regards,
Zhijie Hou