Re: PATCH: decreasing memory needlessly consumed by array_agg

Tomas Vondra <tv(at)fuzzy(dot)cz> writes:
> On 1.4.2014 19:08, Tom Lane wrote:
>> Actually, though, the patch as given outright breaks things for both
>> the grouped and ungrouped cases, because the aggregate no longer
>> releases memory when it's done. That's going to result in memory
>> bloat not savings, in any situation where the aggregate is executed
>> repeatedly.

> Looking at array_agg_finalfn (which is the final function for
> array_agg), I see it does this:

> /*
> * Make the result. We cannot release the ArrayBuildState because
> * sometimes aggregate final functions are re-executed. Rather, it
> * is nodeAgg.c's responsibility to reset the aggcontext when it's
> * safe to do so.
> */
> result = makeMdArrayResult(state, 1, dims, lbs,
> CurrentMemoryContext,
> false);

> i.e. it sets release=false. So I fail to see how the current code
> behaves differently from the patch?

You're conveniently ignoring the callers that set release=true.
Reverse engineering a query that exhibits memory bloat is left
as an exercise for the reader (but in a quick look, I'll bet
ARRAY_SUBLINK subplans are one locus for problems).

It's possible that it'd work to use a subcontext only if release=true;
I've not dug through the code enough to convince myself of that.

regards, tom lane