Subject: Add enable_groupagg GUC parameter to control GroupAggregate usage

From: Tatsuro Yamada <[email-masked]>
Date: Fri, 6 Jun 2025 16:59:09 +0900

Hi hackers,

When I measured the execution time of a certain query with parallel query
enabled and disabled, I found that the execution time was slower when
parallel query was enabled.

To improve the performance of the parallel query, I considered adjusting
the execution plan and attempted to switch from GroupAggregate to
HashAggregate. However, I noticed that there was no GUC parameter to
disable GroupAggregate.

Therefore, I propose adding a new GUC parameter: enable_groupagg.

Below are the results of a performance test where I disabled
GroupAggregate using enable_groupagg. In this case, the planner chose
HashAggregate instead, which improved performance by about 35 times.

# Query Execution Results (Average of 3 measurements)
- With parallel query:                                 39546 seconds
- With parallel query and enable_groupagg turned off:   1115 seconds

# Query and Data Used (attached to this email)
- Query: test_query.sql
- Data:  create_table.sql

# The steps to run the test are as follows.
For example, on psql:

1. Create tables:
    \i create_table.sql

2. Execute a query:
    \i test_query.sql

3. Execute a query using the new GUC parameter:
    set enable_groupagg to off;
    \i test_query.sql

As a benefit to users, while there has previously been a GUC parameter
to control HashAggregate, there was no corresponding way to control
GroupAggregate. This patch addresses that, giving users more flexibility
in tuning execution plans.

I've attached a WIP patch that adds this GUC parameter. I would
appreciate any feedback, especially regarding how many test cases I
should create.

To create new test cases for enable_groupagg, I looked into existing
test cases that use enable_hashagg and found that it is used in many
places (62 places). Should I add a test case for enable_groupagg in
the same place as enable_hashagg? I think that adding a new feature
requires a minimum number of test cases, so I would appreciate your
advice.

Additionally, based on the execution plan, I suspect the slowdown in the
parallel query might be caused by misestimates related to Sort or
Gather Merge.
While resolving those misestimates would ideally improve the root issue,
I'd like to keep the focus of this thread on adding the GUC parameter.
Then, I plan to report or address the estimation problem in a separate
thread.

Thanks,
Tatsuro Yamada

---

From: Tatsuro Yamada <[email-masked]>
Date: Fri, 6 Jun 2025 19:03:27 +0900

Hi,

> - With parallel query and enable_groupagg turned off:   1115 seconds

Oops, I made a mistake. The correct execution time is:
- With parallel query:                                 39546 ms
- With parallel query and enable_groupagg turned off:   1115 ms

Regards,
Tatsuro Yamada

---

From: Ashutosh Bapat <[email-masked]>
Date: Mon, 9 Jun 2025 16:20:39 +0530

> appreciate any feedback, especially regarding how many test cases I

I first thought enable_hashagg should be sufficient to choose one strategy
over the other. But that is not true, enable_hashagg = true allows both the
strategies, enable_hashagg = false disables just hash strategy. There's no
way to disable group agg alone. So I think it makes sense to have this GUC.

I am surprised that we didn't see this being a problem for so long.

We seem to disable mixed strategy when enable_hashagg is false. Do we want
to do the same when enable_groupagg = false as well?

> requires a minimum number of test cases, so I would appreciate your

Some of those instances are for plan stability, all of which need not be
replicated. But some of them explicitly test sort based grouping. For rest
of them hash based plan seems to be the best one, so explicit
enable_groupagg = false is not needed. We will need some test to test the
switch though.

> Then, I plan to report or address the estimation problem in a separate
+1.

-- 
Best Wishes,
Ashutosh Bapat

---

From: Tatsuro Yamada <[email-masked]>
Date: Wed, 11 Jun 2025 12:07:44 +0900

Hi Ashutosh,

Thanks for your reply!

Yes, exactly! Thank you for the clarification.

>I am surprised that we didn't see this being a problem for so long.

I was also wondering why this GUC parameter hadn't been introduced
earlier.

As I mentioned in my previous email, setting enable_groupagg = false
resulted in better execution time compared to the mixed strategy (which,
as I understand, includes both HashAgg and GroupAgg). So, I believe
this new GUC parameter would be helpful for users in certain situations.

Here’s how the current and proposed behaviors compare:

Current behavior:
    enable_hashagg = ON  → Mixed
    enable_hashagg = OFF → GroupAgg

After applying the patch:
    enable_hashagg = ON,  enable_groupagg = ON  → Mixed
    enable_hashagg = OFF, enable_groupagg = ON  → GroupAgg
    enable_hashagg = ON,  enable_groupagg = OFF → HashAgg (new)
    enable_hashagg = OFF, enable_groupagg = OFF → GroupAgg

In addition, if the both parameters = OFF, GroupAgg will be selected in
the patch. The reason is that I found a comment that HashAgg might use
too much memory, so I decided to prioritize using GroupAgg, which is
more secure.

In the last case, I chose to default to GroupAgg since I found a
comment suggesting HashAgg might consume excessive memory, so GroupAgg
seemed the safer fallback.

Some hackers may want to compare the actual execution plans and times
under different GUC settings, so I've attached my test results in
"test_result.txt".

Thanks for your advice. I'll create a regression test and send a new patch
to -hackers in my next email.

Regards,
Tatsuro Yamada

---

From: Tatsuro Yamada <[email-masked]>
Date: Wed, 11 Jun 2025 17:36:58 +0900

Hi Ashtosh and hackers,

> to -hackers in my next email.

I created a regression test to check the enable_groupagg parameter in
the new patch.
To ensure plan stability, I disabled parallel query by setting the
max_parallel_*
parameters to 0.

Any feedback is welcome.
Please see the attached file.

Thanks,
Tatsuro Yamada

---

From: David Rowley <[email-masked]>
Date: Mon, 16 Jun 2025 14:49:57 +1200

On Wed, 11 Jun 2025 at 20:37, Tatsuro Yamada <[email-masked]> wrote:
> To ensure plan stability, I disabled parallel query by setting the max_parallel_*
> parameters to 0.
>
> Any feedback is welcome.

Typically, in the regression tests we've used enable_sort to force a
HashAgg. There are certainly times when that's not good enough and you
might also need to disabe enable_indexscan too, so I understand the
desire to add this GUC.

It's probably going to be worth going over the regression tests to
find where we use enable_sort to disable GroupAgg and replace those
with your new GUC. Otherwise, people looking at those tests in the
future will be a bit confused as to why the test didn't just SET
enable_groupagg TO false;  These will likely be good enough to serve
as your test, rather than creating a new table to test this feature.

I think you should also look at create_setop_path(), as I imagine that
the same arguments for using enable_hashagg in that function apply
equally to enable_groupagg.

+ if (aggstrategy == AGG_SORTED && !enable_groupagg && enable_hashagg)
+ ++disabled_nodes;

This code looks a bit strange. You're only going to disable it if hash
agg is enabled? If they're both disabled, let add_path() decide.
Anyone who complains that they didn't get the aggregate type they
wanted with both enable_hashagg and enable_groupagg set to off hasn't
got a leg to stand on.

David

---

From: Tatsuro Yamada <[email-masked]>
Date: Fri, 20 Jun 2025 19:34:40 +0900

Hi David,

desire to add this GUC.

Thank you for the explanation.
I wasn't aware that enable_sort could be used to switch from GroupAgg
to HashAgg.

---

From: Richard Guo <[email-masked]>
Date: Thu, 25 Jun 2026 08:05:28 +0900

On Fri, Jun 20, 2025 at 7:35 PM Tatsuro Yamada <[email-masked]> wrote:
> From a user's perspective, I think many would expect that if
> enable_hashagg exists, then enable_groupagg would as well. Adding
> such a GUC parameter seems intuitive and reasonable.So, I believe
> there's value in introducing the parameter I proposed.
>
> On the other hand, if this technique (using enable_sort) is already
> widely known and commonly used, and I'm simply unfamiliar with it,
> then perhaps adding this GUC might not be worth the effort.
>
> If several people support the idea of adding this GUC parameter,
> I'm thinking of creating and submitting a patch that incorporates your
> comment below. I believe both David and Ashutosh support the proposal.
> Can I go ahead as planned?
>
> What do others involved in planner and plan-tuning think?

I think this is a nice idea.  It is true that we can sometimes use
enable_sort to push the planner off a sorted grouping plan, but it is
a much bigger hammer, as it discourages every Sort in the query, not
just the one under the grouping, so it can move other parts of the
plan around and doesn't really isolate the grouping choice.  And it
doesn't always work anyway.  If the grouping input happens to be
cheaply sorted there's no Sort to penalize, so the sorted plan
survives.  I ran into exactly that in union.sql, where enable_sort=off
isn't really producing the hashed INTERSECT it looks like it is
testing.  Look at this plan from union.out:

-- check hashed implementation
set enable_hashagg = true;
set enable_sort = false;

explain (costs off)
select from generate_series(1,5) intersect select from generate_series(1,3);
                        QUERY PLAN
----------------------------------------------------------
 SetOp Intersect
   ->  Function Scan on generate_series
   ->  Function Scan on generate_series generate_series_1
(3 rows)

Also, without such a GUC, there are some paths that we have no way to
generate.  Look at this comment in union.sql:

-- We've no way to check hashed UNION as the empty pathkeys in the Append are
-- fine to make use of Unique, which is cheaper than HashAggregate and we've
-- no means to disable Unique.

Regarding the patch, currently the new GUC only covers GroupAggregate.
I think we should make it to cover all sort-based equivalents of
enable_hashagg, such as the sorted mode of SetOp (as David suggested),
sort-based Unique step used for DISTINCT and semijoin
unique-ification, and maybe also Group nodes.

I rebased your last patch and made some changes.  Attached is what I
ended up with.

- Richard

---

From: Richard Guo <[email-masked]>
Date: Thu, 25 Jun 2026 12:18:32 +0900

> I rebased your last patch and made some changes.  Attached is what I

I looked into the AGG_MIXED case some more, and I don't think the v3
patch handles it correctly.  I'd assumed an AGG_MIXED plan always does
some sorted grouping, but that's not true.  For the common cube/rollup
queries the grouping sets always include the empty set, which can't be
hashed, so even the maximally-hashed plan comes out as AGG_MIXED.
Disabling it under enable_groupagg therefore penalizes the very plan
we want.

To recap how grouping sets are costed: create_groupingsets_path loops
over the rollups and calls cost_agg once per rollup.  The first call
gets the path's overall strategy (AGG_MIXED for a mixed plan); the
rest come through as AGG_HASHED or AGG_SORTED, one per rollup, with
their disabled_nodes summed into the path.  So an AGG_MIXED plan's
sorted grouping shows up in the per-rollup AGG_SORTED calls, not in
the AGG_MIXED call itself.

And the empty grouping set is really computed like AGG_PLAIN, with no
hash table and no sort, so it shouldn't bump disabled_nodes at all.
It reaches cost_agg with numGroupCols == 0, which makes it easy to
tell apart.  So in the attached patch AGG_MIXED is disabled only by
enable_hashagg, and AGG_SORTED is disabled by enable_groupagg only
when numGroupCols > 0.  That also lets the tests in groupingsets.sql
use enable_groupagg, which they couldn't before.

- Richard

---

From: Tatsuro Yamada <[email-masked]>
Date: Thu, 25 Jun 2026 09:11:59 +0000

Hi Richard!

> select from generate_series(1,5) intersect select from generate_series(1,3);

Thank you for your comments! 
I'm glad that you appreciate the value of the patch. 

Regarding the comments,
I hadn't realized that there were other plan nodes besides GroupAgg that 
would also benefit from this GUC. I believe this feature would be useful to 
many users. I also think it could be valuable for testing and tuning tools 
such as pg_plan_advice, developed by Robert. For that reason, I'd like to 
continue improving the patch with the goal of getting it committed for PG20.

> unique-ification, and maybe also Group nodes.

It sounds like there are several plan nodes that could be covered by this GUC 
(such as SetOp, sort-based Unique for DISTINCT, semijoin uniqueification, 
and Group nodes). Do you think we should cover all of them before the patch 
would be considered complete enough for commit?

I plan to work through them one by one, but if others are interested in 
contributing, I would certainly welcome the collaboration. That would help 
move the patch forward more quickly and allow us to gather feedback on 
different parts of the design in parallel.

As a practical matter, only part of my time can be devoted to community 
development work. If I end up handling everything myself, progress may be 
slower, and I'd like to avoid the patch losing momentum.
A related challenge is that extending the patch to cover more plan nodes will 
likely increase the scope of the regression test changes. Perhaps it would 
make sense to split the work into smaller patches, allowing each set of 
regression test changes to be reviewed independently.

I'd be interested to hear your thoughts on how best to proceed, and whether 
dividing the work into smaller pieces would make sense.

Regards,
Tatsuro Yamada

---

From: Richard Guo <[email-masked]>
Date: Fri, 26 Jun 2026 15:26:20 +0900

> would be considered complete enough for commit?

Yeah, I think so.  The v4 patch actually already did that.  You can
find these changes in create_setop_path(), create_unique_path(), and
cost_group().

> dividing the work into smaller pieces would make sense.

It seems that the scope of the regression test changes isn't too
large, so I think keeping them in one patch is fine.

- Richard

---

From: Tatsuro Yamada <[email-masked]>
Date: Mon, 29 Jun 2026 10:02:29 +0000

Hi Richard,

> find these changes in create_setop_path(), create_unique_path(), and

Thank you for the clarification. I've reviewed the v4 patch and confirmed 
that it already covers SetOp, Unique, Group, etc.
I ran make installcheck in my environment and all tests passed without 
errors.

Since the enable_groupagg parameter also affects SetOp, Unique, and 
other nodes, I've created documentation patches to clarify this. I'm attaching 
them to this email. They apply on top of your v4 patch.

- 0001 patch: This improves the documentation for the enable_groupagg 
    parameter so that users can more easily understand which operations it 
    affects. If you agree with the changes, I'd like to post an updated patch 
    that integrates this into v4.

- 0002 patch: This provides a similar improvement for the enable_hashagg 
    parameter documentation. Since it concerns enable_hashagg, I'm happy 
    to move this to a separate thread if you prefer.

Best Regards,
Tatsuro Yamada

> -----Original Message-----
> From: Richard Guo <[email-masked]>
> Sent: Friday, June 26, 2026 3:26 PM
> To: Tatsuro Yamada（山田達朗） <[email-masked]>
> Cc: Tatsuro Yamada <[email-masked]>; David Rowley
> <[email-masked]>; Ashutosh Bapat
> <[email-masked]>; [email-masked]
> Subject: Re: Add enable_groupagg GUC parameter to control
> GroupAggregate usage
>

---

From: Richard Guo <[email-masked]>
Date: Tue, 7 Jul 2026 15:47:21 +0900

> them to this email. They apply on top of your v4 patch.

I've thought it over, and I'm inclined to leave the docs as they are.

My main concern is consistency with the surrounding parameters.  All
of the other enable_* GUCs are documented conceptually rather than by
the specific node types they affect, and none of them enumerate the
nodes you'd see in EXPLAIN.  Unique and SetOp are really EXPLAIN
labels rather than user-facing concepts, and I'd argue the existing
wording already covers them, since sort-based Unique and sorted SetOp
are both forms of sort-based grouping.

Similarly, enable_hashagg has covered hash-based SetOp for a long time
without us documenting it, which is the same convention at work.  I'd
rather not add that now just to match the new parameter.

Attached is a rebased patch to make cfbot work again.

- Richard

---

From: Richard Guo <[email-masked]>
Date: Tue, 7 Jul 2026 15:53:20 +0900

> Attached is a rebased patch to make cfbot work again.

I intend to push this patch soon.  That way newly added test cases can
start using enable_groupagg directly.  Otherwise we'll have to keep
rebasing it to convert any new tests that use enable_sort to force
hashed grouping.  Any thoughts?

- Richard

---

From: Tatsuro Yamada <[email-masked]>
Date: Wed, 8 Jul 2026 11:09:51 +0000

Hi Richard,

>rather not add that now just to match the new parameter.

Thanks for your comments and the rebased patch.
I understand and agree with the decision not to modify the documentation.

My initial motivation for proposing the documentation update was to make it 
clearer which plan nodes are affected by these GUC parameters. While reviewing 
your previous patch, I realized that enable_hashagg and enable_groupagg affect 
not only HashAgg and GroupAgg nodes but also other plan nodes.

However, documenting that level of detail only for these parameters would be 
inconsistent with the descriptions of the other enable_* GUCs. So I agree that 
we should keep the current documentation as it is.

>Attached is a rebased patch to make cfbot work again.

I applied the attached v5 patch to commit 57f93af36, and confirmed that all 
regression tests completed successfully.

Regards,
Tatsuro Yamada

> Sent: Tuesday, July 7, 2026 3:47 PM
> Subject: Re: Add enable_groupagg GUC parameter to control GroupAggregate
> usage
>

---

From: Tatsuro Yamada <[email-masked]>
Date: Wed, 8 Jul 2026 11:54:13 +0000

Hi Richard,

>hashed grouping.  Any thoughts?

I'm in favor of committing this.
Before you do, I'd like to clarify one point regarding the commit message. 
Would it make sense to list both of us as authors?

The reason I ask is that you made substantial changes to the patch, 
including handling additional plan nodes such as SetOp and adding regression 
tests. Given those contributions, I wondered whether it would be more 
appropriate for both of us to be listed as authors. (I've seen past commits that 
included two Author: lines.)
I'm happy to leave that decision to you.

By the way, I re-ran the test query from the initial email in this thread.
The execution times were:

  Default: 25,791.642 ms
  SET enable_groupagg TO off;: 1,006.839 ms

This again confirmed an approximately 25x speedup.

Of course, the actual benefit depends on the specific query, but I believe 
this parameter will allow users to improve query performance in more situations.

Regards,
Tatsuro Yamada

> Sent: Tuesday, July 7, 2026 3:53 PM

---

From: Richard Guo <[email-masked]>
Date: Thu, 9 Jul 2026 10:01:35 +0900

> I'm happy to leave that decision to you.

Pushed.

I still listed myself as Co-authored-by rather than Author, because I
want to give full credit to you, as this is your idea and your patch.

- Richard

---

From: Tatsuro Yamada <[email-masked]>
Date: Thu, 9 Jul 2026 10:55:50 +0000

Hi Richard,

> want to give full credit to you, as this is your idea and your patch.

Thank you for the commit!
I also appreciate your consideration in giving me full credit for the patch.

Regards,
Tatsuro Yamada

> Sent: Thursday, July 9, 2026 10:02 AM