Skip to content

Commit 5c9d863

Browse files
committed
Avoid mislabeling of lateral references when pulling up a subquery.
If we are pulling up a subquery that's under an outer join, and the subquery's target list contains a strict expression that uses both a subquery variable and a lateral-reference variable, it's okay to pull up the expression without wrapping it in a PlaceHolderVar. That's safe because if the subquery variable is forced to NULL by the outer join, the expression result will come out as NULL too, so we don't have to force that outcome by evaluating the expression below the outer join. It'd be correct to wrap in a PHV, but that can lead to very significantly worse plans, since we'd then have to use a nestloop plan to pass down the lateral reference to where the expression will be evaluated. However, when we do that, we should not mark the lateral reference variable as being nulled by the outer join, because it isn't after we pull up the expression in this way. So the marking logic added by cb8e50a was incorrect in this detail, leading to "wrong varnullingrels" errors from the consistency-checking logic in setrefs.c. It seems to be sufficient to just not mark lateral references at all in this case. (I have a nagging feeling that more complexity may be needed in cases where there are several levels of outer join, but some attempts to break it with that didn't succeed.) Per report from Bertrand Mamasam. Back-patch to v16, as the previous patch was. Discussion: https://postgr.es/m/CACZ67_UA_EVrqiFXJu9XK50baEpH=ofEPJswa2kFxg6xuSw-ww@mail.gmail.com
1 parent 0c01f50 commit 5c9d863

File tree

3 files changed

+80
-2
lines changed

3 files changed

+80
-2
lines changed

src/backend/optimizer/prep/prepjointree.c

+17-2
Original file line numberDiff line numberDiff line change
@@ -2611,6 +2611,17 @@ pullup_replace_vars_callback(Var *var,
26112611
* the plan in cases where the nullingrels get removed again
26122612
* later by outer join reduction.
26132613
*
2614+
* Note that we don't force wrapping of expressions containing
2615+
* lateral references, so long as they also contain Vars/PHVs
2616+
* of the subquery. This is okay because of the restriction
2617+
* to strict constructs: if the subquery's Vars/PHVs have been
2618+
* forced to NULL by an outer join then the end result of the
2619+
* expression will be NULL too, regardless of the lateral
2620+
* references. So it's not necessary to force the expression
2621+
* to be evaluated below the outer join. This can be a very
2622+
* valuable optimization, because it may allow us to avoid
2623+
* using a nested loop to pass the lateral reference down.
2624+
*
26142625
* This analysis could be tighter: in particular, a non-strict
26152626
* construct hidden within a lower-level PlaceHolderVar is not
26162627
* reason to add another PHV. But for now it doesn't seem
@@ -2675,9 +2686,13 @@ pullup_replace_vars_callback(Var *var,
26752686
}
26762687
else
26772688
{
2678-
/* There should be lower-level Vars/PHVs we can modify */
2689+
/*
2690+
* There should be Vars/PHVs within the expression that we can
2691+
* modify. Per above discussion, modify only Vars/PHVs of the
2692+
* subquery, not lateral references.
2693+
*/
26792694
newnode = add_nulling_relids(newnode,
2680-
NULL, /* modify all Vars/PHVs */
2695+
rcon->relids,
26812696
var->varnullingrels);
26822697
/* Assert we did put the varnullingrels into the expression */
26832698
Assert(bms_is_subset(var->varnullingrels,

src/test/regress/expected/subselect.out

+47
Original file line numberDiff line numberDiff line change
@@ -1750,6 +1750,53 @@ where tname = 'tenk1' and attnum = 1;
17501750
tenk1 | unique1
17511751
(1 row)
17521752

1753+
-- Check behavior when there's a lateral reference in the output expression
1754+
explain (verbose, costs off)
1755+
select t1.ten, sum(x) from
1756+
tenk1 t1 left join lateral (
1757+
select t1.ten + t2.ten as x, t2.fivethous from tenk1 t2
1758+
) ss on t1.unique1 = ss.fivethous
1759+
group by t1.ten
1760+
order by t1.ten;
1761+
QUERY PLAN
1762+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1763+
Sort
1764+
Output: t1.ten, (sum((t1.ten + t2.ten)))
1765+
Sort Key: t1.ten
1766+
-> HashAggregate
1767+
Output: t1.ten, sum((t1.ten + t2.ten))
1768+
Group Key: t1.ten
1769+
-> Hash Right Join
1770+
Output: t1.ten, t2.ten
1771+
Hash Cond: (t2.fivethous = t1.unique1)
1772+
-> Seq Scan on public.tenk1 t2
1773+
Output: t2.unique1, t2.unique2, t2.two, t2.four, t2.ten, t2.twenty, t2.hundred, t2.thousand, t2.twothousand, t2.fivethous, t2.tenthous, t2.odd, t2.even, t2.stringu1, t2.stringu2, t2.string4
1774+
-> Hash
1775+
Output: t1.ten, t1.unique1
1776+
-> Seq Scan on public.tenk1 t1
1777+
Output: t1.ten, t1.unique1
1778+
(15 rows)
1779+
1780+
select t1.ten, sum(x) from
1781+
tenk1 t1 left join lateral (
1782+
select t1.ten + t2.ten as x, t2.fivethous from tenk1 t2
1783+
) ss on t1.unique1 = ss.fivethous
1784+
group by t1.ten
1785+
order by t1.ten;
1786+
ten | sum
1787+
-----+-------
1788+
0 | 0
1789+
1 | 2000
1790+
2 | 4000
1791+
3 | 6000
1792+
4 | 8000
1793+
5 | 10000
1794+
6 | 12000
1795+
7 | 14000
1796+
8 | 16000
1797+
9 | 18000
1798+
(10 rows)
1799+
17531800
--
17541801
-- Tests for CTE inlining behavior
17551802
--

src/test/regress/sql/subselect.sql

+16
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,22 @@ select relname::information_schema.sql_identifier as tname, * from
908908
right join pg_attribute a on a.attrelid = ss2.oid
909909
where tname = 'tenk1' and attnum = 1;
910910

911+
-- Check behavior when there's a lateral reference in the output expression
912+
explain (verbose, costs off)
913+
select t1.ten, sum(x) from
914+
tenk1 t1 left join lateral (
915+
select t1.ten + t2.ten as x, t2.fivethous from tenk1 t2
916+
) ss on t1.unique1 = ss.fivethous
917+
group by t1.ten
918+
order by t1.ten;
919+
920+
select t1.ten, sum(x) from
921+
tenk1 t1 left join lateral (
922+
select t1.ten + t2.ten as x, t2.fivethous from tenk1 t2
923+
) ss on t1.unique1 = ss.fivethous
924+
group by t1.ten
925+
order by t1.ten;
926+
911927
--
912928
-- Tests for CTE inlining behavior
913929
--

0 commit comments

Comments
 (0)