Skip to content

Commit 30b4ccd

Browse files
tglsfdctglsfdc
committed
Further adjustments to PlaceHolderVar removal.
A new test case from Andreas Seltenreich showed that we were still a bit confused about removing PlaceHolderVars during join removal. Specifically, remove_rel_from_query would remove a PHV that was used only underneath the removable join, even if the place where it's used was the join partner relation and not the join clause being deleted. This would lead to a "too late to create a new PlaceHolderInfo" error later on. We can defend against that by checking ph_eval_at to see if the PHV could possibly be getting used at some partner rel. Also improve some nearby LATERAL-related logic. I decided that the check on ph_lateral needed to take precedence over the check on ph_needed, in case there's a lateral reference underneath the join being considered. (That may be impossible, but I'm not convinced of it, and it's easy enough to defend against the case.) Also, I realized that remove_rel_from_query's logic for updating LateralJoinInfos is dead code, because we don't build those at all until after join removal. Back-patch to 9.3. Previous versions didn't have the LATERAL issues, of course, and they also didn't attempt to remove PlaceHolderInfos during join removal. (I'm starting to wonder if changing that was really such a great idea.)
1 parent e921546 commit 30b4ccd

File tree

3 files changed

+83
-27
lines changed

3 files changed

+83
-27
lines changed

‎src/backend/optimizer/plan/analyzejoins.c

+17-27
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,10 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
210210
{
211211
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
212212

213-
if (bms_is_subset(phinfo->ph_needed, joinrelids))
214-
continue; /* PHV is not used above the join */
215213
if (bms_overlap(phinfo->ph_lateral, innerrel->relids))
216214
return false; /* it references innerrel laterally */
215+
if (bms_is_subset(phinfo->ph_needed, joinrelids))
216+
continue; /* PHV is not used above the join */
217217
if (!bms_overlap(phinfo->ph_eval_at, innerrel->relids))
218218
continue; /* it definitely doesn't reference innerrel */
219219
if (bms_is_subset(phinfo->ph_eval_at, innerrel->relids))
@@ -357,47 +357,37 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
357357
sjinfo->syn_righthand = bms_del_member(sjinfo->syn_righthand, relid);
358358
}
359359

360-
/*
361-
* Likewise remove references from LateralJoinInfo data structures.
362-
*
363-
* If we are deleting a LATERAL subquery, we can forget its
364-
* LateralJoinInfos altogether. Otherwise, make sure the target is not
365-
* included in any lateral_lhs set. (It probably can't be, since that
366-
* should have precluded deciding to remove it; but let's cope anyway.)
367-
*/
368-
for (l = list_head(root->lateral_info_list); l != NULL; l = nextl)
369-
{
370-
LateralJoinInfo *ljinfo = (LateralJoinInfo *) lfirst(l);
371-
372-
nextl = lnext(l);
373-
ljinfo->lateral_rhs = bms_del_member(ljinfo->lateral_rhs, relid);
374-
if (bms_is_empty(ljinfo->lateral_rhs))
375-
root->lateral_info_list = list_delete_ptr(root->lateral_info_list,
376-
ljinfo);
377-
else
378-
{
379-
ljinfo->lateral_lhs = bms_del_member(ljinfo->lateral_lhs, relid);
380-
Assert(!bms_is_empty(ljinfo->lateral_lhs));
381-
}
382-
}
360+
/* There shouldn't be any LATERAL info to translate, as yet */
361+
Assert(root->lateral_info_list == NIL);
383362

384363
/*
385364
* Likewise remove references from PlaceHolderVar data structures,
386365
* removing any no-longer-needed placeholders entirely.
366+
*
367+
* Removal is a bit tricker than it might seem: we can remove PHVs that
368+
* are used at the target rel and/or in the join qual, but not those that
369+
* are used at join partner rels or above the join. It's not that easy to
370+
* distinguish PHVs used at partner rels from those used in the join qual,
371+
* since they will both have ph_needed sets that are subsets of
372+
* joinrelids. However, a PHV used at a partner rel could not have the
373+
* target rel in ph_eval_at, so we check that while deciding whether to
374+
* remove or just update the PHV. There is no corresponding test in
375+
* join_is_removable because it doesn't need to distinguish those cases.
387376
*/
388377
for (l = list_head(root->placeholder_list); l != NULL; l = nextl)
389378
{
390379
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
391380

392381
nextl = lnext(l);
393-
if (bms_is_subset(phinfo->ph_needed, joinrelids))
382+
Assert(!bms_is_member(relid, phinfo->ph_lateral));
383+
if (bms_is_subset(phinfo->ph_needed, joinrelids) &&
384+
bms_is_member(relid, phinfo->ph_eval_at))
394385
root->placeholder_list = list_delete_ptr(root->placeholder_list,
395386
phinfo);
396387
else
397388
{
398389
phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, relid);
399390
Assert(!bms_is_empty(phinfo->ph_eval_at));
400-
Assert(!bms_is_member(relid, phinfo->ph_lateral));
401391
phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
402392
}
403393
}

‎src/test/regress/expected/join.out

+41
Original file line numberDiff line numberDiff line change
@@ -3759,6 +3759,47 @@ select t1.* from
37593759
Seq Scan on uniquetbl t1
37603760
(1 row)
37613761

3762+
explain (costs off)
3763+
select t0.*
3764+
from
3765+
text_tbl t0
3766+
left join
3767+
(select case t1.ten when 0 then 'doh!'::text else null::text end as case1,
3768+
t1.stringu2
3769+
from tenk1 t1
3770+
join int4_tbl i4 ON i4.f1 = t1.unique2
3771+
left join uniquetbl u1 ON u1.f1 = t1.string4) ss
3772+
on t0.f1 = ss.case1
3773+
where ss.stringu2 !~* ss.case1;
3774+
QUERY PLAN
3775+
--------------------------------------------------------------------------------------------
3776+
Nested Loop
3777+
Join Filter: (CASE t1.ten WHEN 0 THEN 'doh!'::text ELSE NULL::text END = t0.f1)
3778+
-> Nested Loop
3779+
-> Seq Scan on int4_tbl i4
3780+
-> Index Scan using tenk1_unique2 on tenk1 t1
3781+
Index Cond: (unique2 = i4.f1)
3782+
Filter: (stringu2 !~* CASE ten WHEN 0 THEN 'doh!'::text ELSE NULL::text END)
3783+
-> Materialize
3784+
-> Seq Scan on text_tbl t0
3785+
(9 rows)
3786+
3787+
select t0.*
3788+
from
3789+
text_tbl t0
3790+
left join
3791+
(select case t1.ten when 0 then 'doh!'::text else null::text end as case1,
3792+
t1.stringu2
3793+
from tenk1 t1
3794+
join int4_tbl i4 ON i4.f1 = t1.unique2
3795+
left join uniquetbl u1 ON u1.f1 = t1.string4) ss
3796+
on t0.f1 = ss.case1
3797+
where ss.stringu2 !~* ss.case1;
3798+
f1
3799+
------
3800+
doh!
3801+
(1 row)
3802+
37623803
rollback;
37633804
-- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
37643805
select * from

‎src/test/regress/sql/join.sql

+25
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,31 @@ select t1.* from
12091209
left join uniquetbl t3
12101210
on t2.d1 = t3.f1;
12111211

1212+
explain (costs off)
1213+
select t0.*
1214+
from
1215+
text_tbl t0
1216+
left join
1217+
(select case t1.ten when 0 then 'doh!'::text else null::text end as case1,
1218+
t1.stringu2
1219+
from tenk1 t1
1220+
join int4_tbl i4 ON i4.f1 = t1.unique2
1221+
left join uniquetbl u1 ON u1.f1 = t1.string4) ss
1222+
on t0.f1 = ss.case1
1223+
where ss.stringu2 !~* ss.case1;
1224+
1225+
select t0.*
1226+
from
1227+
text_tbl t0
1228+
left join
1229+
(select case t1.ten when 0 then 'doh!'::text else null::text end as case1,
1230+
t1.stringu2
1231+
from tenk1 t1
1232+
join int4_tbl i4 ON i4.f1 = t1.unique2
1233+
left join uniquetbl u1 ON u1.f1 = t1.string4) ss
1234+
on t0.f1 = ss.case1
1235+
where ss.stringu2 !~* ss.case1;
1236+
12121237
rollback;
12131238

12141239
-- bug #8444: we've historically allowed duplicate aliases within aliased JOINs

0 commit comments

Comments
 (0)