From f69466a28f538ad5f771ee4d922cce0aa1fed356 Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Wed, 16 Sep 2026 10:23:05 +0900 Subject: [PATCH v1] Fix preprocessing of PHV copies pushed down into subqueries Commit e28cf96e9 preprocessed the copies of a query level's PlaceHolderVars that were pushed down into its subqueries in a single walk at the start of subquery_planner. That was wrong in two ways. First, such copies can also be inserted later, when a join alias Var of the outer level is expanded within a LATERAL subquery or a SubLink's subselect and the alias expression contains a PHV. Those copies were never preprocessed, so a SubLink within them survived into the subquery's lateral references or into a SubPlan's argument list, tripping an assertion in identify_current_nestloop_params or failing in cost_qual_eval. Second, when a pushed-down copy directly contains another PHV of the same level, the walk preprocessed the inner one first and then the outer one, so the outer one's preprocessing ran into the SubPlans just created in the inner one and tripped an assertion in flatten_join_alias_vars. To fix, preprocess the copies right after join alias expansion instead: within preprocess_expression, just before SubLinks are turned into SubPlans, and for LATERAL subquery RTEs, right after their aliases are flattened. Preprocessing a copy handles everything within it, including any further copies nested inside its SubLinks, so the walker no longer looks inside a copy. Back-patch to v16, as with commit e28cf96e9. Reported-by: Fujii Masao Author: Richard Guo Discussion: https://postgr.es/m/CAHGQGwH-acRnAk1=Ki4EiAJEt67+Rz=_jyg7pbP_c6BK4oHZ4Q@mail.gmail.com Backpatch-through: 16 --- src/backend/optimizer/plan/planner.c | 52 ++++++----- src/backend/optimizer/plan/subselect.c | 7 +- src/backend/optimizer/util/var.c | 9 +- src/test/regress/expected/join.out | 117 ++++++++++++++++++++++--- src/test/regress/sql/join.sql | 26 ++++++ 5 files changed, 170 insertions(+), 41 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 8d30131855a..55a35aa3397 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 75544fe6aa3..1f000d641eb 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5182,7 +5182,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 @@ -5195,7 +5195,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 @@ -5209,7 +5209,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) @@ -5225,7 +5225,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 @@ -5239,7 +5239,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) @@ -5250,7 +5250,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 @@ -7357,25 +7357,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 @@ -7430,6 +7430,97 @@ 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) + -- 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 fb83a96e939..e3955305e42 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2746,6 +2746,32 @@ 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; + -- More tests of correct placement of pseudoconstant quals -- simple constant-false condition -- 2.37.1 (Apple Git-137.1)