diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 90daee7de1e..5f99ad3cdc4 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -153,13 +153,13 @@ typedef struct /* Local functions */ static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind); -static void preprocess_subquery_phvs(PlannerInfo *root); -static bool preprocess_subquery_phvs_walker(Node *node, - preprocess_subquery_phvs_context *context); static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode); static Bitmapset *find_having_conflicts(Query *parse, Index group_rtindex); static Oid having_var_grouping_eqop(Var *var, void *context); static Oid group_var_eqop(Query *parse, Var *var); +static void preprocess_subquery_phvs(PlannerInfo *root, Node *node); +static bool preprocess_subquery_phvs_walker(Node *node, + preprocess_subquery_phvs_context *context); static void grouping_planner(PlannerInfo *root, double tuple_fraction, SetOperationStmt *setops); static grouping_sets_data *preprocess_grouping_sets(PlannerInfo *root); @@ -1004,14 +1004,6 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, */ 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 @@ -1129,6 +1121,14 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, 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) { @@ -1463,6 +1463,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)); @@ -1625,24 +1634,25 @@ group_var_eqop(Query *parse, Var *var) /* * 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 @@ -1666,9 +1676,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 @@ -1686,8 +1696,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 ae7c489b432..5760b616813 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -2255,9 +2255,10 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context) /* * Don't recurse into the arguments of an outer PHV, Aggref, GroupingFunc, * or ReturningExpr 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, GroupingFunc, or ReturningExpr into the - * arguments to be passed down to the current subplan. + * with at the outer query level; for an Aggref, GroupingFunc, or + * ReturningExpr 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 aca7399cd8c..5dfa2e52d84 100644 --- a/src/backend/optimizer/util/var.c +++ b/src/backend/optimizer/util/var.c @@ -943,11 +943,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 b32fb9711ce..af52df9c04f 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -4409,7 +4409,7 @@ where i8.q2 = 123; QUERY PLAN --------------------------------------------------------------------- Nested Loop - Output: i8.q1, i8.q2, (InitPlan expr_2).col1, false, (i8.q2) + Output: i8.q1, i8.q2, (InitPlan expr_1).col1, false, (i8.q2) InitPlan expr_1 -> Result Output: true @@ -4422,7 +4422,7 @@ where i8.q2 = 123; -> Nested Loop Output: i8.q1, i8.q2, (i8.q2) -> Subquery Scan on ss1 - Output: ss1.y, (InitPlan expr_2).col1 + Output: ss1.y, (InitPlan expr_1).col1 -> Limit Output: NULL::integer -> Result @@ -4436,7 +4436,7 @@ where i8.q2 = 123; Output: (i8.q2) -> Result Output: i8.q2 - One-Time Filter: ((InitPlan expr_2).col1) + One-Time Filter: ((InitPlan expr_1).col1) (29 rows) explain (verbose, costs off) @@ -4452,7 +4452,7 @@ where i8.q2 = 123; QUERY PLAN --------------------------------------------------------------------- Nested Loop - Output: i8.q1, i8.q2, (InitPlan expr_2).col1, false, (i8.q2) + Output: i8.q1, i8.q2, (InitPlan expr_1).col1, false, (i8.q2) InitPlan expr_1 -> Result Output: true @@ -4466,7 +4466,7 @@ where i8.q2 = 123; -> Nested Loop Output: i8.q1, i8.q2, (i8.q2) -> Seq Scan on public.int4_tbl i4 - Output: i4.f1, (InitPlan expr_2).col1 + Output: i4.f1, (InitPlan expr_1).col1 Filter: (i4.f1 = 0) -> Nested Loop Output: i8.q1, i8.q2, (i8.q2) @@ -4477,7 +4477,7 @@ where i8.q2 = 123; Output: (i8.q2) -> Result Output: i8.q2 - One-Time Filter: ((InitPlan expr_2).col1) + One-Time Filter: ((InitPlan expr_1).col1) (27 rows) -- Test proper handling of appendrel PHVs during useless-RTE removal @@ -6584,25 +6584,25 @@ from (select case when false then remov.id else (select i41.f1) end as c1 QUERY PLAN --------------------------------------------------------- Nested Loop - Output: ((SubPlan expr_2)), (((SubPlan expr_2))) + Output: ((SubPlan expr_1)), (((SubPlan expr_1))) -> Nested Loop Left Join - Output: ((SubPlan expr_2)) + Output: ((SubPlan expr_1)) Join Filter: false -> Seq Scan on public.int4_tbl i42 Output: i42.f1 -> Result - Output: (SubPlan expr_2) + Output: (SubPlan expr_1) Replaces: Scan on i41 One-Time Filter: false - SubPlan expr_2 + SubPlan expr_1 -> Result Output: i41.f1 -> Memoize - Output: (((SubPlan expr_2))) - Cache Key: ((SubPlan expr_2)) + Output: (((SubPlan expr_1))) + Cache Key: ((SubPlan expr_1)) Cache Mode: binary -> Nested Loop - Output: ((SubPlan expr_2)) + Output: ((SubPlan expr_1)) -> Seq Scan on public.int4_tbl i43 Output: i43.f1 -> Materialize @@ -6657,6 +6657,134 @@ from (select case when false then remov.id else (select i41.f1) end as c1 Output: ((SubPlan expr_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 expr_1)), 0))) + -> Nested Loop Left Join + Output: (COALESCE(((SubPlan expr_1)), 0)) + -> Seq Scan on public.int4_tbl i41 + Output: i41.f1 + -> Materialize + Output: (COALESCE(((SubPlan expr_1)), 0)) + -> Nested Loop Left Join + Output: COALESCE(((SubPlan expr_1)), 0) + -> Seq Scan on public.int4_tbl i42 + Output: i42.f1 + -> Materialize + Output: ((SubPlan expr_1)) + -> Seq Scan on public.int4_tbl i43 + Output: (SubPlan expr_1) + SubPlan expr_1 + -> Result + Output: i43.f1 + -> Memoize + Output: ((COALESCE(((SubPlan expr_1)), 0))) + Cache Key: (COALESCE(((SubPlan expr_1)), 0)) + Cache Mode: binary + -> Seq Scan on public.int4_tbl i44 + Output: (COALESCE(((SubPlan expr_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 expr_1)), i42.c1)) + -> Hash Full Join + Output: ((SubPlan expr_1)), i42.c1 + Hash Cond: (((SubPlan expr_1)) = i42.c1) + -> Seq Scan on public.int4_tbl i41 + Output: (SubPlan expr_1) + SubPlan expr_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 expr_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 expr_2) + Hash Cond: (((SubPlan expr_1)) = i42.c1) + -> Seq Scan on public.int4_tbl i41 + Output: (SubPlan expr_1) + SubPlan expr_1 + -> Result + Output: i41.f1 + -> Hash + Output: i42.c1 + -> Seq Scan on public.int4_tbl i42 + Output: i42.c1 + SubPlan expr_2 + -> Seq Scan on public.int4_tbl i43 + Output: COALESCE(((SubPlan expr_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 expr_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 expr_1)) + Cache Key: (NULL::integer) + Cache Mode: binary + -> Append + -> Result + Output: (SubPlan expr_1) + SubPlan expr_1 + -> Result + Output: (NULL::integer) + -> Result + Output: (SubPlan expr_2) + SubPlan expr_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 77c95dd0f30..586cd3e8717 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2457,6 +2457,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