Skip to content

Commit 0853388

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 caf89b3 commit 0853388

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
@@ -240,10 +240,10 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
240240
{
241241
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
242242

243-
if (bms_is_subset(phinfo->ph_needed, joinrelids))
244-
continue; /* PHV is not used above the join */
245243
if (bms_overlap(phinfo->ph_lateral, innerrel->relids))
246244
return false; /* it references innerrel laterally */
245+
if (bms_is_subset(phinfo->ph_needed, joinrelids))
246+
continue; /* PHV is not used above the join */
247247
if (!bms_overlap(phinfo->ph_eval_at, innerrel->relids))
248248
continue; /* it definitely doesn't reference innerrel */
249249
if (bms_is_subset(phinfo->ph_eval_at, innerrel->relids))
@@ -439,47 +439,37 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
439439
sjinfo->syn_righthand = bms_del_member(sjinfo->syn_righthand, relid);
440440
}
441441

442-
/*
443-
* Likewise remove references from LateralJoinInfo data structures.
444-
*
445-
* If we are deleting a LATERAL subquery, we can forget its
446-
* LateralJoinInfos altogether. Otherwise, make sure the target is not
447-
* included in any lateral_lhs set. (It probably can't be, since that
448-
* should have precluded deciding to remove it; but let's cope anyway.)
449-
*/
450-
for (l = list_head(root->lateral_info_list); l != NULL; l = nextl)
451-
{
452-
LateralJoinInfo *ljinfo = (LateralJoinInfo *) lfirst(l);
453-
454-
nextl = lnext(l);
455-
ljinfo->lateral_rhs = bms_del_member(ljinfo->lateral_rhs, relid);
456-
if (bms_is_empty(ljinfo->lateral_rhs))
457-
root->lateral_info_list = list_delete_ptr(root->lateral_info_list,
458-
ljinfo);
459-
else
460-
{
461-
ljinfo->lateral_lhs = bms_del_member(ljinfo->lateral_lhs, relid);
462-
Assert(!bms_is_empty(ljinfo->lateral_lhs));
463-
}
464-
}
442+
/* There shouldn't be any LATERAL info to translate, as yet */
443+
Assert(root->lateral_info_list == NIL);
465444

466445
/*
467446
* Likewise remove references from PlaceHolderVar data structures,
468447
* removing any no-longer-needed placeholders entirely.
448+
*
449+
* Removal is a bit tricker than it might seem: we can remove PHVs that
450+
* are used at the target rel and/or in the join qual, but not those that
451+
* are used at join partner rels or above the join. It's not that easy to
452+
* distinguish PHVs used at partner rels from those used in the join qual,
453+
* since they will both have ph_needed sets that are subsets of
454+
* joinrelids. However, a PHV used at a partner rel could not have the
455+
* target rel in ph_eval_at, so we check that while deciding whether to
456+
* remove or just update the PHV. There is no corresponding test in
457+
* join_is_removable because it doesn't need to distinguish those cases.
469458
*/
470459
for (l = list_head(root->placeholder_list); l != NULL; l = nextl)
471460
{
472461
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
473462

474463
nextl = lnext(l);
475-
if (bms_is_subset(phinfo->ph_needed, joinrelids))
464+
Assert(!bms_is_member(relid, phinfo->ph_lateral));
465+
if (bms_is_subset(phinfo->ph_needed, joinrelids) &&
466+
bms_is_member(relid, phinfo->ph_eval_at))
476467
root->placeholder_list = list_delete_ptr(root->placeholder_list,
477468
phinfo);
478469
else
479470
{
480471
phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, relid);
481472
Assert(!bms_is_empty(phinfo->ph_eval_at));
482-
Assert(!bms_is_member(relid, phinfo->ph_lateral));
483473
phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
484474
}
485475
}

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

+41
Original file line numberDiff line numberDiff line change
@@ -3878,6 +3878,47 @@ select t1.* from
38783878
Seq Scan on uniquetbl t1
38793879
(1 row)
38803880

3881+
explain (costs off)
3882+
select t0.*
3883+
from
3884+
text_tbl t0
3885+
left join
3886+
(select case t1.ten when 0 then 'doh!'::text else null::text end as case1,
3887+
t1.stringu2
3888+
from tenk1 t1
3889+
join int4_tbl i4 ON i4.f1 = t1.unique2
3890+
left join uniquetbl u1 ON u1.f1 = t1.string4) ss
3891+
on t0.f1 = ss.case1
3892+
where ss.stringu2 !~* ss.case1;
3893+
QUERY PLAN
3894+
--------------------------------------------------------------------------------------------
3895+
Nested Loop
3896+
Join Filter: (CASE t1.ten WHEN 0 THEN 'doh!'::text ELSE NULL::text END = t0.f1)
3897+
-> Nested Loop
3898+
-> Seq Scan on int4_tbl i4
3899+
-> Index Scan using tenk1_unique2 on tenk1 t1
3900+
Index Cond: (unique2 = i4.f1)
3901+
Filter: (stringu2 !~* CASE ten WHEN 0 THEN 'doh!'::text ELSE NULL::text END)
3902+
-> Materialize
3903+
-> Seq Scan on text_tbl t0
3904+
(9 rows)
3905+
3906+
select t0.*
3907+
from
3908+
text_tbl t0
3909+
left join
3910+
(select case t1.ten when 0 then 'doh!'::text else null::text end as case1,
3911+
t1.stringu2
3912+
from tenk1 t1
3913+
join int4_tbl i4 ON i4.f1 = t1.unique2
3914+
left join uniquetbl u1 ON u1.f1 = t1.string4) ss
3915+
on t0.f1 = ss.case1
3916+
where ss.stringu2 !~* ss.case1;
3917+
f1
3918+
------
3919+
doh!
3920+
(1 row)
3921+
38813922
rollback;
38823923
-- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
38833924
select * from

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

+25
Original file line numberDiff line numberDiff line change
@@ -1260,6 +1260,31 @@ select t1.* from
12601260
left join uniquetbl t3
12611261
on t2.d1 = t3.f1;
12621262

1263+
explain (costs off)
1264+
select t0.*
1265+
from
1266+
text_tbl t0
1267+
left join
1268+
(select case t1.ten when 0 then 'doh!'::text else null::text end as case1,
1269+
t1.stringu2
1270+
from tenk1 t1
1271+
join int4_tbl i4 ON i4.f1 = t1.unique2
1272+
left join uniquetbl u1 ON u1.f1 = t1.string4) ss
1273+
on t0.f1 = ss.case1
1274+
where ss.stringu2 !~* ss.case1;
1275+
1276+
select t0.*
1277+
from
1278+
text_tbl t0
1279+
left join
1280+
(select case t1.ten when 0 then 'doh!'::text else null::text end as case1,
1281+
t1.stringu2
1282+
from tenk1 t1
1283+
join int4_tbl i4 ON i4.f1 = t1.unique2
1284+
left join uniquetbl u1 ON u1.f1 = t1.string4) ss
1285+
on t0.f1 = ss.case1
1286+
where ss.stringu2 !~* ss.case1;
1287+
12631288
rollback;
12641289

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

0 commit comments

Comments
 (0)