public inbox for [email protected]
help / color / mirror / Atom feedFrom: Justin Pryzby <[email protected]>
To: David Rowley <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Richard Guo <[email protected]>
Cc: Ronan Dunklau <[email protected]>
Cc: [email protected]
Cc: Ranier Vilela <[email protected]>
Subject: Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Date: Tue, 16 Aug 2022 20:57:55 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAApHDvpQ+LmokunMogxc1Ba+AOptP=sHqruCWeC8Xgv2XDbiDg@mail.gmail.com>
References: <3163474.aeNJFYEL58@aivenronan>
<CAApHDvqS-p57j++gLMyQXmttk_MM3u243dWQ54FyW-7=4GP+1Q@mail.gmail.com>
<CAMbWs49hcB-Mj3MJTOesL5=y4oU+md=zEvDYdCBrOSaDvXLRGQ@mail.gmail.com>
<CAApHDvr3JQv4_H7vfu5Djtw0ubKwGDwvNtPuphtaQY4utgj7bQ@mail.gmail.com>
<CAMbWs4_hAK6+0Gk=vZX+ikSFBx=6981iFLGkgLObDkvvGpjogg@mail.gmail.com>
<CAApHDvrai=tCuUhH=YUvDnysivzbabz4pGPGz_3SytysWmyNdg@mail.gmail.com>
<CAMbWs49Xw1Sb4KOZx6CMGBjiCNvGCcAyXkwWRGNZivyYv2fGMg@mail.gmail.com>
<CAApHDvqtmK7AprGs=uh2+BNyew6L3OpC7hPSe43uD-7m1Dtx7w@mail.gmail.com>
<[email protected]>
<CAApHDvpQ+LmokunMogxc1Ba+AOptP=sHqruCWeC8Xgv2XDbiDg@mail.gmail.com>
On Tue, Aug 02, 2022 at 11:21:04PM +1200, David Rowley wrote:
> I've now pushed the patch.
I've not studied the patch at all.
But in a few places, it removes the locally-computed group_pathkeys:
- List *group_pathkeys = root->group_pathkeys;
However it doesn't do that here:
/*
* Instead of operating directly on the input relation, we can
* consider finalizing a partially aggregated path.
*/
if (partially_grouped_rel != NULL)
{
foreach(lc, partially_grouped_rel->pathlist)
{
ListCell *lc2;
Path *path = (Path *) lfirst(lc);
Path *path_original = path;
List *pathkey_orderings = NIL;
List *group_pathkeys = root->group_pathkeys;
I noticed because that creates a new shadow variable, which seems accidental.
make src/backend/optimizer/plan/planner.o COPT=-Wshadow=compatible-local
src/backend/optimizer/plan/planner.c:6642:14: warning: declaration of ‘group_pathkeys’ shadows a previous local [-Wshadow=compatible-local]
6642 | List *group_pathkeys = root->group_pathkeys;
| ^~~~~~~~~~~~~~
src/backend/optimizer/plan/planner.c:6438:12: note: shadowed declaration is here
6438 | List *group_pathkeys;
--
Justin
view thread (64+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add proper planner support for ORDER BY / DISTINCT aggregates
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox