MIME-Version: 1.0
References: 
 <CAMbWs48jzLrPt1J_00ZcPZXWUQKawQOFE8ROc-ADiYqsqrpBNw@mail.gmail.com>
 <87il22cj51.fsf@163.com>
 <CAMbWs49=eAd2W9jCtGhaZPPp+SOC_2rg16RTG74xAht=hkr5JQ@mail.gmail.com>
 <CAMbWs49Nc4M3H+eCf1+8w8piDyEECjRb-gK_JMF4VvcyWwGEVQ@mail.gmail.com>
 <CAMbWs49E_dR0nobsExsyetpnBpHObLTsQLsEbWKQLkh0omPxNg@mail.gmail.com>
 <CAMbWs49B_qUiHvu2EqLHZRpLr3p_+QPBs50n2=L5ibYzniwTzA@mail.gmail.com>
 <CAMbWs48KCQtDymnYi4M=Vz+WMzo3fkBxffJsyk6VX6hOXXv+VA@mail.gmail.com>
 <CAMbWs49sv_MuOYqqrtmBN_oYf8VSQ2BXDwXaTpJTn_YfwyYdWQ@mail.gmail.com>
 <CAMbWs49U8Sddx_fGszPdvA3jp_nheynxaqm5Y4NqMV21VBYAuQ@mail.gmail.com>
 <CAMbWs4-LwyOg9ga+NVF7yQbMi0ZsZdN1G_sO2v=YJHV18=19+A@mail.gmail.com>
 <CALA8mJquG_zCJXfVwash5LKqHGtZXQmq7RfTSaRDUzGYeW=7Rw@mail.gmail.com>
 <CAMbWs4_EjgcBib5+y1LYcGB3EK3Y6R+OOxGKfJo42fDovadk1g@mail.gmail.com>
 <CALA8mJqe0anNM8_V6cOeOQnCHUTQggn7iOQNyQr1VaN_xMjz+w@mail.gmail.com>
 <CAMbWs48eE-s-jCicC8pSVfXk8Ws-ZvUKnsw8qH-DkVBdYv0eJQ@mail.gmail.com>
 <CAMbWs483a7-8M0pDttG44r-+8Gevn9VG0xNceE3WpkEQxJXPZw@mail.gmail.com>
 <CAHewXNmYM6DvR_kaxDL0w0fz9BwKbac+TSU3QS10aA3cXHyMmA@mail.gmail.com>
 <CA+TgmoaxH=P63hLYgyJJcEbMRnw3xi16d=HxFi1j-m7MhH6W_w@mail.gmail.com>
 <CAMbWs4_cOnpGsywj9Jt1WAgzJLW9Rxt5X13cfGz4iN2qvZQ68g@mail.gmail.com>
 <CA+Tgmob0q7bRbsFTVDMjxHE6zA4uDQLQa-s0CtwUw49V53UL_A@mail.gmail.com>
 <CAMbWs4-Xru_eKBeRHFduigSGihdixFWVTR8A+dtMw7Mao+RkJA@mail.gmail.com>
 <CAMbWs49dLjSSQRWeud+KSN0G531ciZdYoLBd5qktXA+3JQm_UQ@mail.gmail.com>
 <CAMbWs48LXGC-Y63YtzEeM-3f0NUXWCUEMs7XwGzywXTjUNMcxQ@mail.gmail.com>
 <CAMbWs48XdzvnwfTHWxQ7qK-yjvdrbwsPpqhJBuKDnO+hcbsVwA@mail.gmail.com>
 <CA+TgmoaO-7RHdyJuizWChXZm7EJGvDcfoePDDEyUA-y8vTB1tg@mail.gmail.com>
 <CAMbWs4-+jXRpKuFMZa08bS34-TBka3qqjVMAUjF=-1RA9BKvgg@mail.gmail.com>
 <CA+TgmoZapU1y59-s3o8oPt7Hv+cxRh_34FMu6MXumomLe+U1Cw@mail.gmail.com>
 <CAMbWs4_sEeeBmucBzbamBMfA9uLxVmOc_MV=ZpSyDbTcrUO_XQ@mail.gmail.com>
 <CA+Tgmob4fnv57PQB0Oox86mHSJQ0vVL249eT=gqPvrMkG7h1zw@mail.gmail.com>
 <CAMbWs489NYyTcCTbrUi7hPXKtNY5vHrrFcHyMRAv=CA5WsszVw@mail.gmail.com>
 <CA+TgmoazmDdcc7NeTo3WM5HW3DASNP4rfZw6X+2nnQKHampOng@mail.gmail.com>
 <CAMbWs49bYr-ULhA+-At0iQ+NaFKy72AWB6jzughk8MPTiY+gMQ@mail.gmail.com>
 <CA+TgmoYa-zexdbc5nO_D6oxPMZYs06hkYwZK5Dufq+4Hhe6uNQ@mail.gmail.com>
 <CAMbWs4_aji0kME490phz6nTXnPToddUn19OF3rLm1g4TbNkuzQ@mail.gmail.com>
 <CA+Tgmoa3+G_=8XuQWN+0ugv6r-WV6ruFESpOxpXAAKrne3oVDQ@mail.gmail.com>
 <CAMbWs49qiox13EKb7bqgLu7Gu9oar+xe6KMwBjgFwod3JzPfUw@mail.gmail.com>
 <CAMbWs48F8WGA-Lzj1Dk76mFqRFxPEwG2_9Zb7+pFs8oi6ew2pw@mail.gmail.com>
 <CAMbWs484ms=WRZamOyWnVditREKFqipLsdaQjcv2uKur8SZuqw@mail.gmail.com>
 <CAMbWs49bL2ZMSc0W4G8=R7bjaa-vO6grucEOFYLZFUZE7+nzrQ@mail.gmail.com>
 <CA+TgmobqbeJ9iRQO4ym6OiHt71sSv2eai=01kOZjxhdof9K4Mw@mail.gmail.com>
 <CAMbWs4_2BzuAX+BSO1p7rtUwmQjORrG-b906Cw-RkfRjFP0oSQ@mail.gmail.com>
 <CA+TgmoYbkvYwLa+1vOP7RDY7kO2=A7rppoPusoRXe44VDOGBPg@mail.gmail.com>
 <CAMbWs4_aezTYOZSj7v+aypLo0dnjAierJtdx2gf6se28p88WHg@mail.gmail.com>
 <CA+TgmoaY6E5-UFTWp5BtAjBO=tDQd=UVAgeJ3dRbFFzhnP5NHg@mail.gmail.com>
 <CAMbWs484dnecwXT2WzWFvzEmWPzC3U9F8SRDXg-SEegTYUFyXQ@mail.gmail.com>
 <CAMbWs4-2cVfBk1HNGtqV1QFo2yKnzdxLy0BAqQaJHBt+8+kspw@mail.gmail.com>
 <CAMbWs48W80HFm9b+yZPKER=MA5M_bveYvBx1AwOrxdPYbLmYmQ@mail.gmail.com>
 <CAMbWs4_W6PmVXxgqM8LMerX2iwqOwR5EC5RGWSAMuvkm8o+-jw@mail.gmail.com>
 <CAApHDvrxyGSLv3Sbg9fpmz6yYri_7M6SaKYnqQQv59uZfQdduA@mail.gmail.com>
 <CAMbWs48qOOfBV6svQgbdzZ4HbTV18jsjyFD_b6ofJQfAt5n11w@mail.gmail.com>
In-Reply-To: 
 <CAMbWs48qOOfBV6svQgbdzZ4HbTV18jsjyFD_b6ofJQfAt5n11w@mail.gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
Date: Thu, 9 Oct 2025 00:14:39 +1300
Message-ID: 
 <CAApHDvr4YWpiMR3RsgYwJWv-u8xoRqTAKRiYy9zUszjZOqG4Ug@mail.gmail.com>
Subject: Re: Eager aggregation, take 3
To: Richard Guo <guofenglinux@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	Tender Wang <tndrwang@gmail.com>, Paul George <p.a.george19@gmail.com>,
	Andy Fan <zhihuifan1213@163.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
 pgsql-hackers@lists.postgresql.org,
	Matheus Alcantara <matheusssilv97@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAApHDvr4YWpiMR3RsgYwJWv-u8xoRqTAKRiYy9zUszjZOqG4Ug%40mail.gmail.com>
Precedence: bulk

On Tue, 7 Oct 2025 at 23:57, Richard Guo <guofenglinux@gmail.com> wrote:
>
> On Mon, Oct 6, 2025 at 10:59=E2=80=AFPM David Rowley <dgrowleyml@gmail.co=
m> wrote:
> > 6. Shouldn't this be using lappend()?
> >
> >  agg_clause_list =3D list_append_unique(agg_clause_list, ac_info);
> >
> > I don't understand why ac_info could already be in the list. You've
> > just done: ac_info =3D makeNode(AggClauseInfo);
>
> A query can specify the same Aggref expressions multiple times in the
> target list.  Using lappend here can lead to duplicate partial Aggref
> nodes in the targetlist of a grouped path, which is what I want to
> avoid.

I was getting that mixed up with list_append_unique_ptr().

> > 9. In get_expression_sortgroupref(), a comment claims "We ignore child
> > members here.". I think that's outdated since ec_members no longer has
> > child members.
>
> I think that comment is used to explain why we only scan ec_members
> here.  Similar comments can be found in many other places, such as in
> equivclass.c:
>
>   /*
>    * Found our match.  Scan the other EC members and attempt to generate
>    * joinclauses.  Ignore children here.
>    */
>   foreach(lc2, cur_ec->ec_members)
>   {

I'd say that's also wrong. "Ignore" means not to pay attention to
something that's there. The child members are not there.

> > 11. The way you've written the header comments for typedef struct
> > RelAggInfo seems weird.  I've only ever seen extra details in the
> > header comment when the inline comments have been kept to a single
> > line. You're spanning multiple lines, so why have the out of line
> > comments in the header at all?

> I've also updated the comments within RelAggInfo to use one-line
> style.

The style I'd thought of had the comments on the same line as the
field. Something like struct EquivalenceClass.

>I wrapped the long queries in v24.

+-- Enable eager aggregation, which by default is disabled.
+SET enable_eager_aggregate TO on;

The above comment and command mismatch to my understanding from
looking at postgresql.conf.sample and guc_parameters.dat.

David