Re: BUG #14344: string_agg(DISTINCT ..) crash

On 09/29/2016 07:04 PM, Peter Geoghegan wrote:
> On Thu, Sep 29, 2016 at 4:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> My immediate reaction to this is WTF.  It seems like you have completely
>> broken the expected contract of tuplesort_gettupleslot, which is that
>> it copies the data into caller-owned memory.  That cannot stand.
>
> Other comparable routines, like tuplesort_getindextuple(), could be
> argued to have always had a contract that allows for this kind of
> thing (recycling batch memory) because they get to examine a
> *should_free pointer -- do you accept that much?

The comments in those other routines explicitly say that "If it is not
set [*should_free], caller should not use tuple following next call
here". I also didn't notice that tuplesort_gettupleslot doesn't contain
say that, until now.

That's unfortunate. AFAICS, we have no choice but palloc(), when
tuplesort_gettupleslot() is used. For version 10, maybe we should
reconsider that API.

- Heikki