Re: PATCH: decreasing memory needlessly consumed by array_agg

Hi,

On 21.1.2015 09:01, Jeff Davis wrote:
> On Tue, 2015-01-20 at 23:37 +0100, Tomas Vondra wrote:
>> Tom's message where he points that out is here:
>> http://www.postgresql.org/message-id/20707.1396372100@sss.pgh.pa.us
>
> That message also says:
>
> "I think a patch that stood a chance of getting committed would need
> to detect whether the aggregate was being called in simple or
> grouped contexts, and apply different behaviors in the two cases."
>
> I take that as an objection to any patch which does not distinguish
> between the grouped and ungrouped aggregate cases, which includes
> your patch.

I don't think 'simple context' in this case means 'aggregate without a
group by clause'.

The way I understood it back in April 2014 was that while the patch
worked fine with grouped cases (i.e. in nodeAgg.c or such), the
underlying function are used elsewhere in a simple context (e.g. in an
xpath() or so) - and in this case it was broken. And that was a correct
point, and was fixed by the recent patches.

But maybe I'm missing something?

> I don't agree with that objection (or perhaps I don't understand
> it); but given the strong words above, I need to get some kind of
> response before I can consider committing your patch.
>
>> I generally agree that having two API 'facets' with different behavior
>> is slightly awkward and assymetric, but I wouldn't call that ugly.
>
> Right, your words are more precise (and polite). My apologies.

Ummmm ... I wasn't suggesting calling the resulting API 'ugly' is
impolite or so. It was meant just as a comment that the aesthetics of
the API is quite subjective matter. No apology needed.

>> I actually modified both APIs initially, but I think Ali is right
>> that not breaking the existing API (and keeping the original
>> behavior in that case) is better. We can break it any time we want
>> in the future, but it's impossible to "unbreak it" ;-)
>
> We can't break the old API, and I'm not suggesting that we do. I was
> hoping to find some alternative.

Why can't we? I'm not saying we should in this cases, but there are
cases when breaking the API is the right thing to do (e.g. when the
behavior changes radically, and needs to be noticed by the users).

--
Tomas Vondra http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services