diff options
author | David Rowley <drowley@postgresql.org> | 2022-08-18 11:32:55 +1200 |
---|---|---|
committer | David Rowley <drowley@postgresql.org> | 2022-08-18 11:32:55 +1200 |
commit | af7d270dd3c7523dad023f5761f630daa7fb2730 (patch) | |
tree | f4ecd7e1057c843d8abe7749c24a7ae40eca491a | |
parent | a340359a4f9bed54e0af67dd90050c13e0976fb6 (diff) | |
download | postgresql-af7d270dd3c7523dad023f5761f630daa7fb2730.tar.gz postgresql-af7d270dd3c7523dad023f5761f630daa7fb2730.zip |
Fix hypothetical problem passing the wrong GROUP BY pathkeys
1349d2790 changed things to make the planner request that the
query_pathkeys contain pathkeys for any ORDER BY / DISTINCT aggregates.
Some code added prior to that commit in db0d67db2 made it so the order
that the pathkeys appear in the group_pathkeys could be changed so that
the GROUP BY could be executed in a more optimal order which minimized
sort comparisons. 1349d2790 had to make sure that the pathkeys for any
ORDER BY / DISTINCT aggregates remained at the end of the groupby_pathkeys
and wasn't reordered, so some code was added to
add_paths_to_grouping_rel() to first strip off any pathkeys belonging to
ORDER BY / DISTINCT aggregates before passing to the function to optimize
the order of the group_pathkeys.
It seems I dropped the ball in 1349d2790 and mistakenly used the untouched
PlannerInfo.group_pathkeys to pass to get_useful_group_keys_orderings()
instead of the version that had the aggregate pathkeys removed. It was
only the code path that was handling creating paths for
partially_grouped_rel which made this mistake. In practice, we'll never
have any extra pathkeys to strip off when processing
partially_grouped_rel as that's only used when considering partial
paths, which we never do when there are ORDER BY / DISTINCT aggregates.
So this is just a hypothetical bug, not a live bug. We already have the
correct pathkeys determined, so it's of no extra cost to pass the
correct variable.
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20220817015755.GB26426@telsasoft.com
-rw-r--r-- | src/backend/optimizer/plan/planner.c | 3 |
1 files changed, 0 insertions, 3 deletions
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index bf45b498ba3..b781080dbac 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -6637,10 +6637,7 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel, ListCell *lc2; Path *path = (Path *) lfirst(lc); Path *path_original = path; - List *pathkey_orderings = NIL; - - List *group_pathkeys = root->group_pathkeys; List *group_clauses = parse->groupClause; /* generate alternative group orderings that might be useful */ |