diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 18424f4b81d..4aef57aae58 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -138,10 +138,10 @@ typedef struct /* Local functions */ static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind); -static void preprocess_subquery_phvs(PlannerInfo *root); +static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode); +static void preprocess_subquery_phvs(PlannerInfo *root, Node *node); static bool preprocess_subquery_phvs_walker(Node *node, preprocess_subquery_phvs_context *context); -static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode); static void grouping_planner(PlannerInfo *root, double tuple_fraction); static grouping_sets_data *preprocess_grouping_sets(PlannerInfo *root); static List *remap_to_groupclause_idx(List *groupClause, List *gsets, @@ -830,14 +830,6 @@ subquery_planner(PlannerGlobal *glob, Query *parse, */ root->hasHavingQual = (parse->havingQual != NULL); - /* - * Preprocess any PlaceHolderVars of our level that were pushed down into - * subqueries. This must happen before anything can consume those copies, - * in particular before SubLinks below are turned into SubPlans. - */ - if (root->glob->lastPHId != 0) - preprocess_subquery_phvs(root); - /* * Do expression preprocessing on targetlist and quals, as well as other * random expressions in the querytree. Note that we do not need to @@ -959,6 +951,14 @@ subquery_planner(PlannerGlobal *glob, Query *parse, rte->subquery = (Query *) flatten_join_alias_vars(root, root->parse, (Node *) rte->subquery); + + /* + * Likewise for copies of our PlaceHolderVars in the subquery. + * This must be done after the alias expansion above, which can + * insert such copies. + */ + if (rte->lateral && root->glob->lastPHId != 0) + preprocess_subquery_phvs(root, (Node *) rte->subquery); } else if (rte->rtekind == RTE_FUNCTION) { @@ -1203,6 +1203,15 @@ preprocess_expression(PlannerInfo *root, Node *expr, int kind) convert_saop_to_hashed_saop(expr); } + /* + * Preprocess any copies of our PlaceHolderVars within SubLink subselects. + * This must be done after join alias expansion, which can insert such + * copies, and before the SubLinks are turned into SubPlans, which collect + * those copies as SubPlan arguments. + */ + if (root->parse->hasSubLinks && root->glob->lastPHId != 0) + preprocess_subquery_phvs(root, expr); + /* Expand SubLinks to SubPlans */ if (root->parse->hasSubLinks) expr = SS_process_sublinks(root, expr, (kind == EXPRKIND_QUAL)); @@ -1286,24 +1295,25 @@ preprocess_phv_expression(PlannerInfo *root, Expr *expr) /* * preprocess_subquery_phvs * Preprocess copies of this level's PlaceHolderVars that were pushed - * down into subqueries. + * down into subqueries within the given tree. * * When a subquery (a LATERAL RTE or a SubLink's subselect) references a * pulled-up output that must be wrapped in a PlaceHolderVar, the PHV * expression is pushed down into the subquery. The subquery's own planning * leaves that copy alone, since it belongs to our level, so we need to * preprocess it. We modify the PHVs in place, temporarily adjusting each to - * our level, and handle nested copies innermost-first. + * our level. Preprocessing a copy's expression takes care of everything + * within it, including any further copies nested inside SubLinks there, so + * we don't look inside a copy ourselves. */ static void -preprocess_subquery_phvs(PlannerInfo *root) +preprocess_subquery_phvs(PlannerInfo *root, Node *node) { preprocess_subquery_phvs_context context; context.root = root; context.sublevels_up = 0; - (void) query_tree_walker(root->parse, preprocess_subquery_phvs_walker, - &context, 0); + (void) preprocess_subquery_phvs_walker(node, &context); } static bool @@ -1327,9 +1337,9 @@ preprocess_subquery_phvs_walker(Node *node, { PlaceHolderVar *phv = (PlaceHolderVar *) node; - /* Handle any nested copies within the expression first */ - (void) expression_tree_walker(node, preprocess_subquery_phvs_walker, - context); + /* A PHV of an upper level can't contain anything of our level */ + if (phv->phlevelsup > context->sublevels_up) + return false; /* * Is this a copy of one of our PHVs that is pushed down into a @@ -1347,8 +1357,10 @@ preprocess_subquery_phvs_walker(Node *node, expr = preprocess_expression(context->root, expr, EXPRKIND_PHV); IncrementVarSublevelsUp(expr, levelsup, 0); phv->phexpr = (Expr *) expr; + return false; } - return false; + + /* Otherwise, it's ours or a lower level's; look inside it */ } return expression_tree_walker(node, preprocess_subquery_phvs_walker, context); diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 3c40362e483..d4fdcab5c0b 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -1962,9 +1962,10 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context) /* * Don't recurse into the arguments of an outer PHV, Aggref or * GroupingFunc here. Any SubLinks in the arguments have to be dealt with - * at the outer query level; they'll be handled when build_subplan - * collects the PHV, Aggref or GroupingFunc into the arguments to be - * passed down to the current subplan. + * at the outer query level; for an Aggref or GroupingFunc, they'll be + * handled when build_subplan collects it into the arguments to be passed + * down to the current subplan, while an outer PHV's expression has + * already been preprocessed by its owning level. */ if (IsA(node, PlaceHolderVar)) { diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c index 110d8c7f8db..6600ccb9eca 100644 --- a/src/backend/optimizer/util/var.c +++ b/src/backend/optimizer/util/var.c @@ -868,11 +868,10 @@ flatten_join_alias_vars_mutator(Node *node, return node; /* no need to copy, really */ /* - * A pushed-down copy of a target-level PHV was already preprocessed - * by preprocess_subquery_phvs, so it may contain SubPlans and holds - * no unflattened target-level aliases; shallow-copy it instead of - * recursing. Any other PHV is recursed into, as it may still hold - * target-level aliases. + * A pushed-down copy of a target-level PHV is preprocessed by + * preprocess_subquery_phvs, which handles its target-level aliases + * and may leave SubPlans in it; so don't recurse into it. Any other + * PHV may still hold target-level aliases, so recurse. */ if (phv->phlevelsup == context->sublevels_up && context->sublevels_up > 0) diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 47f48b38f7e..37ac979e74e 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5854,24 +5854,24 @@ from (select case when false then remov.id else (select i41.f1) end as c1 QUERY PLAN --------------------------------------------------------- Nested Loop - Output: ((SubPlan 2)), (((SubPlan 2))) + Output: ((SubPlan 1)), (((SubPlan 1))) -> Nested Loop Left Join - Output: ((SubPlan 2)) + Output: ((SubPlan 1)) Join Filter: false -> Seq Scan on public.int4_tbl i42 Output: i42.f1 -> Result - Output: (SubPlan 2) + Output: (SubPlan 1) One-Time Filter: false - SubPlan 2 + SubPlan 1 -> Result Output: f1 -> Memoize - Output: (((SubPlan 2))) - Cache Key: ((SubPlan 2)) + Output: (((SubPlan 1))) + Cache Key: ((SubPlan 1)) Cache Mode: binary -> Nested Loop - Output: ((SubPlan 2)) + Output: ((SubPlan 1)) -> Seq Scan on public.int4_tbl i43 Output: i43.f1 -> Materialize @@ -5926,6 +5926,134 @@ from (select case when false then remov.id else (select i41.f1) end as c1 Output: ((SubPlan 1)) (14 rows) +-- likewise, where the pushed-down PHV's expression contains another PHV of +-- the same level, which must not be preprocessed separately from its parent +explain (verbose, costs off) +select ss3.c2 +from int4_tbl i41 + left join (select coalesce(ss1.c1, 0) as c1 + from int4_tbl i42 + left join (select (select i43.f1) as c1 + from int4_tbl i43) ss1 on true) ss2 + on true, + lateral (select ss2.c1 as c2 from int4_tbl i44 offset 0) ss3; + QUERY PLAN +--------------------------------------------------------------- + Nested Loop + Output: ((COALESCE(((SubPlan 1)), 0))) + -> Nested Loop Left Join + Output: (COALESCE(((SubPlan 1)), 0)) + -> Seq Scan on public.int4_tbl i41 + Output: i41.f1 + -> Materialize + Output: (COALESCE(((SubPlan 1)), 0)) + -> Nested Loop Left Join + Output: COALESCE(((SubPlan 1)), 0) + -> Seq Scan on public.int4_tbl i42 + Output: i42.f1 + -> Materialize + Output: ((SubPlan 1)) + -> Seq Scan on public.int4_tbl i43 + Output: (SubPlan 1) + SubPlan 1 + -> Result + Output: i43.f1 + -> Memoize + Output: ((COALESCE(((SubPlan 1)), 0))) + Cache Key: (COALESCE(((SubPlan 1)), 0)) + Cache Mode: binary + -> Seq Scan on public.int4_tbl i44 + Output: (COALESCE(((SubPlan 1)), 0)) +(25 rows) + +-- likewise, where the PHV copy is only inserted into the LATERAL subquery by +-- expanding a join alias Var of the outer level +explain (verbose, costs off) +select ss2.c2 +from ((select (select i41.f1) as c1 from int4_tbl i41) ss1 + full join int4_tbl i42(c1) using (c1)) j, + lateral (select j.c1 as c2 from int4_tbl i43 offset 0) ss2; + QUERY PLAN +--------------------------------------------------- + Nested Loop + Output: (COALESCE(((SubPlan 1)), i42.c1)) + -> Hash Full Join + Output: ((SubPlan 1)), i42.c1 + Hash Cond: (((SubPlan 1)) = i42.c1) + -> Seq Scan on public.int4_tbl i41 + Output: (SubPlan 1) + SubPlan 1 + -> Result + Output: i41.f1 + -> Hash + Output: i42.c1 + -> Seq Scan on public.int4_tbl i42 + Output: i42.c1 + -> Seq Scan on public.int4_tbl i43 + Output: COALESCE(((SubPlan 1)), i42.c1) +(16 rows) + +-- likewise, where the join alias is expanded within a SubLink's subselect +explain (verbose, costs off) +select (select j.c1 from int4_tbl i43 offset 0) as c2 +from ((select (select i41.f1) as c1 from int4_tbl i41) ss1 + full join int4_tbl i42(c1) using (c1)) j; + QUERY PLAN +--------------------------------------------------- + Hash Full Join + Output: (SubPlan 2) + Hash Cond: (((SubPlan 1)) = i42.c1) + -> Seq Scan on public.int4_tbl i41 + Output: (SubPlan 1) + SubPlan 1 + -> Result + Output: i41.f1 + -> Hash + Output: i42.c1 + -> Seq Scan on public.int4_tbl i42 + Output: i42.c1 + SubPlan 2 + -> Seq Scan on public.int4_tbl i43 + Output: COALESCE(((SubPlan 1)), i42.c1) +(15 rows) + +-- likewise, where the SubLink holding the copy ends up in the translated +-- Vars of an appendrel child pulled up from a LATERAL UNION ALL subquery +explain (verbose, costs off) +select ss2.c2 +from (select case when false then remov.id end as c1 + from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1 + right join int4_tbl i42 on true, + lateral ((select (select ss1.c1) as c2) union all (select (select ss1.c1))) ss2; + QUERY PLAN +----------------------------------------------------- + Nested Loop + Output: ((SubPlan 1)) + -> Nested Loop Left Join + Output: (NULL::integer) + -> Seq Scan on public.int4_tbl i42 + Output: i42.f1 + -> Materialize + Output: (NULL::integer) + -> Seq Scan on public.int4_tbl i41 + Output: NULL::integer + -> Memoize + Output: ((SubPlan 1)) + Cache Key: (NULL::integer), (NULL::integer) + Cache Mode: binary + -> Append + -> Result + Output: (SubPlan 1) + SubPlan 1 + -> Result + Output: (NULL::integer) + -> Result + Output: (SubPlan 2) + SubPlan 2 + -> Result + Output: (NULL::integer) +(25 rows) + -- More tests of correct placement of pseudoconstant quals -- simple constant-false condition explain (costs off) diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 2171a8f2bd6..fc984fff672 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2164,6 +2164,41 @@ from (select case when false then remov.id else (select i41.f1) end as c1 from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1 right join int4_tbl i42 on true; +-- likewise, where the pushed-down PHV's expression contains another PHV of +-- the same level, which must not be preprocessed separately from its parent +explain (verbose, costs off) +select ss3.c2 +from int4_tbl i41 + left join (select coalesce(ss1.c1, 0) as c1 + from int4_tbl i42 + left join (select (select i43.f1) as c1 + from int4_tbl i43) ss1 on true) ss2 + on true, + lateral (select ss2.c1 as c2 from int4_tbl i44 offset 0) ss3; + +-- likewise, where the PHV copy is only inserted into the LATERAL subquery by +-- expanding a join alias Var of the outer level +explain (verbose, costs off) +select ss2.c2 +from ((select (select i41.f1) as c1 from int4_tbl i41) ss1 + full join int4_tbl i42(c1) using (c1)) j, + lateral (select j.c1 as c2 from int4_tbl i43 offset 0) ss2; + +-- likewise, where the join alias is expanded within a SubLink's subselect +explain (verbose, costs off) +select (select j.c1 from int4_tbl i43 offset 0) as c2 +from ((select (select i41.f1) as c1 from int4_tbl i41) ss1 + full join int4_tbl i42(c1) using (c1)) j; + +-- likewise, where the SubLink holding the copy ends up in the translated +-- Vars of an appendrel child pulled up from a LATERAL UNION ALL subquery +explain (verbose, costs off) +select ss2.c2 +from (select case when false then remov.id end as c1 + from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1 + right join int4_tbl i42 on true, + lateral ((select (select ss1.c1) as c2) union all (select (select ss1.c1))) ss2; + -- More tests of correct placement of pseudoconstant quals -- simple constant-false condition