From 6672f3c59c17715cba4500b19bf6cb7b64b19b71 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Thu, 16 Jul 2026 19:05:55 -0300 Subject: [PATCH] Fix wrong results from remove_useless_result_rtes with nested PHVs. find_dependent_phvs() was checking whether any PlaceHolderVar's phrels was exactly equal to the RTE_RESULT relid being considered for removal. But a PHV can legitimately have additional relids in phrels beyond the one being removed, when the PHV's value is computed jointly with a sibling RTE_RESULT that gets folded away first, as happens when two commutable left joins are nested and the lower one's RHS reduces to a constant. In such cases the PHV still depends on the outer RTE_RESULT being removed, but the exact-match test missed it, causing remove_useless_results_recurse to discard the enclosing left join along with the RTE_RESULT. That silently dropped the outer join's null-extension semantics, so a PHV-wrapped constant that should have been nulled out for unmatched rows was instead emitted unconditionally for every row. Fix find_dependent_phvs to test whether the given relid is a member of phrels rather than requiring an exact match, since membership is what actually indicates a dependency. find_dependent_phvs_in_jointree, which is used for a different purpose (deciding whether a FromExpr or inner join can be elided) and has different correctness requirements, keeps its original exact-match behavior. The two now share the walker via an added "exact" flag on the context struct. Author: Matheus Alcantara Reported-by: leis@in.tum.de Discussion: https://www.postgresql.org/message-id/19553-4561747f93f368a7@postgresql.org --- src/backend/optimizer/prep/prepjointree.c | 23 ++++++++++++++----- src/test/regress/expected/join.out | 28 +++++++++++++++++++++++ src/test/regress/sql/join.sql | 12 ++++++++++ 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index ca5ca8bfe22..188eccea4d5 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -4280,20 +4280,27 @@ remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc) /* - * find_dependent_phvs - are there any PlaceHolderVars whose relids are - * exactly the given varno? + * find_dependent_phvs - are there any PlaceHolderVars that depend on the + * given varno? + * + * "Depend on" means the varno is a member of the PHV's phrels, i.e. the PHV's + * value is (partly) computed at that relation. We must consider such PHVs and + * not only those whose phrels are exactly {varno}, because a left join whose + * RHS is that relation nulls the whole PHV; dropping the join would silently + * un-null it. See the JOIN_LEFT case in remove_useless_results_recurse. * * find_dependent_phvs should be used when we want to see if there are * any such PHVs anywhere in the Query. Another use-case is to see if - * a subtree of the join tree contains such PHVs; but for that, we have - * to look not only at the join tree nodes themselves but at the - * referenced RTEs. For that, use find_dependent_phvs_in_jointree. + * a subtree of the join tree contains PHVs whose phrels are exactly {varno}; + * but for that, we have to look not only at the join tree nodes themselves but + * at the referenced RTEs. For that, use find_dependent_phvs_in_jointree. */ typedef struct { Relids relids; int sublevels_up; + bool exact; /* match phrels exactly, else by membership */ } find_dependent_phvs_context; static bool @@ -4307,7 +4314,9 @@ find_dependent_phvs_walker(Node *node, PlaceHolderVar *phv = (PlaceHolderVar *) node; if (phv->phlevelsup == context->sublevels_up && - bms_equal(context->relids, phv->phrels)) + (context->exact ? + bms_equal(context->relids, phv->phrels) : + bms_is_subset(context->relids, phv->phrels))) return true; /* fall through to examine children */ } @@ -4342,6 +4351,7 @@ find_dependent_phvs(PlannerInfo *root, int varno) context.relids = bms_make_singleton(varno); context.sublevels_up = 0; + context.exact = false; if (query_tree_walker(root->parse, find_dependent_phvs_walker, &context, 0)) return true; @@ -4366,6 +4376,7 @@ find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno) context.relids = bms_make_singleton(varno); context.sublevels_up = 0; + context.exact = true; /* * See if the jointree fragment itself contains references (in join quals) diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 83bd5649d5c..65fc3c820bc 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -2630,6 +2630,34 @@ select * from int4_tbl t1 -> Seq Scan on tenk1 t4 (14 rows) +-- check a case where we formerly failed to detect that a PlaceHolderVar +-- containing a constant should still be nulled by an outer join above the +-- one that was removed by remove_useless_result_rtes +explain (verbose, costs off) +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; + QUERY PLAN +------------------------------------ + Nested Loop Left Join + Output: "*VALUES*".column1, (7) + Join Filter: false + -> Values Scan on "*VALUES*" + Output: "*VALUES*".column1 + -> Result + Output: 7 + One-Time Filter: false +(8 rows) + +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; + x | q +---+--- + 1 | + 2 | +(2 rows) + explain (costs off) select * from onek t1 left join onek t2 on t1.unique1 = t2.unique1 diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 32d4a5a677e..af12542b4ff 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -531,6 +531,18 @@ select * from int4_tbl t1 left join tenk1 t4 on s.f1 > 1) on s.f1 = t1.f1; +-- check a case where we formerly failed to detect that a PlaceHolderVar +-- containing a constant should still be nulled by an outer join above the +-- one that was removed by remove_useless_result_rtes +explain (verbose, costs off) +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; + +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; + explain (costs off) select * from onek t1 left join onek t2 on t1.unique1 = t2.unique1 -- 2.50.1 (Apple Git-155)