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

Commit 29f114b

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 a7097ca commit 29f114b

File tree

3 files changed

+46
-2
lines changed

3 files changed

+46
-2
lines changed

src/backend/optimizer/prep/prepjointree.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2435,8 +2435,13 @@ pullup_replace_vars_callback(Var *var,
24352435
else if (newnode && IsA(newnode, PlaceHolderVar) &&
24362436
((PlaceHolderVar *) newnode)->phlevelsup == 0)
24372437
{
2438-
/* No need to wrap a PlaceHolderVar with another one, either */
2439-
wrap = false;
2438+
/* The same rules apply for a PlaceHolderVar */
2439+
if (rcon->target_rte->lateral &&
2440+
!bms_is_subset(((PlaceHolderVar *) newnode)->phrels,
2441+
rcon->relids))
2442+
wrap = true;
2443+
else
2444+
wrap = false;
24402445
}
24412446
else
24422447
{

src/test/regress/expected/join.out

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7980,6 +7980,33 @@ select * from
79807980
Output: (COALESCE((COALESCE(b.q2, '42'::bigint)), d.q2))
79817981
(24 rows)
79827982

7983+
-- another case requiring nested PlaceHolderVars
7984+
explain (verbose, costs off)
7985+
select * from
7986+
(select 0 as val0) as ss0
7987+
left join (select 1 as val) as ss1 on true
7988+
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
7989+
QUERY PLAN
7990+
--------------------------------
7991+
Nested Loop Left Join
7992+
Output: 0, (1), ((1))
7993+
Join Filter: false
7994+
-> Result
7995+
Output: 1
7996+
-> Result
7997+
Output: (1)
7998+
One-Time Filter: false
7999+
(8 rows)
8000+
8001+
select * from
8002+
(select 0 as val0) as ss0
8003+
left join (select 1 as val) as ss1 on true
8004+
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
8005+
val0 | val | val_filtered
8006+
------+-----+--------------
8007+
0 | 1 |
8008+
(1 row)
8009+
79838010
-- case that breaks the old ph_may_need optimization
79848011
explain (verbose, costs off)
79858012
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
@@ -2950,6 +2950,18 @@ select * from
29502950
) on c.q2 = ss2.q1,
29512951
lateral (select ss2.y offset 0) ss3;
29522952

2953+
-- another case requiring nested PlaceHolderVars
2954+
explain (verbose, costs off)
2955+
select * from
2956+
(select 0 as val0) as ss0
2957+
left join (select 1 as val) as ss1 on true
2958+
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
2959+
2960+
select * from
2961+
(select 0 as val0) as ss0
2962+
left join (select 1 as val) as ss1 on true
2963+
left join lateral (select ss1.val as val_filtered where false) as ss2 on true;
2964+
29532965
-- case that breaks the old ph_may_need optimization
29542966
explain (verbose, costs off)
29552967
select c.*,a.*,ss1.q1,ss2.q1,ss3.* from

0 commit comments

Comments
 (0)