Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit ea190ed

Browse files
committed
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
1 parent bd6939a commit ea190ed

File tree

3 files changed

+19
-9
lines changed

3 files changed

+19
-9
lines changed

src/backend/optimizer/path/allpaths.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2840,7 +2840,8 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
28402840
cheapest_partial_path = linitial(rel->partial_pathlist);
28412841

28422842
/*
2843-
* Consider incremental sort paths for each interesting ordering.
2843+
* Consider sorted paths for each interesting ordering. We generate both
2844+
* incremental and full sort.
28442845
*/
28452846
foreach(lc, useful_pathkeys_list)
28462847
{
@@ -2854,14 +2855,6 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
28542855
Path *subpath = (Path *) lfirst(lc2);
28552856
GatherMergePath *path;
28562857

2857-
/*
2858-
* If the path has no ordering at all, then we can't use either
2859-
* incremental sort or rely on implict sorting with a gather
2860-
* merge.
2861-
*/
2862-
if (subpath->pathkeys == NIL)
2863-
continue;
2864-
28652858
is_sorted = pathkeys_count_contained_in(useful_pathkeys,
28662859
subpath->pathkeys,
28672860
&presorted_keys);

src/test/regress/expected/incremental_sort.out

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,6 +1468,19 @@ explain (costs off) select * from t union select * from t order by 1,3;
14681468
-> Parallel Seq Scan on t t_1
14691469
(13 rows)
14701470

1471+
-- Full sort, not just incremental sort can be pushed below a gather merge path
1472+
-- by generate_useful_gather_paths.
1473+
explain (costs off) select distinct a,b from t;
1474+
QUERY PLAN
1475+
------------------------------------------
1476+
Unique
1477+
-> Gather Merge
1478+
Workers Planned: 2
1479+
-> Sort
1480+
Sort Key: a, b
1481+
-> Parallel Seq Scan on t
1482+
(6 rows)
1483+
14711484
drop table t;
14721485
-- Sort pushdown can't go below where expressions are part of the rel target.
14731486
-- In particular this is interesting for volatile expressions which have to

src/test/regress/sql/incremental_sort.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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
220220
set enable_hashagg to off;
221221
explain (costs off) select * from t union select * from t order by 1,3;
222222

223+
-- Full sort, not just incremental sort can be pushed below a gather merge path
224+
-- by generate_useful_gather_paths.
225+
explain (costs off) select distinct a,b from t;
226+
223227
drop table t;
224228

225229
-- Sort pushdown can't go below where expressions are part of the rel target.

0 commit comments

Comments
 (0)