Consider unsorted paths in generate_useful_gather_paths

generate_useful_gather_paths used to skip unsorted paths (without any
pathkeys), but that is unnecessary - the later code actually can handle
such paths just fine by adding a Sort node. This is clearly a thinko,
preventing construction of useful plans.

Backpatch to 13, where Incremental Sort was introduced.

Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 84a69b064a9850f312a38c0b863c32a501d79831..a1b3e4b82158d83a84ba4f15bcee767b8ad1678d 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2900,7 +2900,8 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
    cheapest_partial_path = linitial(rel->partial_pathlist);
 
    /*
-    * Consider incremental sort paths for each interesting ordering.
+    * Consider sorted paths for each interesting ordering. We generate both
+    * incremental and full sort.
     */
    foreach(lc, useful_pathkeys_list)
    {
@@ -2914,14 +2915,6 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
            Path       *subpath = (Path *) lfirst(lc2);
            GatherMergePath *path;
 
-           /*
-            * If the path has no ordering at all, then we can't use either
-            * incremental sort or rely on implicit sorting with a gather
-            * merge.
-            */
-           if (subpath->pathkeys == NIL)
-               continue;
-
            is_sorted = pathkeys_count_contained_in(useful_pathkeys,
                                                    subpath->pathkeys,
                                                    &presorted_keys);

diff --git a/src/test/regress/expected/incremental_sort.out b/src/test/regress/expected/incremental_sort.out
index 7cf2eee7c14cbe78288f46a75689a576d360854d..51471ae92de9d286d56045ae39356eb9988ce0d4 100644 (file)
--- a/src/test/regress/expected/incremental_sort.out
+++ b/src/test/regress/expected/incremental_sort.out
@@ -1468,6 +1468,19 @@ explain (costs off) select * from t union select * from t order by 1,3;
                            ->  Parallel Seq Scan on t t_1
 (13 rows)
 
+-- Full sort, not just incremental sort can be pushed below a gather merge path
+-- by generate_useful_gather_paths.
+explain (costs off) select distinct a,b from t;
+                QUERY PLAN                
+------------------------------------------
+ Unique
+   ->  Gather Merge
+         Workers Planned: 2
+         ->  Sort
+               Sort Key: a, b
+               ->  Parallel Seq Scan on t
+(6 rows)
+
 drop table t;
 -- Sort pushdown can't go below where expressions are part of the rel target.
 -- In particular this is interesting for volatile expressions which have to

diff --git a/src/test/regress/sql/incremental_sort.sql b/src/test/regress/sql/incremental_sort.sql
index 3ee11c394b91bc83be122d08d9ee1b844bf69912..cb48f200ce555a1249e2cbcf1d0dac0c750a8c8a 100644 (file)
--- a/src/test/regress/sql/incremental_sort.sql
+++ b/src/test/regress/sql/incremental_sort.sql
@@ -220,6 +220,10 @@ explain (costs off) select a,b,sum(c) from t group by 1,2 order by 1,2,3 limit 1
 set enable_hashagg to off;
 explain (costs off) select * from t union select * from t order by 1,3;
 
+-- Full sort, not just incremental sort can be pushed below a gather merge path
+-- by generate_useful_gather_paths.
+explain (costs off) select distinct a,b from t;
+
 drop table t;
 
 -- Sort pushdown can't go below where expressions are part of the rel target.