From 571ab907506f78557c1917192024f131f1dff7fb Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Thu, 15 Oct 2026 18:15:30 +0900 Subject: [PATCH v1] Don't remove an RTE_RESULT needed by a nested PlaceHolderVar When remove_useless_result_rtes() removes an RTE_RESULT, any PlaceHolderVars that were to be evaluated there must be evaluated somewhere else. If the RTE_RESULT is the nullable side of an outer join, there is no other place that would preserve the PHVs' nulling behavior, so find_dependent_phvs() refuses the removal if any such PHV exists anywhere in the query. If the RTE_RESULT is a child of an inner join, its PHVs can instead be evaluated at the join of all its sibling rels, unless a sibling needs them before that join is formed. find_dependent_phvs_in_jointree() checks for that by looking for such PHVs in the join quals and LATERAL RTEs of the siblings. That misses a PHV that is evaluated at some but not all of the sibling rels and contains one of the RTE_RESULT's PHVs. Such a PHV can appear anywhere in the query, such as in the targetlist. This can happen when pulled-up expressions are wrapped in PHVs because of outer joins or grouping sets. Once the RTE_RESULT is removed, the outer PHV needs a value that is only available at a join above its own evaluation level. The outer PHV's rel thus becomes laterally dependent on the other sibling rels. If one of those rels laterally references it in turn, we end up with a rel that is laterally dependent on itself, causing an assertion failure in create_lateral_join_info(). To fix, also search the whole query for PHVs that are evaluated at some but not all of the sibling rels, and refuse to remove the RTE_RESULT if any of them contains one of its PHVs. An outer PHV that is evaluated at all of the sibling rels is not a problem, since the moved PHV is available there, so we don't block removal in that common case. Back-patch to all supported branches. --- src/backend/optimizer/prep/prepjointree.c | 63 +++++++++++++++++ src/test/regress/expected/join.out | 83 +++++++++++++++++++++++ src/test/regress/sql/join.sql | 34 ++++++++++ 3 files changed, 180 insertions(+) diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 68cf6f18570..1504f84149c 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -4580,6 +4580,7 @@ typedef struct { Relids relids; /* target relid, represented as a relid set */ Relids baserels; /* base RT indexes in query, NULL if no PHVs */ + Relids siblingrels; /* base RT indexes of target's sibling rels */ int sublevels_up; /* current nesting level */ } find_dependent_phvs_context; @@ -4625,6 +4626,53 @@ find_dependent_phvs_walker(Node *node, return expression_tree_walker(node, find_dependent_phvs_walker, context); } +/* + * Look for PlaceHolderVars that are to be evaluated at some but not all of + * context->siblingrels and contain a dependent PHV in their expressions. + */ +static bool +find_nested_dependent_phvs_walker(Node *node, + find_dependent_phvs_context *context) +{ + if (node == NULL) + return false; + if (IsA(node, PlaceHolderVar)) + { + PlaceHolderVar *phv = (PlaceHolderVar *) node; + + if (phv->phlevelsup == context->sublevels_up) + { + Relids phbaserels = bms_intersect(phv->phrels, + context->baserels); + bool match; + + match = (!bms_is_subset(context->relids, phbaserels) && + bms_overlap(phbaserels, context->siblingrels) && + !bms_is_subset(context->siblingrels, phbaserels)); + bms_free(phbaserels); + if (match && + find_dependent_phvs_walker((Node *) phv->phexpr, context)) + return true; + } + /* fall through to examine children */ + } + if (IsA(node, Query)) + { + /* Recurse into subselects */ + bool result; + + context->sublevels_up++; + result = query_tree_walker((Query *) node, + find_nested_dependent_phvs_walker, + context, 0); + context->sublevels_up--; + return result; + } + + return expression_tree_walker(node, find_nested_dependent_phvs_walker, + context); +} + static bool find_dependent_phvs(PlannerInfo *root, int varno, Relids baserels) { @@ -4636,6 +4684,7 @@ find_dependent_phvs(PlannerInfo *root, int varno, Relids baserels) context.relids = bms_make_singleton(varno); context.baserels = baserels; + context.siblingrels = NULL; context.sublevels_up = 0; if (query_tree_walker(root->parse, find_dependent_phvs_walker, &context, 0)) @@ -4662,6 +4711,7 @@ find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno, context.relids = bms_make_singleton(varno); context.baserels = baserels; + context.siblingrels = NULL; context.sublevels_up = 0; /* @@ -4688,6 +4738,19 @@ find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno, return true; } + /* + * Lastly, check for PHVs that are evaluated at some but not all of the + * sibling rels and contain a dependent PHV. Removing the RTE_RESULT + * would move the dependent PHV up to the join of all the sibling rels, so + * it would not yet be available where the outer PHV is evaluated. The + * outer PHV can appear anywhere in the query (e.g., in the targetlist), + * so search the whole query. + */ + context.siblingrels = bms_del_member(subrelids, varno); + if (query_tree_walker(root->parse, find_nested_dependent_phvs_walker, + &context, 0)) + return true; + return false; } diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 042060f3aa4..36f2f983182 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5348,6 +5348,89 @@ select * from (values (1),(2)) v(x) 2 | (2 rows) +-- Nor an RTE_RESULT whose PHV is nested in a PHV evaluated at some but not +-- all of the sibling rels +explain (verbose, costs off) +select ss.z +from (select 1) s1 left join + ((select 1) s2 left join (select 1 as x) v on true + cross join (lateral (select coalesce(v.x, 0) as z, f1 from int4_tbl) ss + cross join lateral (select ss.f1 offset 0) g)) + on true; + QUERY PLAN +----------------------------------------------------------- + Nested Loop Left Join + Output: (COALESCE((1), 0)) + -> Result + -> Nested Loop + Output: (COALESCE((1), 0)) + -> Nested Loop + Output: int4_tbl.f1, (COALESCE((1), 0)) + -> Result + Output: 1 + -> Seq Scan on public.int4_tbl + Output: int4_tbl.f1, COALESCE((1), 0) + -> Result + Output: NULL::integer +(13 rows) + +select ss.z +from (select 1) s1 left join + ((select 1) s2 left join (select 1 as x) v on true + cross join (lateral (select coalesce(v.x, 0) as z, f1 from int4_tbl) ss + cross join lateral (select ss.f1 offset 0) g)) + on true; + z +--- + 1 + 1 + 1 + 1 + 1 +(5 rows) + +-- Same, with the PHVs coming from grouping sets +explain (costs off) +select s1.x +from (values (1)) as v(x), + lateral (select v.x + 1, 2) as s1(x, y) + cross join generate_series(s1.y, s1.y) as g +group by grouping sets (1, 1); + QUERY PLAN +------------------------------------------------ + HashAggregate + Hash Key: (((1) + 1)) + Hash Key: (((1) + 1)) + -> Nested Loop + -> Nested Loop + -> Result + -> Result + -> Function Scan on generate_series g +(8 rows) + +select s1.x +from (values (1)) as v(x), + lateral (select v.x + 1, 2) as s1(x, y) + cross join generate_series(s1.y, s1.y) as g +group by grouping sets (1, 1); + x +--- + 2 + 2 +(2 rows) + +-- But we can remove it if the outer PHV covers all of the sibling rels +explain (costs off) +select s.y from (values (1)) v(x), lateral (select v.x + f1 as y from int4_tbl) s +group by grouping sets (1, ()); + QUERY PLAN +------------------------------- + MixedAggregate + Hash Key: (1 + int4_tbl.f1) + Group Key: () + -> Seq Scan on int4_tbl +(4 rows) + -- This example demonstrates the folly of our old "have_dangerous_phv" logic begin; set local from_collapse_limit to 2; diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index c373cdeaa52..c204a694f2c 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1832,6 +1832,40 @@ select * from (values (1),(2)) v(x) left join (select q from (select 7 as q from (select where false) ss1) ss2 left join (select 8 as z) ss3 on true) ss4 on true; +-- Nor an RTE_RESULT whose PHV is nested in a PHV evaluated at some but not +-- all of the sibling rels +explain (verbose, costs off) +select ss.z +from (select 1) s1 left join + ((select 1) s2 left join (select 1 as x) v on true + cross join (lateral (select coalesce(v.x, 0) as z, f1 from int4_tbl) ss + cross join lateral (select ss.f1 offset 0) g)) + on true; +select ss.z +from (select 1) s1 left join + ((select 1) s2 left join (select 1 as x) v on true + cross join (lateral (select coalesce(v.x, 0) as z, f1 from int4_tbl) ss + cross join lateral (select ss.f1 offset 0) g)) + on true; + +-- Same, with the PHVs coming from grouping sets +explain (costs off) +select s1.x +from (values (1)) as v(x), + lateral (select v.x + 1, 2) as s1(x, y) + cross join generate_series(s1.y, s1.y) as g +group by grouping sets (1, 1); +select s1.x +from (values (1)) as v(x), + lateral (select v.x + 1, 2) as s1(x, y) + cross join generate_series(s1.y, s1.y) as g +group by grouping sets (1, 1); + +-- But we can remove it if the outer PHV covers all of the sibling rels +explain (costs off) +select s.y from (values (1)) v(x), lateral (select v.x + f1 as y from int4_tbl) s +group by grouping sets (1, ()); + -- This example demonstrates the folly of our old "have_dangerous_phv" logic begin; set local from_collapse_limit to 2; -- 2.37.1 (Apple Git-137.1)