MIME-Version: 1.0
References: 
 <CAOKkKFvYHSEsFazkrf9bRH14p-H27XMaqbZfRYjS6EHBruvZMQ@mail.gmail.com>
In-Reply-To: 
 <CAOKkKFvYHSEsFazkrf9bRH14p-H27XMaqbZfRYjS6EHBruvZMQ@mail.gmail.com>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Mon, 9 Jun 2025 16:20:39 +0530
Message-ID: 
 <CAExHW5tPhCw5HpHG6F7uAXx3re_gRXqm_ryJFjFQ2+WrVY+hPQ@mail.gmail.com>
Subject: Re: Add enable_groupagg GUC parameter to control GroupAggregate usage
To: Tatsuro Yamada <yamatattsu@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Content-Type: multipart/alternative; boundary="000000000000705d610637215ac6"
Archived-At: 
 <https://www.postgresql.org/message-id/CAExHW5tPhCw5HpHG6F7uAXx3re_gRXqm_ryJFjFQ2%2BWrVY%2BhPQ%40mail.gmail.com>
Precedence: bulk

--000000000000705d610637215ac6
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Jun 6, 2025 at 1:29=E2=80=AFPM Tatsuro Yamada <yamatattsu@gmail.com=
> wrote:

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

I first thought enable_hashagg should be sufficient to choose one strategy
over the other. But that is not true, enable_hashagg =3D true allows both t=
he
strategies, enable_hashagg =3D false disables just hash strategy. There's n=
o
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 =3D false as well?


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

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 =3D false is not needed. We will need some test to test the
switch though.


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

--=20
Best Wishes,
Ashutosh Bapat

--000000000000705d610637215ac6
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote g=
mail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Fri, Jun 6, =
2025 at 1:29=E2=80=AFPM Tatsuro Yamada &lt;<a href=3D"mailto:yamatattsu@gma=
il.com">yamatattsu@gmail.com</a>&gt; wrote:<br></div><blockquote class=3D"g=
mail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204=
,204,204);padding-left:1ex"><div dir=3D"ltr"><div><div><div>Hi hackers,<br>=
<br>When I measured the execution time of a certain query with parallel que=
ry<br>enabled and disabled, I found that the execution time was slower when=
<br>parallel query was enabled.<br><br>To improve the performance of the pa=
rallel query, I considered adjusting<br>the execution plan and attempted to=
 switch from GroupAggregate to<br>HashAggregate. However, I noticed that th=
ere was no GUC parameter to<br>disable GroupAggregate.<br><br>Therefore, I =
propose adding a new GUC parameter: enable_groupagg.<br><br>Below are the r=
esults of a performance test where I disabled<br>GroupAggregate using enabl=
e_groupagg. In this case, the planner chose<br>HashAggregate instead, which=
 improved performance by about 35 times.<br><br># Query Execution Results (=
Average of 3 measurements)<br>- With parallel query: =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 39546 seconds<br>- With parallel query and enable_groupag=
g turned off: =C2=A0 1115 seconds<br><br># Query and Data Used (attached to=
 this email)<br>- Query: test_query.sql<br>- Data: =C2=A0create_table.sql<b=
r><br># The steps to run the test are as follows.<br>For example, on psql:<=
br><br>1. Create tables:<br>=C2=A0 =C2=A0 \i create_table.sql<br><br>2. Exe=
cute a query:<br>=C2=A0 =C2=A0 \i test_query.sql<br><br>3. Execute a query =
using the new GUC parameter:<br>=C2=A0 =C2=A0 set enable_groupagg to off;<b=
r>=C2=A0 =C2=A0 \i test_query.sql<br><br>As a benefit to users, while there=
 has previously been a GUC parameter<br>to control HashAggregate, there was=
 no corresponding way to control<br>GroupAggregate. This patch addresses th=
at, giving users more flexibility<br>in tuning execution plans.<br><br>I=
9;ve attached a WIP patch that adds this GUC parameter. I would<br>apprecia=
te any feedback, especially regarding how many test cases I <br>should crea=
te.<br></div></div></div></div></blockquote><div><br></div><div><div>I firs=
t thought enable_hashagg should be sufficient to choose one strategy over t=
he other. But that is not true, enable_hashagg =3D true allows both the str=
ategies, enable_hashagg =3D false disables just hash strategy. There&#39;s =
no way to disable group agg alone. So I think it makes sense to have this G=
UC.</div><div><br></div><div>I am surprised that we didn&#39;t see this bei=
ng a problem for so long.</div><div><br></div><div>We seem to disable mixed=
 strategy when enable_hashagg is false. Do we want to do the same when enab=
le_groupagg =3D false as well?</div></div><div>=C2=A0</div><blockquote clas=
s=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid r=
gb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><div><div><div><br>To cr=
eate new test cases for enable_groupagg, I looked into existing <br>test ca=
ses that use enable_hashagg and found that it is used in many <br>places (6=
2 places). Should I add a test case for enable_groupagg in <br>the same pla=
ce as enable_hashagg? I think that adding a new feature <br>requires a mini=
mum number of test cases, so I would appreciate your <br>advice.<br></div><=
/div></div></div></blockquote><div><br></div><div>Some of those instances a=
re for plan stability, all of which need not be replicated. But some of the=
m explicitly test sort based grouping. For rest of them hash based plan see=
ms to be the best one, so explicit enable_groupagg =3D false is not needed.=
 We will need some test to test the switch though.</div><div>=C2=A0</div><b=
lockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-le=
ft:1px solid rgb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><div><div>=
<div><br><br>Additionally, based on the execution plan, I suspect the slowd=
own in the<br>parallel query might be caused by misestimates related to Sor=
t or<br>Gather Merge.<br>While resolving those misestimates would ideally i=
mprove the root issue,<br>I&#39;d like to keep the focus of this thread on =
adding the GUC parameter.<br>Then, I plan to report or address the estimati=
on problem in a separate<br>thread.<br></div></div></div><div><br></div></d=
iv></blockquote></div><div><br></div><div>+1.<br></div><div><br></div><span=
 class=3D"gmail_signature_prefix">-- </span><br><div dir=3D"ltr" class=3D"g=
mail_signature"><div dir=3D"ltr"><div><div dir=3D"ltr"><div>Best Wishes,</d=
iv><div>Ashutosh Bapat<br></div></div></div></div></div></div>

--000000000000705d610637215ac6--