diff options
author | Tomas Vondra <tomas.vondra@postgresql.org> | 2020-12-21 18:58:32 +0100 |
---|---|---|
committer | Tomas Vondra <tomas.vondra@postgresql.org> | 2020-12-21 19:36:03 +0100 |
commit | d0167631e8b7388b78203c6798621f98beed93d5 (patch) | |
tree | 81259e3de996059f7f1e11126263e40fdf4b4ece | |
parent | 38d30a14b05e0cc2996fd311d94d7ae4fe2122aa (diff) | |
download | postgresql-d0167631e8b7388b78203c6798621f98beed93d5.tar.gz postgresql-d0167631e8b7388b78203c6798621f98beed93d5.zip |
Disallow SRFs when considering sorts below Gather Merge
While we do allow SRFs in ORDER BY, scan/join processing should not
consider such cases - such sorts should only happen via final Sort atop
a ProjectSet. So make sure we don't try adding such sorts below Gather
Merge, just like we do for expressions that are volatile and/or not
parallel safe.
Backpatch to PostgreSQL 13, where this code was introduced as part of
the Incremental Sort patch.
Author: James Coleman
Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
Discussion: https://postgr.es/m/295524.1606246314%40sss.pgh.pa.us
-rw-r--r-- | src/backend/optimizer/path/equivclass.c | 7 | ||||
-rw-r--r-- | src/backend/optimizer/util/tlist.c | 5 | ||||
-rw-r--r-- | src/include/optimizer/optimizer.h | 5 | ||||
-rw-r--r-- | src/test/regress/expected/incremental_sort.out | 12 | ||||
-rw-r--r-- | src/test/regress/sql/incremental_sort.sql | 2 |
5 files changed, 26 insertions, 5 deletions
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index a0a4699c813..344148aa56c 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -838,6 +838,13 @@ find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec, continue; /* + * Disallow SRFs so that all of them can be evaluated at the correct + * time as determined by make_sort_input_target. + */ + if (IS_SRF_CALL((Node *) em_expr)) + continue; + + /* * As long as the expression isn't volatile then * prepare_sort_from_pathkeys is able to generate a new target entry, * so there's no need to verify that one already exists. diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c index 02a3c6b1656..01cea102eab 100644 --- a/src/backend/optimizer/util/tlist.c +++ b/src/backend/optimizer/util/tlist.c @@ -21,11 +21,6 @@ #include "optimizer/tlist.h" -/* Test if an expression node represents a SRF call. Beware multiple eval! */ -#define IS_SRF_CALL(node) \ - ((IsA(node, FuncExpr) && ((FuncExpr *) (node))->funcretset) || \ - (IsA(node, OpExpr) && ((OpExpr *) (node))->opretset)) - /* * Data structures for split_pathtarget_at_srfs(). To preserve the identity * of sortgroupref items even if they are textually equal(), what we track is diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h index 3e4171056e8..1e135652b97 100644 --- a/src/include/optimizer/optimizer.h +++ b/src/include/optimizer/optimizer.h @@ -24,6 +24,11 @@ #include "nodes/parsenodes.h" +/* Test if an expression node represents a SRF call. Beware multiple eval! */ +#define IS_SRF_CALL(node) \ + ((IsA(node, FuncExpr) && ((FuncExpr *) (node))->funcretset) || \ + (IsA(node, OpExpr) && ((OpExpr *) (node))->opretset)) + /* * We don't want to include nodes/pathnodes.h here, because non-planner * code should generally treat PlannerInfo as an opaque typedef. diff --git a/src/test/regress/expected/incremental_sort.out b/src/test/regress/expected/incremental_sort.out index d316a7c4b90..a8cbfd9f5f9 100644 --- a/src/test/regress/expected/incremental_sort.out +++ b/src/test/regress/expected/incremental_sort.out @@ -1620,3 +1620,15 @@ order by 1, 2; -> Function Scan on generate_series (7 rows) +-- Disallow pushing down sort when pathkey is an SRF. +explain (costs off) select unique1 from tenk1 order by unnest('{1,2}'::int[]); + QUERY PLAN +------------------------------------------------------------------------- + Sort + Sort Key: (unnest('{1,2}'::integer[])) + -> Gather + Workers Planned: 2 + -> ProjectSet + -> Parallel Index Only Scan using tenk1_unique1 on tenk1 +(6 rows) + diff --git a/src/test/regress/sql/incremental_sort.sql b/src/test/regress/sql/incremental_sort.sql index ff3af0fd165..62a037b0cfb 100644 --- a/src/test/regress/sql/incremental_sort.sql +++ b/src/test/regress/sql/incremental_sort.sql @@ -267,3 +267,5 @@ from tenk1, lateral (select tenk1.unique1 from generate_series(1, 1000)) as sub; explain (costs off) select sub.unique1, stringu1 || random()::text from tenk1, lateral (select tenk1.unique1 from generate_series(1, 1000)) as sub order by 1, 2; +-- Disallow pushing down sort when pathkey is an SRF. +explain (costs off) select unique1 from tenk1 order by unnest('{1,2}'::int[]); |