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

On 10/13/2016 08:27 PM, Peter Geoghegan wrote:
> On Thu, Oct 13, 2016 at 12:59 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> 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?
>
> I think that simply reading random locations in memory is the dominant
> cost, but the exact overhead should be investigated before proceeding.

Ok. In quick testing, the extra palloc() indeed didn't seem to cost much.

> The point I'm making is that we might be better off worrying about the
> general problem, by adding a tuplestore_gettupleslot()-style "copy"
> boolean argument at the same time, and having some callers pass
> "false" to avoid copying (when they determine no risk of
> use-after-free, by not keeping the contents of a slot active across
> calls to tuplesort_gettupleslot()). You indicated that you don't
> really want to go there for 9.6, but maybe it's worth reconsidering
> that. For example, maybe ABI breakage is avoided by making
> tuplesort_gettupleslot() into a shim. Or, maybe it's okay to put it in
> the release notes of 9.6.1 -- I'm not sure how manageable that is.

Pushed this isolated fix for now.

- Heikki