postgres.git / summary / log / commit / refs
commit fac1b470a9f7e846534620b78ea4cf122ed432b2
Author: Tomas Vondra <tomas.vondra@postgresql.org>
Date: Mon Dec 21 17:58:32 2020 +0000
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
src/backend/optimizer/path/equivclass.c | 7 +++++++
src/backend/optimizer/util/tlist.c | 5 -----
src/include/optimizer/optimizer.h | 5 +++++
src/test/regress/expected/incremental_sort.out | 12 ++++++++++++
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 b49ee52cbad..0b2e212b2bc 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -840,6 +840,13 @@ find_em_expr_usable_for_sorting_rel(PlannerInfo *root, EquivalenceClass *ec,
if (require_parallel_safe && !is_parallel_safe(root, (Node *) em_expr))
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,
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 dea0e7338d5..446071c554d 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[]);
[parent: ff5d5611c01f]