🌐 AI搜索 & 代理 主页
Skip to content

Commit be9c3cd

Browse files
tvondratvondra
committed
Check parallel safety in generate_useful_gather_paths
Commit ebb7ae839d ensured we ignore pathkeys with volatile expressions when considering adding a sort below a Gather Merge. Turns out we need to care about parallel safety of the pathkeys too, otherwise we might try sorting e.g. on results of a correlated subquery (as demonstrated by a report from Luis Roberto). Initial investigation by Tom Lane, patch by James Coleman. Backpatch to 13, where the code was instroduced (as part of Incremental Sort). Reported-by: Luis Roberto Author: James Coleman Reviewed-by: Tomas Vondra Backpatch-through: 13 Discussion: https://postgr.es/m/622580997.37108180.1604080457319.JavaMail.zimbra%40siscobra.com.br Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
1 parent ea190ed commit be9c3cd

File tree

5 files changed

+73
-5
lines changed

5 files changed

+73
-5
lines changed

‎src/backend/optimizer/path/allpaths.c‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2742,14 +2742,18 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows)
27422742
* This allows us to do incremental sort on top of an index scan under a gather
27432743
* merge node, i.e. parallelized.
27442744
*
2745+
* If the require_parallel_safe is true, we also require the expressions to
2746+
* be parallel safe (which allows pushing the sort below Gather Merge).
2747+
*
27452748
* XXX At the moment this can only ever return a list with a single element,
27462749
* because it looks at query_pathkeys only. So we might return the pathkeys
27472750
* directly, but it seems plausible we'll want to consider other orderings
27482751
* in the future. For example, we might want to consider pathkeys useful for
27492752
* merge joins.
27502753
*/
27512754
static List *
2752-
get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
2755+
get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel,
2756+
bool require_parallel_safe)
27532757
{
27542758
List *useful_pathkeys_list = NIL;
27552759

@@ -2779,8 +2783,11 @@ get_useful_pathkeys_for_relation(PlannerInfo *root, RelOptInfo *rel)
27792783
* meet criteria of EC membership in the current relation, we
27802784
* enable not just an incremental sort on the entirety of
27812785
* query_pathkeys but also incremental sort below a JOIN.
2786+
*
2787+
* If requested, ensure the expression is parallel safe too.
27822788
*/
2783-
if (!find_em_expr_usable_for_sorting_rel(pathkey_ec, rel))
2789+
if (!find_em_expr_usable_for_sorting_rel(root, pathkey_ec, rel,
2790+
require_parallel_safe))
27842791
break;
27852792

27862793
npathkeys++;
@@ -2834,7 +2841,7 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r
28342841
generate_gather_paths(root, rel, override_rows);
28352842

28362843
/* consider incremental sort for interesting orderings */
2837-
useful_pathkeys_list = get_useful_pathkeys_for_relation(root, rel);
2844+
useful_pathkeys_list = get_useful_pathkeys_for_relation(root, rel, true);
28382845

28392846
/* used for explicit (full) sort paths */
28402847
cheapest_partial_path = linitial(rel->partial_pathlist);

‎src/backend/optimizer/path/equivclass.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -800,7 +800,8 @@ find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel)
800800
* applied in prepare_sort_from_pathkeys.
801801
*/
802802
Expr *
803-
find_em_expr_usable_for_sorting_rel(EquivalenceClass *ec, RelOptInfo *rel)
803+
find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec,
804+
RelOptInfo *rel, bool require_parallel_safe)
804805
{
805806
ListCell *lc_em;
806807

@@ -830,6 +831,12 @@ find_em_expr_usable_for_sorting_rel(EquivalenceClass *ec, RelOptInfo *rel)
830831
if (!bms_is_subset(em->em_relids, rel->relids))
831832
continue;
832833

834+
/*
835+
* If requested, reject expressions that are not parallel-safe.
836+
*/
837+
if (require_parallel_safe && !is_parallel_safe(root, (Node *) em_expr))
838+
continue;
839+
833840
/*
834841
* As long as the expression isn't volatile then
835842
* prepare_sort_from_pathkeys is able to generate a new target entry,

‎src/include/optimizer/paths.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,10 @@ extern EquivalenceClass *get_eclass_for_sort_expr(PlannerInfo *root,
135135
Relids rel,
136136
bool create_it);
137137
extern Expr *find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel);
138-
extern Expr *find_em_expr_usable_for_sorting_rel(EquivalenceClass *ec, RelOptInfo *rel);
138+
extern Expr *find_em_expr_usable_for_sorting_rel(PlannerInfo *root,
139+
EquivalenceClass *ec,
140+
RelOptInfo *rel,
141+
bool require_parallel_safe);
139142
extern void generate_base_implied_equalities(PlannerInfo *root);
140143
extern List *generate_join_implied_equalities(PlannerInfo *root,
141144
Relids join_relids,

‎src/test/regress/expected/incremental_sort.out‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,6 +1551,46 @@ order by 1, 2;
15511551
-> Function Scan on generate_series
15521552
(7 rows)
15531553

1554+
-- Parallel sort but with expression (correlated subquery) that
1555+
-- is prohibited in parallel plans.
1556+
explain (costs off) select distinct
1557+
unique1,
1558+
(select t.unique1 from tenk1 where tenk1.unique1 = t.unique1)
1559+
from tenk1 t, generate_series(1, 1000);
1560+
QUERY PLAN
1561+
---------------------------------------------------------------------------------
1562+
Unique
1563+
-> Sort
1564+
Sort Key: t.unique1, ((SubPlan 1))
1565+
-> Gather
1566+
Workers Planned: 2
1567+
-> Nested Loop
1568+
-> Parallel Index Only Scan using tenk1_unique1 on tenk1 t
1569+
-> Function Scan on generate_series
1570+
SubPlan 1
1571+
-> Index Only Scan using tenk1_unique1 on tenk1
1572+
Index Cond: (unique1 = t.unique1)
1573+
(11 rows)
1574+
1575+
explain (costs off) select
1576+
unique1,
1577+
(select t.unique1 from tenk1 where tenk1.unique1 = t.unique1)
1578+
from tenk1 t, generate_series(1, 1000)
1579+
order by 1, 2;
1580+
QUERY PLAN
1581+
---------------------------------------------------------------------------
1582+
Sort
1583+
Sort Key: t.unique1, ((SubPlan 1))
1584+
-> Gather
1585+
Workers Planned: 2
1586+
-> Nested Loop
1587+
-> Parallel Index Only Scan using tenk1_unique1 on tenk1 t
1588+
-> Function Scan on generate_series
1589+
SubPlan 1
1590+
-> Index Only Scan using tenk1_unique1 on tenk1
1591+
Index Cond: (unique1 = t.unique1)
1592+
(10 rows)
1593+
15541594
-- Parallel sort but with expression not available until the upper rel.
15551595
explain (costs off) select distinct sub.unique1, stringu1 || random()::text
15561596
from tenk1, lateral (select tenk1.unique1 from generate_series(1, 1000)) as sub;

‎src/test/regress/sql/incremental_sort.sql‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,17 @@ from tenk1, lateral (select tenk1.unique1 from generate_series(1, 1000)) as sub;
250250
explain (costs off) select sub.unique1, md5(stringu1)
251251
from tenk1, lateral (select tenk1.unique1 from generate_series(1, 1000)) as sub
252252
order by 1, 2;
253+
-- Parallel sort but with expression (correlated subquery) that
254+
-- is prohibited in parallel plans.
255+
explain (costs off) select distinct
256+
unique1,
257+
(select t.unique1 from tenk1 where tenk1.unique1 = t.unique1)
258+
from tenk1 t, generate_series(1, 1000);
259+
explain (costs off) select
260+
unique1,
261+
(select t.unique1 from tenk1 where tenk1.unique1 = t.unique1)
262+
from tenk1 t, generate_series(1, 1000)
263+
order by 1, 2;
253264
-- Parallel sort but with expression not available until the upper rel.
254265
explain (costs off) select distinct sub.unique1, stringu1 || random()::text
255266
from tenk1, lateral (select tenk1.unique1 from generate_series(1, 1000)) as sub;

0 commit comments

Comments
 (0)