summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2024-01-11 15:28:13 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2024-01-11 15:28:13 -0500
commita0b4fda442a7acf31d919f5a8988d230fa4db6cb (patch)
treefe696cda4f295db9cd7e738de1276357cbb1bd86
parentbcaf41c608b1b7b462d6ff13c0e4fa80a8847f66 (diff)
Allow subquery pullup to wrap a PlaceHolderVar in another one.
The code for wrapping subquery output expressions in PlaceHolderVars believed that if the expression already was a PlaceHolderVar, it was never necessary to wrap that in another one. That's wrong if the expression is underneath an outer join and involves a lateral reference to outside that scope: failing to add an additional PHV risks evaluating the expression at the wrong place and hence not forcing it to null when the outer join should do so. This is an oversight in commit 9e7e29c75, which added logic to forcibly wrap lateral-reference Vars in PlaceHolderVars, but didn't see that the adjacent case for PlaceHolderVars needed the same treatment. The test case we have for this doesn't fail before 4be058fe9, but now that I see the problem I wonder if it is possible to demonstrate related errors before that. That's moot though, since all such branches are out of support. Per bug #18284 from Holger Reise. Back-patch to all supported branches. Discussion: https://postgr.es/m/18284-47505a20c23647f8@postgresql.org
-rw-r--r--src/backend/optimizer/prep/prepjointree.c9
-rw-r--r--src/test/regress/expected/join.out26
-rw-r--r--src/test/regress/sql/join.sql12
3 files changed, 45 insertions, 2 deletions
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 7f4bb7b99a2..ea0576305d0 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -2471,8 +2471,13 @@ pullup_replace_vars_callback(Var *var,
else if (newnode && IsA(newnode, PlaceHolderVar) &&
((PlaceHolderVar *) newnode)->phlevelsup == 0)
{
- /* No need to wrap a PlaceHolderVar with another one, either */
- wrap = false;
+ /* The same rules apply for a PlaceHolderVar */
+ if (rcon->target_rte->lateral &&
+ !bms_is_subset(((PlaceHolderVar *) newnode)->phrels,
+ rcon->relids))
+ wrap = true;
+ else
+ wrap = false;
}
else if (rcon->wrap_non_vars)
{
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 0eb6339f608..867c6d20ccf 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -5803,6 +5803,32 @@ select * from
Output: (COALESCE((COALESCE(b.q2, '42'::bigint)), d.q2))
(24 rows)
+-- another case requiring nested PlaceHolderVars
+explain (verbose, costs off)
+select * from
+ (select 0 as val0) as ss0
+ left join (select 1 as val) as ss1 on true
+ left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
+ QUERY PLAN
+--------------------------------
+ Nested Loop Left Join
+ Output: 0, (1), ((1))
+ -> Result
+ Output: 1
+ -> Result
+ Output: (1)
+ One-Time Filter: false
+(7 rows)
+
+select * from
+ (select 0 as val0) as ss0
+ left join (select 1 as val) as ss1 on true
+ left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
+ val0 | val | val_filtered
+------+-----+--------------
+ 0 | 1 |
+(1 row)
+
-- case that breaks the old ph_may_need optimization
explain (verbose, costs off)
select c.*,a.*,ss1.q1,ss2.q1,ss3.* from
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 5fdacce91df..1113e98445a 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1978,6 +1978,18 @@ select * from
) on c.q2 = ss2.q1,
lateral (select ss2.y offset 0) ss3;
+-- another case requiring nested PlaceHolderVars
+explain (verbose, costs off)
+select * from
+ (select 0 as val0) as ss0
+ left join (select 1 as val) as ss1 on true
+ left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
+
+select * from
+ (select 0 as val0) as ss0
+ left join (select 1 as val) as ss1 on true
+ left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
+
-- case that breaks the old ph_may_need optimization
explain (verbose, costs off)
select c.*,a.*,ss1.q1,ss2.q1,ss3.* from