diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 7b11067090..098c23d2c7 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -822,8 +822,8 @@ contain_subplans_walker(Node *node, void *context) * mistakenly think that something like "WHERE random() < 0.5" can be treated * as a constant qualification. * - * XXX we do not examine sub-selects to see if they contain uses of - * mutable functions. It's not real clear if that is correct or not... + * We will recursively look into Query nodes (i.e., SubLink sub-selects) + * but not into SubPlans. See comments for contain_volatile_functions(). */ bool contain_mutable_functions(Node *clause) @@ -920,6 +920,13 @@ contain_mutable_functions_walker(Node *node, void *context) } /* else fall through to check args */ } + else if (IsA(node, Query)) + { + /* Recurse into subselects */ + return query_tree_walker((Query *) node, + contain_mutable_functions_walker, + context, 0); + } return expression_tree_walker(node, contain_mutable_functions_walker, context); } @@ -934,11 +941,18 @@ contain_mutable_functions_walker(Node *node, void *context) * Recursively search for volatile functions within a clause. * * Returns true if any volatile function (or operator implemented by a - * volatile function) is found. This test prevents invalid conversions - * of volatile expressions into indexscan quals. + * volatile function) is found. This test prevents, for example, + * invalid conversions of volatile expressions into indexscan quals. * - * XXX we do not examine sub-selects to see if they contain uses of - * volatile functions. It's not real clear if that is correct or not... + * We will recursively look into Query nodes (i.e., SubLink sub-selects) + * but not into SubPlans. This is a bit odd, but intentional. If we are + * looking at a SubLink, we are probably deciding whether a query tree + * transformation is safe, and a contained sub-select should affect that; + * for example, duplicating a sub-select containing a volatile function + * would be bad. However, once we've got to the stage of having SubPlans, + * subsequent planning need not consider volatility within those, since + * the executor won't change its evaluation rules for a SubPlan based on + * volatility. */ bool contain_volatile_functions(Node *clause) @@ -1036,6 +1050,13 @@ contain_volatile_functions_walker(Node *node, void *context) } /* else fall through to check args */ } + else if (IsA(node, Query)) + { + /* Recurse into subselects */ + return query_tree_walker((Query *) node, + contain_volatile_functions_walker, + context, 0); + } return expression_tree_walker(node, contain_volatile_functions_walker, context); } diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index 8bda3039d6..1baf3d3e23 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -636,3 +636,67 @@ where a.thousand = b.thousand ---------- (0 rows) +-- +-- Check that nested sub-selects are not pulled up if they contain volatiles +-- +explain (verbose, costs off) + select x, x from + (select (select now()) as x from (values(1),(2)) v(y)) ss; + QUERY PLAN +--------------------------- + Values Scan on "*VALUES*" + Output: $0, $1 + InitPlan 1 (returns $0) + -> Result + Output: now() + InitPlan 2 (returns $1) + -> Result + Output: now() +(8 rows) + +explain (verbose, costs off) + select x, x from + (select (select random()) as x from (values(1),(2)) v(y)) ss; + QUERY PLAN +---------------------------------- + Subquery Scan on ss + Output: ss.x, ss.x + -> Values Scan on "*VALUES*" + Output: $0 + InitPlan 1 (returns $0) + -> Result + Output: random() +(7 rows) + +explain (verbose, costs off) + select x, x from + (select (select now() where y=y) as x from (values(1),(2)) v(y)) ss; + QUERY PLAN +---------------------------------------------------------------------- + Values Scan on "*VALUES*" + Output: (SubPlan 1), (SubPlan 2) + SubPlan 1 + -> Result + Output: now() + One-Time Filter: ("*VALUES*".column1 = "*VALUES*".column1) + SubPlan 2 + -> Result + Output: now() + One-Time Filter: ("*VALUES*".column1 = "*VALUES*".column1) +(10 rows) + +explain (verbose, costs off) + select x, x from + (select (select random() where y=y) as x from (values(1),(2)) v(y)) ss; + QUERY PLAN +---------------------------------------------------------------------------- + Subquery Scan on ss + Output: ss.x, ss.x + -> Values Scan on "*VALUES*" + Output: (SubPlan 1) + SubPlan 1 + -> Result + Output: random() + One-Time Filter: ("*VALUES*".column1 = "*VALUES*".column1) +(8 rows) + diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 8a55474b54..0795d43534 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -389,3 +389,19 @@ where a.thousand = b.thousand and exists ( select 1 from tenk1 c where b.hundred = c.hundred and not exists ( select 1 from tenk1 d where a.thousand = d.thousand ) ); + +-- +-- Check that nested sub-selects are not pulled up if they contain volatiles +-- +explain (verbose, costs off) + select x, x from + (select (select now()) as x from (values(1),(2)) v(y)) ss; +explain (verbose, costs off) + select x, x from + (select (select random()) as x from (values(1),(2)) v(y)) ss; +explain (verbose, costs off) + select x, x from + (select (select now() where y=y) as x from (values(1),(2)) v(y)) ss; +explain (verbose, costs off) + select x, x from + (select (select random() where y=y) as x from (values(1),(2)) v(y)) ss;