Hi Richard, > 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().
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 protected]>
> Sent: Friday, June 26, 2026 3:26 PM
> To: Tatsuro Yamada（山田達朗） <[email protected]>
> Cc: Tatsuro Yamada <[email protected]>; David Rowley
> <[email protected]>; Ashutosh Bapat
> <[email protected]>; [email protected]
> Subject: Re: Add enable_groupagg GUC parameter to control
> GroupAggregate usage
>
> On Thu, Jun 25, 2026 at 6:12 PM Tatsuro Yamada <[email protected]>
> wrote:
> > 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?
>
> 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().
>
> > I'd be interested to hear your thoughts on how best to proceed, and
> whether
> > 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
0001-doc-Clarify-enable_groupagg-also-covers-Unique-and-S.patch
Description: 0001-doc-Clarify-enable_groupagg-also-covers-Unique-and-S.patch
0002-doc-Clarify-enable_hashagg-also-covers-hash-based-Se.patch
Description: 0002-doc-Clarify-enable_hashagg-also-covers-hash-based-Se.patch
