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);
 

Reply via email to