| From: | Richard Guo <guofenglinux(at)gmail(dot)com> |
|---|---|
| To: | Tatsuro Yamada <yamatattsu(at)gmail(dot)com> |
| Cc: | David Rowley <dgrowleyml(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Add enable_groupagg GUC parameter to control GroupAggregate usage |
| Date: | 2026-06-25 03:18:32 |
| Message-ID: | CAMbWs4-YtrPXJ+pmYYC+6y+q=Dwj0f845MHAz4gm41skmzWFtg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Jun 25, 2026 at 8:05 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> I rebased your last patch and made some changes. Attached is what I
> ended up with.
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
| Attachment | Content-Type | Size |
|---|---|---|
| v4-0001-Add-an-enable_groupagg-GUC-parameter.patch | application/octet-stream | 33.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Smith | 2026-06-25 03:31:29 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Chengpeng Yan | 2026-06-25 03:12:13 | Re: Improve UNION's output rowcount estimate |