On Wed, Oct 12, 2016 at 2:20 PM, Peter Geoghegan <pg@heroku.com> wrote:
>> That's unfortunate. AFAICS, we have no choice but palloc(), when
>> tuplesort_gettupleslot() is used. For version 10, maybe we should reconsider
>> that API.
>
> We have to do something to fix this bug, and performing an extra
> retail palloc() looks like the easiest solution. This does not
> actually defeat the purpose of batch memory, which was mostly about
> cache efficiency. I tend to think that the possible uses of the slot
> by tuplesort_gettupleslot() callers are too complex to tie down.
Attached is a fix inspired by the similar routine in tuplestore.c:
tuplestore_gettupleslot(). This bug seems totally analogous to the one
fixed by 25bf7f8b -- history repeats itself.
This fix has us copy the MinimalTuple into sortcontext palloc() memory
within tuplesort_gettupleslot() (based on commit 25bf7f8b). This still
differs a little from tuplestore_gettupleslot(), which explicitly uses
current context of caller, but we've always done things that way for
tuplesort.c.
It wouldn't be very hard to also add a tuplestore_gettupleslot()-style
"copy" boolean argument to tuplesort_gettupleslot(), but I haven't
done that here. That way, callers that are happy to rely on the
current behavior (the sometimes dangerous behavior) could do so, and
so could still benefit from avoiding the new heap_copy_minimal_tuple()
call (doing so isn't broken for some callers -- those that are happy
to not reuse contents of tuple slot across calls). For now, I haven't
added that. I guess that's Postgres 10 work, as Heikki mentioned.
Apologies for the delay on this.
--
Peter Geoghegan
