From f4a3c0b06250ddc8ae09b59b87cf68e9bc0d7ca1 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.vondra@postgresql.org>
Date: Mon, 21 Dec 2020 18:09:57 +0100
Subject: [PATCH] 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
---
 src/backend/optimizer/path/allpaths.c          | 11 ++---------
 src/test/regress/expected/incremental_sort.out | 13 +++++++++++++
 src/test/regress/sql/incremental_sort.sql      |  4 ++++
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 84a69b064a9..a1b3e4b8215 100644
--- 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 7cf2eee7c14..51471ae92de 100644
--- 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 3ee11c394b9..cb48f200ce5 100644
--- 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.