agora inbox for pgsql-bugs@postgresql.org  
help / color / mirror / Atom feed
From: Matheus Alcantara <matheusssilv97@gmail.com>
To: leis@in.tum.de
To: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19553: Wrong results from nested LEFT JOINs over an empty subquery (regression since v16)
Date: Thu, 16 Jul 2026 19:37:27 -0300
Message-ID: <DK0CT1H1TW0G.3AJAOZTZJVTLL@gmail.com> (raw)
In-Reply-To: <19553-4561747f93f368a7@postgresql.org>
References: <19553-4561747f93f368a7@postgresql.org>

On Thu Jul 16, 2026 at 11:12 AM -03, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference:      19553
> Logged by:          Viktor Leis
> Email address:      leis@in.tum.de
> PostgreSQL version: 19beta2
> Operating system:   Ubuntu 26.04
> Description:        
>
> Hi,
>
> The following self-contained query returns wrong results on every release
> since v16:
>
>   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 | 7
>    2 | 7
>   (2 rows)
>
> The right-hand side of the top left join is provably empty (ss1 produces no
> rows, and the inner left join preserves that), so the correct result
> null-extends both rows:
>
>    x | q
>   ---+---
>    1 |
>    2 |
>   (2 rows)
>
> EXPLAIN (VERBOSE) on affected versions shows that the entire RHS has been
> optimized away and the constant is emitted unconditionally:
>
>    Values Scan on "*VALUES*"
>      Output: "*VALUES*".column1, 7
>
> I bisected the regression to commit 3af87736bf5 ("Fix another cause of
> 'wrong varnullingrels' planner failures" by Tom Lane).
>

Hi, thank you for the report and for the script reproducer.

According to my findings, the issue seems to be in
remove_useless_results_recurse(), specifically in find_dependent_phvs().
When a LEFT JOIN's RHS reduces to an RTE_RESULT, that function is used
to check whether any PlaceHolderVar still depends on the RTE_RESULT
being considered for removal; if not, the join is discarded along with
the RTE_RESULT. The check was testing whether a PHV's phrels was exactly
equal to {varno}, but that's too narrow. In your example, the constant
"7" gets wrapped in a PlaceHolderVar whose phrels ends up as the union
of both RTE_RESULTs in the inner commuted left join (the ss1 "WHERE
false" result and the ss3 "SELECT 8" result), because the pulled-up
subquery's output needs to be nullable by the outer join. Since phrels
was {ss1, ss3} rather than exactly {ss1}, find_dependent_phvs() wrongly
concluded the outer join had no remaining dependents and dropped it,
taking the join's null-extension semantics with it. That's why the
constant got emitted unconditionally instead of being nulled out for the
"no match" case.

Attached patch seems to fix find_dependent_phvs() to test membership
rather than exact equality, since that's what actually indicates the
PHV's value depends on that relation. find_dependent_phvs_in_jointree(),
used for a different purpose so keeps the exact-match behavior it had,
as that's it seems to still correct for its use case. I added a field to
the shared context struct to keep both behaviors sharing one walker.

Verified against your original repro and a version using tables instead
of VALUES/constants (make it more easier for me to debug):

create table t (id int, name text);
insert into t values (1, 'Alice'), (2, 'Bob');

select c.id, c.name, promo.discount
from t c
left join (
    select discount
    from (select 0.20 as discount from (select where false) no_matching_promo) rates
    left join (select true as eligible) elig on true
) promo on true;

--
Matheus Alcantara
EDB: https://www.enterprisedb.com
From 6672f3c59c17715cba4500b19bf6cb7b64b19b71 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
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 <mths.dev@pm.me>
Reported-by: leis@in.tum.de <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)



Attachments:

  [text/plain] 0001-Fix-wrong-results-from-remove_useless_result_rtes-wi.patch (6.8K, ../DK0CT1H1TW0G.3AJAOZTZJVTLL@gmail.com/2-0001-Fix-wrong-results-from-remove_useless_result_rtes-wi.patch)
  download | inline diff:
From 6672f3c59c17715cba4500b19bf6cb7b64b19b71 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <mths.dev@pm.me>
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 <mths.dev@pm.me>
Reported-by: leis@in.tum.de <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)



view thread (13+ messages)  latest in thread

Message-ID: <DK0CT1H1TW0G.3AJAOZTZJVTLL@gmail.com>
Permalink:  ../DK0CT1H1TW0G.3AJAOZTZJVTLL@gmail.com/
Also on:    postgresql.org/message-id/DK0CT1H1TW0G.3AJAOZTZJVTLL@gmail.com

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: matheusssilv97@gmail.com, leis@in.tum.de, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #19553: Wrong results from nested LEFT JOINs over an empty subquery (regression since v16)
  In-Reply-To: <DK0CT1H1TW0G.3AJAOZTZJVTLL@gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox