Re: Add proper planner support for ORDER BY / DISTINCT aggregates

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