From: Tom Lane <tgl@sss.pgh.pa.us>
To: Scott Carey <scott.carey@algonomy.com>
cc: Jeff Davis <pgsql@j-davis.com>, David Rowley <dgrowleyml@gmail.com>,
        pgsql-performance@lists.postgresql.org
Subject: Re: Significant performance issues with array_agg() + HashAggregate
 plans on Postgres 17
In-reply-to: <3671092.1775167393@sss.pgh.pa.us>
References: 
 <CA+vubOGaanYaGGb98etJPeJCbN=RAawx652MdmJVRK8+BeEP0w@mail.gmail.com>
 <CAApHDvq5fMzhK7OwGOhOgqRF-wErGNsT75yie5LtobSir1HhnA@mail.gmail.com>
 <CA+vubOE6feEWh7XxoWqdPh634SUVjHKW7s9BgMD755wJTqQFPw@mail.gmail.com>
 <CA+vubOHgrdkm+3wgC0FPcStDeMS9ZQ_Du9vUXWjxN9MN9CO2uw@mail.gmail.com>
 <2764544.1774985169@sss.pgh.pa.us>
 <CA+vubOEbfm5+BxtuFGb_tTj3sSX1u0Y+=zHUxoF1Q9feGpRfNg@mail.gmail.com>
 <2897039.1775051081@sss.pgh.pa.us>
 <CA+vubOGqg7f2E0_OW8EMruAObejnCS1gCgguT+cnwHG9bRxMDQ@mail.gmail.com>
 <3579239.1775151498@sss.pgh.pa.us>
 <CA+vubOH1mcEbsHnkm5L7aJWR+eeiVpOnGfcdWax9td6pyOkMww@mail.gmail.com>
 <3671092.1775167393@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Thu, 02 Apr 2026 18:03:13 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3825074.1775244255.1@sss.pgh.pa.us>
Date: Fri, 03 Apr 2026 15:24:15 -0400
Message-ID: <3825075.1775244255@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3825075.1775244255%40sss.pgh.pa.us>
Precedence: bulk

I wrote:
> So the main problem here is we're leaking the arrays made by
> array_agg, and a secondary problem is that that drives the
> cost of hash_agg_check_limits to an unacceptable level.

After further study I no longer think there's a leak.  This
test query involves 50000 GROUP BY groups, and we have an
array being accumulated for each one.  The difference between
the fast array_agg implementation and the slow one is just
that the fast one keeps all its working state in the aggregate
context, while the slow one makes a separate sub-context for
each "expanded array".  v12 creates 50000 "expanded arrays"
too, but it's not noticeably slow.

So the problem is exactly that repeating hash_agg_check_limits
each time we start a new group is O(N^2), because if there is
a sub-context for each group then MemoryContextMemAllocated
requires O(N) time.

The other little problem with this approach is acknowledged in the
comments:

 * ... Allocations may happen without adding new groups (for instance,
 * if the transition state size grows), so this check is imperfect.

So really the whole thing is kind of unsatisfactory.
I'm not seeing cheap ways to make it better though.

A very quick and dirty idea is to tell MemoryContextMemAllocated
not to recurse, which would restore it to constant-time.  But
that would make the results much less accurate in cases like
this one.

			regards, tom lane