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

On 10/13/2016 03:02 AM, Peter Geoghegan wrote:
> On Wed, Oct 12, 2016 at 4:36 PM, Peter Geoghegan <pg@heroku.com> wrote:
>> 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.
>
> Actually, it's only true that tuplesort sortcontext context is used
> when copy isn't needed, which is not predictable to caller, so the new
> comment is a bit inaccurate. The inconsistency seems inconsequential,
> since we've always assume that caller tuples allocated within
> sortcontext may be "owned" by caller (when should_free = true),
> despite not being in caller's own memory context.
>
> Attached is revision with tiny tweak to relevant comment.

Hmm. That also adds a copy to the sorted-in-mem case. That's safe, but
should we be worried about performance. Or is the extra copy so cheap
that it doesn't matter?

We could easily also check for TSS_SORTEDINMEM, if that matters.

- Heikki