agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: Matheus Alcantara <matheusssilv97@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: leis@in.tum.de
Cc: pgsql-bugs@lists.postgresql.org
Cc: Richard Guo <guofenglinux@gmail.com>
Subject: Re: BUG #19553: Wrong results from nested LEFT JOINs over an empty subquery (regression since v16)
Date: Fri, 17 Jul 2026 12:05:06 -0300
Message-ID: <DK0XT8FDBCLK.A43UXISCEHW9@gmail.com> (raw)
In-Reply-To: <3824828.1784244210@sss.pgh.pa.us>
References: <19553-4561747f93f368a7@postgresql.org>
<DK0CT1H1TW0G.3AJAOZTZJVTLL@gmail.com>
<3824828.1784244210@sss.pgh.pa.us>
On Thu Jul 16, 2026 at 8:23 PM -03, Tom Lane wrote:
> So the simplest fix is probably to mask off OJ bits and consider only
> baserels when deciding if phrels equals the target. Unfortunately,
> this happens long before we compute root->all_baserels or anything
> like that, so remove_useless_result_rtes is on its own to figure out
> which those are. I think we can extend it to build a bitmapset of
> relevant baserel RT indexes while it is scanning the tree (so that we
> don't need an additional recursive scan just to get that). But I've
> not tried to write any code yet; do you feel like attacking that?
>
Attached patch does this: remove_useless_result_rtes() now precomputes a
Relids of all non-join RT indexes by scanning root->parse->rtable once
up front (rather than doing a second recursive walk), and threads it
through remove_useless_results_recurse() down to both
find_dependent_phvs() and find_dependent_phvs_in_jointree(). Both now
intersect a candidate PHV's phrels with that baserels set before
comparing to the target relid, instead of comparing phrels as-is.
I checked that a plain flat scan of the rtable is sufficient here rather
than needing to accumulate the set incrementally during the jointree
recursion. Every PHV this code ever matches has phlevelsup ==
sublevels_up, which by construction means its phrels are relids in the
outermost query's rangetable, not some inner subquery's. So a single
baserels set computed once from root->parse->rtable is valid at every
recursion depth, and we don't need to recompute or thread a different
set per query level.
Note: Richard have also shared a fix for this bug on this thread, it's
also seems to fix the issue and IIUC it fixes without the extra loop to
collect the baserels. I decided to continue with my attempt following
your suggestion so we can discuss both approaches (and also because I'm
still not super familiar with this part of the code and I wanted to
learn more about it).
--
Matheus Alcantara
EDB: https://www.enterprisedb.com
From 79df439cd98f2e408ff54e760b3d1cdc68e4c8c8 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 v2] Fix wrong results from remove_useless_result_rtes with
nested PHVs.
find_dependent_phvs() and find_dependent_phvs_in_jointree() decide
whether a PlaceHolderVar still needs the RTE_RESULT rel we're about to
remove by comparing the PHV's phrels to a singleton set containing that
rel's RT index. But phrels is documented to hold "base+OJ relids
syntactically within" the PHV's expression, so it can contain outer-join
RT indexes as well as base-relation ones. The comparison was done
against the raw phrels, so a PHV whose phrels also included some
enclosing outer join's RT index would never compare equal to the
singleton, even though the PHV genuinely still depended on the
RTE_RESULT rel. This let remove_useless_results_recurse conclude it was
safe to discard the enclosing left join along with the RTE_RESULT,
silently losing that join's null-extension semantics. A PHV-wrapped
constant that should have gone to NULL for unmatched rows was then
emitted unconditionally for every row.
This can be seen with two nested commutable left joins where the lower
join's RHS reduces to a single-row RTE_RESULT: the PHV built for the
lower join's output picks up the outer join's RT index in its phrels
alongside the two RTE_RESULT relids, which broke the exact-match test
against either RTE_RESULT individually.
Fix this by masking phrels down to base relids before comparing, since
only base relids indicate a real evaluation-site dependency; the
presence of an OJ relid doesn't. remove_useless_result_rtes now
precomputes the set of base (non-join) RT indexes once and threads it
through the recursion and into both helper functions, which intersect it
with each candidate PHV's phrels before testing equality.
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 | 82 ++++++++++++++++++-----
src/test/regress/expected/join.out | 28 ++++++++
src/test/regress/sql/join.sql | 12 ++++
3 files changed, 106 insertions(+), 16 deletions(-)
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index ca5ca8bfe22..e9098322182 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -167,12 +167,15 @@ static bool has_notnull_forced_var(PlannerInfo *root, List *forced_null_vars,
reduce_outer_joins_pass1_state *right_state);
static Node *remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
Node **parent_quals,
- Relids *dropped_outer_joins);
+ Relids *dropped_outer_joins,
+ Relids baserels);
static int get_result_relid(PlannerInfo *root, Node *jtnode);
static void remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc);
-static bool find_dependent_phvs(PlannerInfo *root, int varno);
+static bool find_dependent_phvs(PlannerInfo *root, int varno,
+ Relids baserels);
static bool find_dependent_phvs_in_jointree(PlannerInfo *root,
- Node *node, int varno);
+ Node *node, int varno,
+ Relids baserels);
static void substitute_phv_relids(Node *node,
int varno, Relids subrelids);
static void fix_append_rel_relids(PlannerInfo *root, int varno,
@@ -3887,8 +3890,24 @@ void
remove_useless_result_rtes(PlannerInfo *root)
{
Relids dropped_outer_joins = NULL;
+ Relids baserels = NULL;
+ int i;
ListCell *cell;
+ /*
+ * Precompute the set of base (i.e., non-join) RT indexes in the query's
+ * rangetable. We'll need this below to filter out OJ relids when
+ * checking whether a PlaceHolderVar's phrels indicates a genuine
+ * dependency on some other relation, since phrels can contain OJ relids
+ * as well as base relids, and the presence of an OJ relid there doesn't
+ * mean the PHV must be evaluated at some other place.
+ */
+ for (i = 1; i <= list_length(root->parse->rtable); i++)
+ {
+ if (rt_fetch(i, root->parse->rtable)->rtekind != RTE_JOIN)
+ baserels = bms_add_member(baserels, i);
+ }
+
/* Top level of jointree must always be a FromExpr */
Assert(IsA(root->parse->jointree, FromExpr));
/* Recurse ... */
@@ -3896,7 +3915,8 @@ remove_useless_result_rtes(PlannerInfo *root)
remove_useless_results_recurse(root,
(Node *) root->parse->jointree,
NULL,
- &dropped_outer_joins);
+ &dropped_outer_joins,
+ baserels);
/* We should still have a FromExpr */
Assert(IsA(root->parse->jointree, FromExpr));
@@ -3955,11 +3975,14 @@ remove_useless_result_rtes(PlannerInfo *root)
* the parent's quals list; otherwise, pass NULL for parent_quals.
* (Note that in some cases, parent_quals points to the quals of a parent
* more than one level up in the tree.)
+ *
+ * baserels is the set of base (non-join) RT indexes in the whole query.
*/
static Node *
remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
Node **parent_quals,
- Relids *dropped_outer_joins)
+ Relids *dropped_outer_joins,
+ Relids baserels)
{
Assert(jtnode != NULL);
if (IsA(jtnode, RangeTblRef))
@@ -3989,7 +4012,8 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
/* Recursively transform child, allowing it to push up quals ... */
child = remove_useless_results_recurse(root, child,
&f->quals,
- dropped_outer_joins);
+ dropped_outer_joins,
+ baserels);
/* ... and stick it back into the tree */
lfirst(cell) = child;
@@ -4001,7 +4025,8 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
*/
if (list_length(f->fromlist) > 1 &&
(varno = get_result_relid(root, child)) != 0 &&
- !find_dependent_phvs_in_jointree(root, (Node *) f, varno))
+ !find_dependent_phvs_in_jointree(root, (Node *) f, varno,
+ baserels))
{
f->fromlist = foreach_delete_current(f->fromlist, cell);
result_relids = bms_add_member(result_relids, varno);
@@ -4074,12 +4099,14 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
&j->quals :
(j->jointype == JOIN_LEFT) ?
parent_quals : NULL,
- dropped_outer_joins);
+ dropped_outer_joins,
+ baserels);
j->rarg = remove_useless_results_recurse(root, j->rarg,
(j->jointype == JOIN_INNER ||
j->jointype == JOIN_LEFT) ?
&j->quals : NULL,
- dropped_outer_joins);
+ dropped_outer_joins,
+ baserels);
/* Apply join-type-specific optimization rules */
switch (j->jointype)
@@ -4103,7 +4130,8 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
* allowed to have such refs.
*/
if ((varno = get_result_relid(root, j->larg)) != 0 &&
- !find_dependent_phvs_in_jointree(root, j->rarg, varno))
+ !find_dependent_phvs_in_jointree(root, j->rarg, varno,
+ baserels))
{
remove_result_refs(root, varno, j->rarg);
if (j->quals != NULL && parent_quals == NULL)
@@ -4158,7 +4186,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
*/
if ((varno = get_result_relid(root, j->rarg)) != 0 &&
(j->quals == NULL ||
- !find_dependent_phvs(root, varno)))
+ !find_dependent_phvs(root, varno, baserels)))
{
remove_result_refs(root, varno, j->larg);
*dropped_outer_joins = bms_add_member(*dropped_outer_joins,
@@ -4283,6 +4311,17 @@ remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc)
* find_dependent_phvs - are there any PlaceHolderVars whose relids are
* exactly the given varno?
*
+ * "relids" means the PHV's phrels with any outer-join relids masked
+ * off by intersecting with the caller-supplied "baserels". phrels can
+ * contain OJ relids as well as base relids (it's the set of base+OJ relids
+ * syntactically within the PHV's expression), but the presence of an OJ
+ * relid there doesn't create any additional place where the PHV must be
+ * evaluated; it's only base relids that pin down the evaluation location.
+ * If we compared phrels as-is, we could wrongly conclude that a PHV isn't
+ * dependent on a RTE_RESULT rel we're about to remove, just because the
+ * PHV's phrels also happens to include some OJ that sits between the PHV
+ * and the RTE_RESULT syntactically.
+ *
* 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
@@ -4293,6 +4332,7 @@ remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc)
typedef struct
{
Relids relids;
+ Relids baserels; /* set of base (non-OJ) RT indexes in query */
int sublevels_up;
} find_dependent_phvs_context;
@@ -4306,9 +4346,16 @@ find_dependent_phvs_walker(Node *node,
{
PlaceHolderVar *phv = (PlaceHolderVar *) node;
- if (phv->phlevelsup == context->sublevels_up &&
- bms_equal(context->relids, phv->phrels))
- return true;
+ if (phv->phlevelsup == context->sublevels_up)
+ {
+ Relids phbaserels = bms_intersect(phv->phrels,
+ context->baserels);
+ bool match = bms_equal(context->relids, phbaserels);
+
+ bms_free(phbaserels);
+ if (match)
+ return true;
+ }
/* fall through to examine children */
}
if (IsA(node, Query))
@@ -4332,7 +4379,7 @@ find_dependent_phvs_walker(Node *node,
}
static bool
-find_dependent_phvs(PlannerInfo *root, int varno)
+find_dependent_phvs(PlannerInfo *root, int varno, Relids baserels)
{
find_dependent_phvs_context context;
@@ -4341,6 +4388,7 @@ find_dependent_phvs(PlannerInfo *root, int varno)
return false;
context.relids = bms_make_singleton(varno);
+ context.baserels = baserels;
context.sublevels_up = 0;
if (query_tree_walker(root->parse, find_dependent_phvs_walker, &context, 0))
@@ -4354,7 +4402,8 @@ find_dependent_phvs(PlannerInfo *root, int varno)
}
static bool
-find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno)
+find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno,
+ Relids baserels)
{
find_dependent_phvs_context context;
Relids subrelids;
@@ -4365,6 +4414,7 @@ find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno)
return false;
context.relids = bms_make_singleton(varno);
+ context.baserels = baserels;
context.sublevels_up = 0;
/*
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] v2-0001-Fix-wrong-results-from-remove_useless_result_rtes.patch (12.9K, ../DK0XT8FDBCLK.A43UXISCEHW9@gmail.com/2-v2-0001-Fix-wrong-results-from-remove_useless_result_rtes.patch)
download | inline diff:
From 79df439cd98f2e408ff54e760b3d1cdc68e4c8c8 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 v2] Fix wrong results from remove_useless_result_rtes with
nested PHVs.
find_dependent_phvs() and find_dependent_phvs_in_jointree() decide
whether a PlaceHolderVar still needs the RTE_RESULT rel we're about to
remove by comparing the PHV's phrels to a singleton set containing that
rel's RT index. But phrels is documented to hold "base+OJ relids
syntactically within" the PHV's expression, so it can contain outer-join
RT indexes as well as base-relation ones. The comparison was done
against the raw phrels, so a PHV whose phrels also included some
enclosing outer join's RT index would never compare equal to the
singleton, even though the PHV genuinely still depended on the
RTE_RESULT rel. This let remove_useless_results_recurse conclude it was
safe to discard the enclosing left join along with the RTE_RESULT,
silently losing that join's null-extension semantics. A PHV-wrapped
constant that should have gone to NULL for unmatched rows was then
emitted unconditionally for every row.
This can be seen with two nested commutable left joins where the lower
join's RHS reduces to a single-row RTE_RESULT: the PHV built for the
lower join's output picks up the outer join's RT index in its phrels
alongside the two RTE_RESULT relids, which broke the exact-match test
against either RTE_RESULT individually.
Fix this by masking phrels down to base relids before comparing, since
only base relids indicate a real evaluation-site dependency; the
presence of an OJ relid doesn't. remove_useless_result_rtes now
precomputes the set of base (non-join) RT indexes once and threads it
through the recursion and into both helper functions, which intersect it
with each candidate PHV's phrels before testing equality.
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 | 82 ++++++++++++++++++-----
src/test/regress/expected/join.out | 28 ++++++++
src/test/regress/sql/join.sql | 12 ++++
3 files changed, 106 insertions(+), 16 deletions(-)
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index ca5ca8bfe22..e9098322182 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -167,12 +167,15 @@ static bool has_notnull_forced_var(PlannerInfo *root, List *forced_null_vars,
reduce_outer_joins_pass1_state *right_state);
static Node *remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
Node **parent_quals,
- Relids *dropped_outer_joins);
+ Relids *dropped_outer_joins,
+ Relids baserels);
static int get_result_relid(PlannerInfo *root, Node *jtnode);
static void remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc);
-static bool find_dependent_phvs(PlannerInfo *root, int varno);
+static bool find_dependent_phvs(PlannerInfo *root, int varno,
+ Relids baserels);
static bool find_dependent_phvs_in_jointree(PlannerInfo *root,
- Node *node, int varno);
+ Node *node, int varno,
+ Relids baserels);
static void substitute_phv_relids(Node *node,
int varno, Relids subrelids);
static void fix_append_rel_relids(PlannerInfo *root, int varno,
@@ -3887,8 +3890,24 @@ void
remove_useless_result_rtes(PlannerInfo *root)
{
Relids dropped_outer_joins = NULL;
+ Relids baserels = NULL;
+ int i;
ListCell *cell;
+ /*
+ * Precompute the set of base (i.e., non-join) RT indexes in the query's
+ * rangetable. We'll need this below to filter out OJ relids when
+ * checking whether a PlaceHolderVar's phrels indicates a genuine
+ * dependency on some other relation, since phrels can contain OJ relids
+ * as well as base relids, and the presence of an OJ relid there doesn't
+ * mean the PHV must be evaluated at some other place.
+ */
+ for (i = 1; i <= list_length(root->parse->rtable); i++)
+ {
+ if (rt_fetch(i, root->parse->rtable)->rtekind != RTE_JOIN)
+ baserels = bms_add_member(baserels, i);
+ }
+
/* Top level of jointree must always be a FromExpr */
Assert(IsA(root->parse->jointree, FromExpr));
/* Recurse ... */
@@ -3896,7 +3915,8 @@ remove_useless_result_rtes(PlannerInfo *root)
remove_useless_results_recurse(root,
(Node *) root->parse->jointree,
NULL,
- &dropped_outer_joins);
+ &dropped_outer_joins,
+ baserels);
/* We should still have a FromExpr */
Assert(IsA(root->parse->jointree, FromExpr));
@@ -3955,11 +3975,14 @@ remove_useless_result_rtes(PlannerInfo *root)
* the parent's quals list; otherwise, pass NULL for parent_quals.
* (Note that in some cases, parent_quals points to the quals of a parent
* more than one level up in the tree.)
+ *
+ * baserels is the set of base (non-join) RT indexes in the whole query.
*/
static Node *
remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
Node **parent_quals,
- Relids *dropped_outer_joins)
+ Relids *dropped_outer_joins,
+ Relids baserels)
{
Assert(jtnode != NULL);
if (IsA(jtnode, RangeTblRef))
@@ -3989,7 +4012,8 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
/* Recursively transform child, allowing it to push up quals ... */
child = remove_useless_results_recurse(root, child,
&f->quals,
- dropped_outer_joins);
+ dropped_outer_joins,
+ baserels);
/* ... and stick it back into the tree */
lfirst(cell) = child;
@@ -4001,7 +4025,8 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
*/
if (list_length(f->fromlist) > 1 &&
(varno = get_result_relid(root, child)) != 0 &&
- !find_dependent_phvs_in_jointree(root, (Node *) f, varno))
+ !find_dependent_phvs_in_jointree(root, (Node *) f, varno,
+ baserels))
{
f->fromlist = foreach_delete_current(f->fromlist, cell);
result_relids = bms_add_member(result_relids, varno);
@@ -4074,12 +4099,14 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
&j->quals :
(j->jointype == JOIN_LEFT) ?
parent_quals : NULL,
- dropped_outer_joins);
+ dropped_outer_joins,
+ baserels);
j->rarg = remove_useless_results_recurse(root, j->rarg,
(j->jointype == JOIN_INNER ||
j->jointype == JOIN_LEFT) ?
&j->quals : NULL,
- dropped_outer_joins);
+ dropped_outer_joins,
+ baserels);
/* Apply join-type-specific optimization rules */
switch (j->jointype)
@@ -4103,7 +4130,8 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
* allowed to have such refs.
*/
if ((varno = get_result_relid(root, j->larg)) != 0 &&
- !find_dependent_phvs_in_jointree(root, j->rarg, varno))
+ !find_dependent_phvs_in_jointree(root, j->rarg, varno,
+ baserels))
{
remove_result_refs(root, varno, j->rarg);
if (j->quals != NULL && parent_quals == NULL)
@@ -4158,7 +4186,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode,
*/
if ((varno = get_result_relid(root, j->rarg)) != 0 &&
(j->quals == NULL ||
- !find_dependent_phvs(root, varno)))
+ !find_dependent_phvs(root, varno, baserels)))
{
remove_result_refs(root, varno, j->larg);
*dropped_outer_joins = bms_add_member(*dropped_outer_joins,
@@ -4283,6 +4311,17 @@ remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc)
* find_dependent_phvs - are there any PlaceHolderVars whose relids are
* exactly the given varno?
*
+ * "relids" means the PHV's phrels with any outer-join relids masked
+ * off by intersecting with the caller-supplied "baserels". phrels can
+ * contain OJ relids as well as base relids (it's the set of base+OJ relids
+ * syntactically within the PHV's expression), but the presence of an OJ
+ * relid there doesn't create any additional place where the PHV must be
+ * evaluated; it's only base relids that pin down the evaluation location.
+ * If we compared phrels as-is, we could wrongly conclude that a PHV isn't
+ * dependent on a RTE_RESULT rel we're about to remove, just because the
+ * PHV's phrels also happens to include some OJ that sits between the PHV
+ * and the RTE_RESULT syntactically.
+ *
* 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
@@ -4293,6 +4332,7 @@ remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc)
typedef struct
{
Relids relids;
+ Relids baserels; /* set of base (non-OJ) RT indexes in query */
int sublevels_up;
} find_dependent_phvs_context;
@@ -4306,9 +4346,16 @@ find_dependent_phvs_walker(Node *node,
{
PlaceHolderVar *phv = (PlaceHolderVar *) node;
- if (phv->phlevelsup == context->sublevels_up &&
- bms_equal(context->relids, phv->phrels))
- return true;
+ if (phv->phlevelsup == context->sublevels_up)
+ {
+ Relids phbaserels = bms_intersect(phv->phrels,
+ context->baserels);
+ bool match = bms_equal(context->relids, phbaserels);
+
+ bms_free(phbaserels);
+ if (match)
+ return true;
+ }
/* fall through to examine children */
}
if (IsA(node, Query))
@@ -4332,7 +4379,7 @@ find_dependent_phvs_walker(Node *node,
}
static bool
-find_dependent_phvs(PlannerInfo *root, int varno)
+find_dependent_phvs(PlannerInfo *root, int varno, Relids baserels)
{
find_dependent_phvs_context context;
@@ -4341,6 +4388,7 @@ find_dependent_phvs(PlannerInfo *root, int varno)
return false;
context.relids = bms_make_singleton(varno);
+ context.baserels = baserels;
context.sublevels_up = 0;
if (query_tree_walker(root->parse, find_dependent_phvs_walker, &context, 0))
@@ -4354,7 +4402,8 @@ find_dependent_phvs(PlannerInfo *root, int varno)
}
static bool
-find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno)
+find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno,
+ Relids baserels)
{
find_dependent_phvs_context context;
Relids subrelids;
@@ -4365,6 +4414,7 @@ find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno)
return false;
context.relids = bms_make_singleton(varno);
+ context.baserels = baserels;
context.sublevels_up = 0;
/*
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: <DK0XT8FDBCLK.A43UXISCEHW9@gmail.com>
Permalink: ../DK0XT8FDBCLK.A43UXISCEHW9@gmail.com/
Also on: postgresql.org/message-id/DK0XT8FDBCLK.A43UXISCEHW9@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, tgl@sss.pgh.pa.us, leis@in.tum.de, pgsql-bugs@lists.postgresql.org, guofenglinux@gmail.com
Subject: Re: BUG #19553: Wrong results from nested LEFT JOINs over an empty subquery (regression since v16)
In-Reply-To: <DK0XT8FDBCLK.A43UXISCEHW9@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