Remove useless param for create_groupingsets_path

Started by XueJing Zhaoalmost 4 years ago4 messageshackers
Jump to latest
#1XueJing Zhao
zxuejing@vmware.com

Dear Postgres,

Recently I work on grouping sets and I find the last param numGroups of create_groupingsets_path is not used.
In create_groupingsets_path we use rollup->numGroups to do cost_agg. So I remove the param numGroups for
create_groupingsets_path.

I generate a diff.patch, which is sent as an attachment.
I really hope this can be committed to Postgres. Thank you a lot!

Best wishes to you!
zxuejing

Attachments:

diff.patchapplication/octet-stream; name=diff.patchDownload+11-5
#2Richard Guo
guofenglinux@gmail.com
In reply to: XueJing Zhao (#1)
Re: Remove useless param for create_groupingsets_path

On Wed, Jun 15, 2022 at 11:33 AM XueJing Zhao <zxuejing@vmware.com> wrote:

Recently I work on grouping sets and I find the last param numGroups of
create_groupingsets_path is not used.

In create_groupingsets_path we use rollup->numGroups to do cost_agg.

Yes indeed. The param 'numGroups' was used originally when we first
introduced in create_groupingsets_path(), and then all its references
inside that function were removed and replaced with the numGroups inside
RollupData in b5635948.

I generate a diff.patch, which is sent as an attachment.

BTW, the patch looks weird to me that it seems operates in the inverse
direction, i.e. it's adding the param 'numGroups', not removing it.

Thanks
Richard

#3XueJing Zhao
zxuejing@vmware.com
In reply to: Richard Guo (#2)
Reply: Remove useless param for create_groupingsets_path

Hi, Richard
You are right, The patch is incorrect, and I generate a patch once more, It is sent as as attachment named new,patch, please check, thanks!

Best regards!
Zxuejing

From: Richard Guo <guofenglinux@gmail.com>
Date: 2022-06-15 12:12
To: XueJing Zhao <zxuejing@vmware.com>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Subject: Re: Remove useless param for create_groupingsets_path

On Wed, Jun 15, 2022 at 11:33 AM XueJing Zhao <zxuejing@vmware.com<mailto:zxuejing@vmware.com>> wrote:
Recently I work on grouping sets and I find the last param numGroups of create_groupingsets_path is not used.
In create_groupingsets_path we use rollup->numGroups to do cost_agg.

Yes indeed. The param 'numGroups' was used originally when we first
introduced in create_groupingsets_path(), and then all its references
inside that function were removed and replaced with the numGroups inside
RollupData in b5635948.

I generate a diff.patch, which is sent as an attachment.

BTW, the patch looks weird to me that it seems operates in the inverse
direction, i.e. it's adding the param 'numGroups', not removing it.

Thanks
Richard

________________________________

Attachments:

new.diffapplication/octet-stream; name=new.diffDownload+5-11
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: XueJing Zhao (#3)
Re: Reply: Remove useless param for create_groupingsets_path

XueJing Zhao <zxuejing@vmware.com> writes:

You are right, The patch is incorrect, and I generate a patch once more, It is sent as as attachment named new,patch, please check, thanks!

LGTM. Pushed, thanks!

regards, tom lane