Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 69c12c4

Browse files
committed
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 9e7e29c, 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 4be058f, 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
1 parent 18d51c0 commit 69c12c4

File tree

3 files changed

+45
-2
lines changed

3 files changed

+45
-2
lines changed

src/backend/optimizer/prep/prepjointree.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2183,8 +2183,13 @@ pullup_replace_vars_callback(Var *var,
21832183
else if (newnode && IsA(newnode, PlaceHolderVar) &&
21842184
((PlaceHolderVar *) newnode)->phlevelsup == 0)
21852185
{
2186-
/* No need to wrap a PlaceHolderVar with another one, either */
2187-
wrap = false;
2186+
/* The same rules apply for a PlaceHolderVar */
2187+
if (rcon->target_rte->lateral &&
2188+
!bms_is_subset(((PlaceHolderVar *) newnode)->phrels,
2189+
rcon->relids))
2190+
wrap = true;
2191+
else
2192+
wrap = false;
21882193
}
21892194
else if (rcon->wrap_non_vars)
21902195
{

src/test/regress/expected/join.out

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5526,6 +5526,32 @@ select * from
55265526
Output: (COALESCE((COALESCE(b.q2, '42'::bigint)), d.q2))
55275527
(24 rows)
55285528

5529+
-- another case requiring nested PlaceHolderVars
5530+
explain (verbose, costs off)
5531+
select * from
5532+
(select 0 as val0) as ss0
5533+
left join (select 1 as val) as ss1 on true
5534+
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
5535+
QUERY PLAN
5536+
--------------------------------
5537+
Nested Loop Left Join
5538+
Output: 0, (1), ((1))
5539+
-> Result
5540+
Output: 1
5541+
-> Result
5542+
Output: (1)
5543+
One-Time Filter: false
5544+
(7 rows)
5545+
5546+
select * from
5547+
(select 0 as val0) as ss0
5548+
left join (select 1 as val) as ss1 on true
5549+
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
5550+
val0 | val | val_filtered
5551+
------+-----+--------------
5552+
0 | 1 |
5553+
(1 row)
5554+
55295555
-- case that breaks the old ph_may_need optimization
55305556
explain (verbose, costs off)
55315557
select c.*,a.*,ss1.q1,ss2.q1,ss3.* from

src/test/regress/sql/join.sql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,6 +1869,18 @@ select * from
18691869
) on c.q2 = ss2.q1,
18701870
lateral (select ss2.y offset 0) ss3;
18711871

1872+
-- another case requiring nested PlaceHolderVars
1873+
explain (verbose, costs off)
1874+
select * from
1875+
(select 0 as val0) as ss0
1876+
left join (select 1 as val) as ss1 on true
1877+
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
1878+
1879+
select * from
1880+
(select 0 as val0) as ss0
1881+
left join (select 1 as val) as ss1 on true
1882+
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
1883+
18721884
-- case that breaks the old ph_may_need optimization
18731885
explain (verbose, costs off)
18741886
select c.*,a.*,ss1.q1,ss2.q1,ss3.* from

0 commit comments

Comments
 (0)