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