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

Commit bab163e

Browse files
committed
Fix old oversight in join removal logic.
Commit 9e7e29c introduced an Assert that join removal didn't reduce the eval_at set of any PlaceHolderVar to empty. At first glance it looks like join_is_removable ensures that's true --- but actually, the loop in join_is_removable skips PlaceHolderVars that are not referenced above the join due to be removed. So, if we don't want any empty eval_at sets, the right thing to do is to delete any now-unreferenced PlaceHolderVars from the data structure entirely. Per fuzz testing by Andreas Seltenreich. Back-patch to 9.3 where the aforesaid Assert was added.
1 parent 58e09b9 commit bab163e

File tree

3 files changed

+45
-6
lines changed

3 files changed

+45
-6
lines changed

src/backend/optimizer/plan/analyzejoins.c

+14-6
Original file line numberDiff line numberDiff line change
@@ -464,16 +464,24 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
464464
}
465465

466466
/*
467-
* Likewise remove references from PlaceHolderVar data structures.
467+
* Likewise remove references from PlaceHolderVar data structures,
468+
* removing any no-longer-needed placeholders entirely.
468469
*/
469-
foreach(l, root->placeholder_list)
470+
for (l = list_head(root->placeholder_list); l != NULL; l = nextl)
470471
{
471472
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
472473

473-
phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, relid);
474-
Assert(!bms_is_empty(phinfo->ph_eval_at));
475-
Assert(!bms_is_member(relid, phinfo->ph_lateral));
476-
phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
474+
nextl = lnext(l);
475+
if (bms_is_subset(phinfo->ph_needed, joinrelids))
476+
root->placeholder_list = list_delete_ptr(root->placeholder_list,
477+
phinfo);
478+
else
479+
{
480+
phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, relid);
481+
Assert(!bms_is_empty(phinfo->ph_eval_at));
482+
Assert(!bms_is_member(relid, phinfo->ph_lateral));
483+
phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
484+
}
477485
}
478486

479487
/*

src/test/regress/expected/join.out

+16
Original file line numberDiff line numberDiff line change
@@ -3862,6 +3862,22 @@ SELECT * FROM
38623862
1 | 4567890123456789 | -4567890123456789 | 4567890123456789
38633863
(5 rows)
38643864

3865+
rollback;
3866+
-- another join removal bug: we must clean up correctly when removing a PHV
3867+
begin;
3868+
create temp table uniquetbl (f1 text unique);
3869+
explain (costs off)
3870+
select t1.* from
3871+
uniquetbl as t1
3872+
left join (select *, '***'::text as d1 from uniquetbl) t2
3873+
on t1.f1 = t2.f1
3874+
left join uniquetbl t3
3875+
on t2.d1 = t3.f1;
3876+
QUERY PLAN
3877+
--------------------------
3878+
Seq Scan on uniquetbl t1
3879+
(1 row)
3880+
38653881
rollback;
38663882
-- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
38673883
select * from

src/test/regress/sql/join.sql

+15
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,21 @@ SELECT * FROM
12471247

12481248
rollback;
12491249

1250+
-- another join removal bug: we must clean up correctly when removing a PHV
1251+
begin;
1252+
1253+
create temp table uniquetbl (f1 text unique);
1254+
1255+
explain (costs off)
1256+
select t1.* from
1257+
uniquetbl as t1
1258+
left join (select *, '***'::text as d1 from uniquetbl) t2
1259+
on t1.f1 = t2.f1
1260+
left join uniquetbl t3
1261+
on t2.d1 = t3.f1;
1262+
1263+
rollback;
1264+
12501265
-- bug #8444: we've historically allowed duplicate aliases within aliased JOINs
12511266

12521267
select * from

0 commit comments

Comments
 (0)