On Thu, 14 Mar 2024 at 06:00, Alexander Lakhin <exclus...@gmail.com> wrote: > I've stumbled upon the same error, but this time it apparently has another > cause. It can be produced (on REL_16_STABLE and master) as follows: > CREATE TABLE t (a int, b int) PARTITION BY RANGE (a); > CREATE TABLE td PARTITION OF t DEFAULT; > CREATE TABLE tp1 PARTITION OF t FOR VALUES FROM (1) TO (2); > SET enable_partitionwise_aggregate = on; > SET parallel_setup_cost = 0; > SELECT a, sum(b order by b) FROM t GROUP BY a ORDER BY a; > > ERROR: could not find pathkey item to sort > > `git bisect` for this anomaly blames the same commit 1349d2790.
Thanks for finding and for the recreator script. I've attached a patch which fixes the problem for me. On debugging this I uncovered some other stuff that looks broken which seems to caused by partition-wise aggregates. With your example query, in get_useful_pathkeys_for_relation(), we call relation_can_be_sorted_early() to check if the pathkey can be used as a set of pathkeys in useful_pathkeys_list. The problem is that in your query the 'rel' is the base relation belonging to the partitioned table and relation_can_be_sorted_early() looks through the targetlist for that relation and finds columns "a" and "b" in there. The problem is "b" has been aggregated away as partial aggregation has taken place due to the partition-wise aggregation. I believe whichever rel we should be using there should have an Aggref in the target exprs rather than the plain unaggregated column. I've added Robert and Ashutosh to see what their thoughts are on this. David
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 443ab08d75..eaba6ddc03 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -7320,6 +7320,15 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel) { ListCell *lc; Path *cheapest_partial_path; + List *groupby_pathkeys; + + + /* trim off any pathkeys added for ORDER BY / DISTINCT aggregates */ + if (list_length(root->group_pathkeys) > root->num_groupby_pathkeys) + groupby_pathkeys = list_copy_head(root->group_pathkeys, + root->num_groupby_pathkeys); + else + groupby_pathkeys = root->group_pathkeys; /* Try Gather for unordered paths and Gather Merge for ordered ones. */ generate_useful_gather_paths(root, rel, true); @@ -7334,7 +7343,7 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel) int presorted_keys; double total_groups; - is_sorted = pathkeys_count_contained_in(root->group_pathkeys, + is_sorted = pathkeys_count_contained_in(groupby_pathkeys, path->pathkeys, &presorted_keys); @@ -7366,7 +7375,7 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel) path = (Path *) create_incremental_sort_path(root, rel, path, - root->group_pathkeys, + groupby_pathkeys, presorted_keys, -1.0); @@ -7375,7 +7384,7 @@ gather_grouping_paths(PlannerInfo *root, RelOptInfo *rel) rel, path, rel->reltarget, - root->group_pathkeys, + groupby_pathkeys, NULL, &total_groups);