postgres.git / summary / log / commit / refs

commit    80bece312c4b957ea5a93db84be1d1776f0e5e67
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Thu Jan 11 20:28:13 2024 +0000
First released in: 17.0, 12.18, 13.14, 14.11, 15.6, 16.2

    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


src/backend/optimizer/prep/prepjointree.c | 9 +++++++-- src/test/regress/expected/join.out | 27 +++++++++++++++++++++++++++ src/test/regress/sql/join.sql | 12 ++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 73ff40721c9..57262f9c64e 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -2435,8 +2435,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 { diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index e3f29b1d16c..3715ae491d5 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -7013,6 +7013,33 @@ 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)) + Join Filter: false + -> Result + Output: 1 + -> Result + Output: (1) + One-Time Filter: false +(8 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 8bf18f11bc8..3443b21e6b0 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2522,6 +2522,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 [parent: af36f1993da1]