RE: Add enable_groupagg GUC parameter to control GroupAggregate usage

From: Tatsuro Yamada <tatsuro(dot)yamada(at)ntt(dot)com>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: Tatsuro Yamada <yamatattsu(at)gmail(dot)com>, 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" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Add enable_groupagg GUC parameter to control GroupAggregate usage
Date: 2026-07-08 11:09:51
Message-ID: TY4P301MB1299C1F20A127C0E538674F39EFF2@TY4P301MB1299.JPNP301.PROD.OUTLOOK.COM
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Richard,

>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.

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

> -----Original Message-----
> From: Richard Guo <guofenglinux(at)gmail(dot)com>
> Sent: Tuesday, July 7, 2026 3:47 PM
> To: Tatsuro Yamada(山田達朗) <tatsuro(dot)yamada(at)ntt(dot)com>
> Cc: Tatsuro Yamada <yamatattsu(at)gmail(dot)com>; 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
>
> On Mon, Jun 29, 2026 at 7:02 PM Tatsuro Yamada
> <tatsuro(dot)yamada(at)ntt(dot)com> wrote:
> > 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.
>
> 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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Hans Buschmann 2026-07-08 11:24:55 AW: Increased SECURITY RISCS from omitting some compikler options when building PG with meson; e.g. -fcf-protection=full
Previous Message vignesh C 2026-07-08 11:06:20 Re: PSQL - prevent describe listing tables that are already in listed schemas