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

Commit c066862

Browse files
danolivoCommitfest Bot
authored and
Commitfest Bot
committed
Consider explicit sort of the MergeAppend subpaths.
Expand the optimiser search scope a little: fetching optimal subpath matching pathkeys of the planning MergeAppend, consider the extra case of overall-optimal path plus explicit Sort node at the top. It may provide a more effective plan in both full and fractional scan cases: 1. The Sort node may be pushed down to subpaths under a parallel or async Append. 2. The case when a minor set of subpaths doesn't have a proper index, and it is profitable to sort them instead of switching to plain Append. In general, analysing the regression tests changed by this code, it seems that the cost model prefers a series of small sortings instead of a single massive one. This feature increases a little the number of such paths. Overhead: It seems multiple subpaths may be encountered, as well as many pathkeys. So, to be as careful as possible here, only cost estimation is performed.
1 parent 706054b commit c066862

File tree

10 files changed

+264
-107
lines changed

10 files changed

+264
-107
lines changed

contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10352,13 +10352,15 @@ SELECT t1.a, t2.b FROM fprt1 t1 INNER JOIN fprt2 t2 ON (t1.a = t2.b) WHERE t1.a
1035210352
-> Nested Loop
1035310353
Join Filter: (t1.a = t2.b)
1035410354
-> Append
10355-
-> Foreign Scan on ftprt1_p1 t1_1
10355+
-> Sort
10356+
Sort Key: t1_1.a
10357+
-> Foreign Scan on ftprt1_p1 t1_1
1035610358
-> Foreign Scan on ftprt1_p2 t1_2
1035710359
-> Materialize
1035810360
-> Append
1035910361
-> Foreign Scan on ftprt2_p1 t2_1
1036010362
-> Foreign Scan on ftprt2_p2 t2_2
10361-
(10 rows)
10363+
(12 rows)
1036210364

1036310365
SELECT t1.a, t2.b FROM fprt1 t1 INNER JOIN fprt2 t2 ON (t1.a = t2.b) WHERE t1.a % 25 = 0 ORDER BY 1,2 FOR UPDATE OF t1;
1036410366
a | b

src/backend/optimizer/path/allpaths.c

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1855,29 +1855,18 @@ generate_orderedappend_paths(PlannerInfo *root, RelOptInfo *rel,
18551855

18561856
/* Locate the right paths, if they are available. */
18571857
cheapest_startup =
1858-
get_cheapest_path_for_pathkeys(childrel->pathlist,
1859-
pathkeys,
1860-
NULL,
1861-
STARTUP_COST,
1862-
false);
1858+
get_cheapest_path_for_pathkeys_ext(root, childrel, pathkeys,
1859+
NULL, STARTUP_COST, false);
18631860
cheapest_total =
1864-
get_cheapest_path_for_pathkeys(childrel->pathlist,
1865-
pathkeys,
1866-
NULL,
1867-
TOTAL_COST,
1868-
false);
1861+
get_cheapest_path_for_pathkeys_ext(root, childrel, pathkeys,
1862+
NULL, TOTAL_COST, false);
18691863

18701864
/*
1871-
* If we can't find any paths with the right order just use the
1872-
* cheapest-total path; we'll have to sort it later.
1865+
* In accordance to current planning logic there are no
1866+
* parameterised paths under a merge append.
18731867
*/
1874-
if (cheapest_startup == NULL || cheapest_total == NULL)
1875-
{
1876-
cheapest_startup = cheapest_total =
1877-
childrel->cheapest_total_path;
1878-
/* Assert we do have an unparameterized path for this child */
1879-
Assert(cheapest_total->param_info == NULL);
1880-
}
1868+
Assert(cheapest_startup != NULL && cheapest_total != NULL);
1869+
Assert(cheapest_total->param_info == NULL);
18811870

18821871
/*
18831872
* When building a fractional path, determine a cheapest
@@ -1904,21 +1893,17 @@ generate_orderedappend_paths(PlannerInfo *root, RelOptInfo *rel,
19041893
path_fraction /= childrel->rows;
19051894

19061895
cheapest_fractional =
1907-
get_cheapest_fractional_path_for_pathkeys(childrel->pathlist,
1908-
pathkeys,
1909-
NULL,
1910-
path_fraction);
1896+
get_cheapest_fractional_path_for_pathkeys_ext(root,
1897+
childrel,
1898+
pathkeys,
1899+
NULL,
1900+
path_fraction);
19111901

19121902
/*
1913-
* If we found no path with matching pathkeys, use the
1914-
* cheapest total path instead.
1915-
*
1916-
* XXX We might consider partially sorted paths too (with an
1917-
* incremental sort on top). But we'd have to build all the
1918-
* incremental paths, do the costing etc.
1903+
* In accordance to current planning logic there are no
1904+
* parameterised fractional paths under a merge append.
19191905
*/
1920-
if (!cheapest_fractional)
1921-
cheapest_fractional = cheapest_total;
1906+
Assert(cheapest_fractional != NULL);
19221907
}
19231908

19241909
/*

src/backend/optimizer/path/pathkeys.c

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@
1919

2020
#include "access/stratnum.h"
2121
#include "catalog/pg_opfamily.h"
22+
#include "miscadmin.h"
2223
#include "nodes/nodeFuncs.h"
2324
#include "optimizer/cost.h"
2425
#include "optimizer/optimizer.h"
2526
#include "optimizer/pathnode.h"
2627
#include "optimizer/paths.h"
28+
#include "optimizer/planner.h"
2729
#include "partitioning/partbounds.h"
2830
#include "rewrite/rewriteManip.h"
2931
#include "utils/lsyscache.h"
@@ -648,6 +650,64 @@ get_cheapest_path_for_pathkeys(List *paths, List *pathkeys,
648650
return matched_path;
649651
}
650652

653+
/*
654+
* get_cheapest_path_for_pathkeys_ext
655+
* Calls get_cheapest_path_for_pathkeys to obtain cheapest path that
656+
* satisfies defined criterias and сonsiders one more option: choose
657+
* overall-optimal path (according the criterion) and explicitly sort its
658+
* output to satisfy the pathkeys.
659+
*
660+
* Caller is responsible to insert corresponding sort path at the top of
661+
* returned path if it will be chosen to be used.
662+
*
663+
* Return NULL if no such path.
664+
*/
665+
Path *
666+
get_cheapest_path_for_pathkeys_ext(PlannerInfo *root, RelOptInfo *rel,
667+
List *pathkeys, Relids required_outer,
668+
CostSelector cost_criterion,
669+
bool require_parallel_safe)
670+
{
671+
Path sort_path;
672+
Path *base_path = rel->cheapest_total_path;
673+
Path *path;
674+
675+
/* In generate_orderedappend_paths() all childrels do have some paths */
676+
Assert(base_path);
677+
678+
path = get_cheapest_path_for_pathkeys(rel->pathlist, pathkeys,
679+
required_outer, cost_criterion,
680+
require_parallel_safe);
681+
682+
/*
683+
* Stop here if the cheapest total path doesn't satisfy necessary
684+
* conditions
685+
*/
686+
if ((require_parallel_safe && !base_path->parallel_safe) ||
687+
!bms_is_subset(PATH_REQ_OUTER(base_path), required_outer))
688+
return path;
689+
690+
if (path == NULL)
691+
692+
/*
693+
* Current pathlist doesn't fit the pathkeys. No need to check extra
694+
* sort path ways.
695+
*/
696+
return base_path;
697+
698+
/* Consider the cheapest total path with extra sort */
699+
if (path != base_path)
700+
{
701+
cost_sort(&sort_path, root, pathkeys, base_path->disabled_nodes,
702+
base_path->total_cost, base_path->rows,
703+
base_path->pathtarget->width, 0.0, work_mem, -1.0);
704+
705+
if (compare_path_costs(&sort_path, path, cost_criterion) < 0)
706+
return base_path;
707+
}
708+
return path;
709+
}
710+
651711
/*
652712
* get_cheapest_fractional_path_for_pathkeys
653713
* Find the cheapest path (for retrieving a specified fraction of all
@@ -690,6 +750,61 @@ get_cheapest_fractional_path_for_pathkeys(List *paths,
690750
return matched_path;
691751
}
692752

753+
/*
754+
* get_cheapest_fractional_path_for_pathkeys_ext
755+
* obtain cheapest fractional path that satisfies defined criterias excluding
756+
* pathkeys and explicitly sort its output to satisfy the pathkeys.
757+
*
758+
* Caller is responsible to insert corresponding sort path at the top of
759+
* returned path if it will be chosen to be used.
760+
*
761+
* Return NULL if no such path.
762+
*/
763+
Path *
764+
get_cheapest_fractional_path_for_pathkeys_ext(PlannerInfo *root,
765+
RelOptInfo *rel,
766+
List *pathkeys,
767+
Relids required_outer,
768+
double fraction)
769+
{
770+
Path sort_path;
771+
Path *base_path = rel->cheapest_total_path;
772+
Path *path;
773+
774+
/* In generate_orderedappend_paths() all childrels do have some paths */
775+
Assert(base_path);
776+
777+
path = get_cheapest_fractional_path_for_pathkeys(rel->pathlist, pathkeys,
778+
required_outer, fraction);
779+
780+
/*
781+
* Stop here if the cheapest total path doesn't satisfy necessary
782+
* conditions
783+
*/
784+
if (!bms_is_subset(PATH_REQ_OUTER(base_path), required_outer))
785+
return path;
786+
787+
if (path == NULL)
788+
789+
/*
790+
* Current pathlist doesn't fit the pathkeys. No need to check extra
791+
* sort path ways.
792+
*/
793+
return base_path;
794+
795+
/* Consider the cheapest total path with extra sort */
796+
if (path != base_path)
797+
{
798+
cost_sort(&sort_path, root, pathkeys, base_path->disabled_nodes,
799+
base_path->total_cost, base_path->rows,
800+
base_path->pathtarget->width, 0.0, work_mem, -1.0);
801+
802+
if (compare_fractional_path_costs(&sort_path, path, fraction) <= 0)
803+
return base_path;
804+
}
805+
806+
return path;
807+
}
693808

694809
/*
695810
* get_cheapest_parallel_safe_total_inner

src/include/optimizer/paths.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,10 +224,20 @@ extern Path *get_cheapest_path_for_pathkeys(List *paths, List *pathkeys,
224224
Relids required_outer,
225225
CostSelector cost_criterion,
226226
bool require_parallel_safe);
227+
extern Path *get_cheapest_path_for_pathkeys_ext(PlannerInfo *root,
228+
RelOptInfo *rel, List *pathkeys,
229+
Relids required_outer,
230+
CostSelector cost_criterion,
231+
bool require_parallel_safe);
227232
extern Path *get_cheapest_fractional_path_for_pathkeys(List *paths,
228233
List *pathkeys,
229234
Relids required_outer,
230235
double fraction);
236+
extern Path *get_cheapest_fractional_path_for_pathkeys_ext(PlannerInfo *root,
237+
RelOptInfo *rel,
238+
List *pathkeys,
239+
Relids required_outer,
240+
double fraction);
231241
extern Path *get_cheapest_parallel_safe_total_inner(List *paths);
232242
extern List *build_index_pathkeys(PlannerInfo *root, IndexOptInfo *index,
233243
ScanDirection scandir);

src/test/regress/expected/collate.icu.utf8.out

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2412,16 +2412,19 @@ EXPLAIN (COSTS OFF)
24122412
SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" ORDER BY 1;
24132413
QUERY PLAN
24142414
--------------------------------------------------------
2415-
Sort
2415+
Merge Append
24162416
Sort Key: ((pagg_tab3.c)::text) COLLATE "C"
2417-
-> Append
2417+
-> Sort
2418+
Sort Key: ((pagg_tab3.c)::text) COLLATE "C"
24182419
-> HashAggregate
24192420
Group Key: (pagg_tab3.c)::text
24202421
-> Seq Scan on pagg_tab3_p2 pagg_tab3
2422+
-> Sort
2423+
Sort Key: ((pagg_tab3_1.c)::text) COLLATE "C"
24212424
-> HashAggregate
24222425
Group Key: (pagg_tab3_1.c)::text
24232426
-> Seq Scan on pagg_tab3_p1 pagg_tab3_1
2424-
(9 rows)
2427+
(12 rows)
24252428

24262429
SELECT c collate "C", count(c) FROM pagg_tab3 GROUP BY c collate "C" ORDER BY 1;
24272430
c | count

src/test/regress/expected/inherit.out

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,11 +1665,13 @@ insert into matest2 (name) values ('Test 3');
16651665
insert into matest2 (name) values ('Test 4');
16661666
insert into matest3 (name) values ('Test 5');
16671667
insert into matest3 (name) values ('Test 6');
1668-
set enable_indexscan = off; -- force use of seqscan/sort, so no merge
1668+
set enable_indexscan = off; -- force use of seqscan/sort
1669+
set enable_sort = off; -- since merge append may employ sort in children we need to disable sort
16691670
explain (verbose, costs off) select * from matest0 order by 1-id;
16701671
QUERY PLAN
16711672
------------------------------------------------------------
16721673
Sort
1674+
Disabled: true
16731675
Output: matest0.id, matest0.name, ((1 - matest0.id))
16741676
Sort Key: ((1 - matest0.id))
16751677
-> Result
@@ -1683,7 +1685,7 @@ explain (verbose, costs off) select * from matest0 order by 1-id;
16831685
Output: matest0_3.id, matest0_3.name
16841686
-> Seq Scan on public.matest3 matest0_4
16851687
Output: matest0_4.id, matest0_4.name
1686-
(14 rows)
1688+
(15 rows)
16871689

16881690
select * from matest0 order by 1-id;
16891691
id | name
@@ -1719,6 +1721,7 @@ select min(1-id) from matest0;
17191721
(1 row)
17201722

17211723
reset enable_indexscan;
1724+
reset enable_sort;
17221725
set enable_seqscan = off; -- plan with fewest seqscans should be merge
17231726
set enable_parallel_append = off; -- Don't let parallel-append interfere
17241727
explain (verbose, costs off) select * from matest0 order by 1-id;
@@ -1844,16 +1847,20 @@ order by t1.b limit 10;
18441847
Merge Cond: (t1.b = t2.b)
18451848
-> Merge Append
18461849
Sort Key: t1.b
1847-
-> Index Scan using matest0i on matest0 t1_1
1850+
-> Sort
1851+
Sort Key: t1_1.b
1852+
-> Seq Scan on matest0 t1_1
18481853
-> Index Scan using matest1i on matest1 t1_2
18491854
-> Materialize
18501855
-> Merge Append
18511856
Sort Key: t2.b
1852-
-> Index Scan using matest0i on matest0 t2_1
1853-
Filter: (c = d)
1857+
-> Sort
1858+
Sort Key: t2_1.b
1859+
-> Seq Scan on matest0 t2_1
1860+
Filter: (c = d)
18541861
-> Index Scan using matest1i on matest1 t2_2
18551862
Filter: (c = d)
1856-
(14 rows)
1863+
(18 rows)
18571864

18581865
reset enable_nestloop;
18591866
drop table matest0 cascade;

0 commit comments

Comments
 (0)