Re: Should setting TupleTableSlotOps get_heap_tuple=NULL break INSERT..ON CONFLICT DO UPDATE..RETURNING? - Mailing list pgsql-hackers

From Mark Dilger
Subject Re: Should setting TupleTableSlotOps get_heap_tuple=NULL break INSERT..ON CONFLICT DO UPDATE..RETURNING?
Date
Msg-id 549147E3-F866-4CC0-9821-8CA1706AF90A@enterprisedb.com
Whole thread Raw
In response to Re: Should setting TupleTableSlotOps get_heap_tuple=NULL break INSERT..ON CONFLICT DO UPDATE..RETURNING?  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers

> On Sep 29, 2022, at 9:22 AM, Andres Freund <andres@anarazel.de> wrote:
>
> I would assume that this can be avoided by the tuple slot implementation, but
> without seeing what precisely you did in your pile slot...

"pile" is just a copy of "heap" placed into an extension with a slightly smarter version of s/heap/pile/g performed
acrossthe sources.  It is intended to behave exactly as heap does.  Without disabling the get_heap_tuple function, it
passesa wide variety of the regression/isolation/tap tests.  To test the claim made in the TupleTableSlotOps code
comments,I disabled that one function: 

    /*
     * Return a heap tuple "owned" by the slot. It is slot's responsibility to
     * free the memory consumed by the heap tuple. If the slot can not "own" a
     * heap tuple, it should not implement this callback and should set it as
     * NULL.
     */
    HeapTuple   (*get_heap_tuple) (TupleTableSlot *slot);

That comment suggests that I do not need to keep a copy of the heap tuple, and per the next comment:

    /*
     * Return a copy of heap tuple representing the contents of the slot. The
     * copy needs to be palloc'd in the current memory context. The slot
     * itself is expected to remain unaffected. It is *not* expected to have
     * meaningful "system columns" in the copy. The copy is not be "owned" by
     * the slot i.e. the caller has to take responsibility to free memory
     * consumed by the slot.
     */
    HeapTuple   (*copy_heap_tuple) (TupleTableSlot *slot);

I do not need to keep a copy of the "system columns".  But clearly this doesn't work.  When get_heap_tuple=NULL, the
AM'stuple_update is at liberty to free the update tuple (per the above documentation) and later return a copy of the
slot'stuple sans any "system columns" (also per the above documentation) and that's when the core code breaks.  It's
notthe TAM that is broken here, not according to the interface's documentation as I read it.  Am I reading it wrong? 



—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company






pgsql-hackers by date:

Previous
From: Matthias van de Meent
Date:
Subject: Re: problems with making relfilenodes 56-bits
Next
From: Bharath Rupireddy
Date:
Subject: Re: Avoid memory leaks during base backups