Re: Why is it not sane to pass ExecStoreTuple(shouldFree=true) for tuples point into buffers

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Why is it not sane to pass ExecStoreTuple(shouldFree=true) for tuples point into buffers
Date: 2014-04-07 20:38:50
Message-ID: 20140407203850.GO4161@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2014-04-07 16:29:57 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-04-07 15:58:31 -0400, Tom Lane wrote:
> >> There's an assumption that if you are asking to pin a buffer, the tuple
> >> pointer you're passing is pointing into that buffer (and is therefore not
> >> something that could be pfree'd). If it isn't pointing into a buffer,
> >> the tuple slot is not the place to be keeping the buffer reference.
>
> > Yea. I *have* a HeapTupleData struct pointing into the buffer. It's just
> > that the lifetime of the indexscans's xs_ctup isn't sufficient for my
> > case. So I have to allocate a new HeapTupleData struct, *again* pointing
> > into the buffer. I'd like to manage the lifetime of that HeapTupleData
> > struct (*not* the entire HeapTupleHeader blob) via the slot.
> > That doesn't sound too crazy to me?
>
> In that case you should have another tuple slot of your own and let it
> keep the tuple (and buffer pin).

Maybe I am just being dense here and misunderstanding tuple slots. The
executor isn't exactly my strong suit.

But say I am doing something like:

slot = ExecInitExtraTupleSlot(estate);
ExecSetSlotDescriptor(slot, RelationGetDescr(rel));
...
while ((scantuple = index_getnext(scan, ForwardScanDirection)) != NULL)
{
/* some check */
...
if (blurb)
ExecStoreTuple(scantuple, slot, scan->xs_cbuf, true);
}

...
index_endscan(scan); /* possibly */

/* now use the stored tuple via slot->tts_tuple */

that's not going to work, because scantuple might be free'd or pointing
to another tuple, from the next index_getnext() call. Right?
So what I now do is essentially:
while ((scantuple = index_getnext(scan, ForwardScanDirection)) != NULL)
{
...
ht = palloc(sizeof(HeapTupleData)); /* in the right context */
memcpy(ht, scantuple, sizeof(HeapTupleData));
ExecStoreTuple(ht, slot, scan->xs_cbuf, false);
slot->tts_shouldFree = true;
...
}

That will a) keep the buffer pinned long enough, b) keep the
HeapTupleData struct around long enough.

That works, but seems pretty ugly. Thus I am wondering if a) there's a
way to avoid the outside manipulation of tts_shouldFree b) why there's
no builtin operation doing the palloc, memcpy and store...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2014-04-07 22:49:34 Re: B-Tree support function number 3 (strxfrm() optimization)
Previous Message Peter Geoghegan 2014-04-07 20:35:08 Re: B-Tree support function number 3 (strxfrm() optimization)