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

Commit a798660

Browse files
committed
Defend against bogus parameterization of join input paths.
An outer join cannot be formed using an input path that is parameterized by a value that is supposed to be nulled by the outer join. This is obviously nonsensical, and it could lead to a bad plan being selected; although currently it seems that we'll hit various sanity-check assertions first. I think that such cases were formerly prevented by the delay_upper_joins mechanism, but now that that's gone we need an explicit check. (Perhaps we should avoid generating baserel paths that could lead to this situation in the first place; but it seems like having a defense at the join level would be a good idea anyway.) Richard Guo and Tom Lane, per report from Jaime Casanova Discussion: https://postgr.es/m/CAJKUy5g2uZRrUDZJ8p-=giwcSHVUn0c9nmdxPSY0jF0Ov8VoEA@mail.gmail.com
1 parent 43af714 commit a798660

File tree

3 files changed

+78
-0
lines changed

3 files changed

+78
-0
lines changed

src/backend/optimizer/path/joinpath.c

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,17 @@ try_nestloop_path(PlannerInfo *root,
698698
Relids inner_paramrels = PATH_REQ_OUTER(inner_path);
699699
Relids outer_paramrels = PATH_REQ_OUTER(outer_path);
700700

701+
/*
702+
* If we are forming an outer join at this join, it's nonsensical to use
703+
* an input path that uses the outer join as part of its parameterization.
704+
* (This can happen despite our join order restrictions, since those apply
705+
* to what is in an input relation not what its parameters are.)
706+
*/
707+
if (extra->sjinfo->ojrelid != 0 &&
708+
(bms_is_member(extra->sjinfo->ojrelid, inner_paramrels) ||
709+
bms_is_member(extra->sjinfo->ojrelid, outer_paramrels)))
710+
return;
711+
701712
/*
702713
* Paths are parameterized by top-level parents, so run parameterization
703714
* tests on the parent relids.
@@ -908,6 +919,17 @@ try_mergejoin_path(PlannerInfo *root,
908919
return;
909920
}
910921

922+
/*
923+
* If we are forming an outer join at this join, it's nonsensical to use
924+
* an input path that uses the outer join as part of its parameterization.
925+
* (This can happen despite our join order restrictions, since those apply
926+
* to what is in an input relation not what its parameters are.)
927+
*/
928+
if (extra->sjinfo->ojrelid != 0 &&
929+
(bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) ||
930+
bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path))))
931+
return;
932+
911933
/*
912934
* Check to see if proposed path is still parameterized, and reject if the
913935
* parameterization wouldn't be sensible.
@@ -1054,6 +1076,17 @@ try_hashjoin_path(PlannerInfo *root,
10541076
Relids required_outer;
10551077
JoinCostWorkspace workspace;
10561078

1079+
/*
1080+
* If we are forming an outer join at this join, it's nonsensical to use
1081+
* an input path that uses the outer join as part of its parameterization.
1082+
* (This can happen despite our join order restrictions, since those apply
1083+
* to what is in an input relation not what its parameters are.)
1084+
*/
1085+
if (extra->sjinfo->ojrelid != 0 &&
1086+
(bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) ||
1087+
bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path))))
1088+
return;
1089+
10571090
/*
10581091
* Check to see if proposed path is still parameterized, and reject if the
10591092
* parameterization wouldn't be sensible.

src/test/regress/expected/join.out

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5063,6 +5063,29 @@ select 1 from
50635063
----------
50645064
(0 rows)
50655065

5066+
--
5067+
-- check a case where we formerly generated invalid parameterized paths
5068+
--
5069+
begin;
5070+
create temp table t (a int unique);
5071+
explain (costs off)
5072+
select 1 from t t1
5073+
join lateral (select t1.a from (select 1) foo offset 0) as s1 on true
5074+
join
5075+
(select 1 from t t2
5076+
inner join (t t3
5077+
left join (t t4 left join t t5 on t4.a = 1)
5078+
on t3.a = t4.a)
5079+
on false
5080+
where t3.a = coalesce(t5.a,1)) as s2
5081+
on true;
5082+
QUERY PLAN
5083+
--------------------------
5084+
Result
5085+
One-Time Filter: false
5086+
(2 rows)
5087+
5088+
rollback;
50665089
--
50675090
-- check a case in which a PlaceHolderVar forces join order
50685091
--

src/test/regress/sql/join.sql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,6 +1751,28 @@ select 1 from
17511751
on false,
17521752
lateral (select i4.f1, ss1.n from int8_tbl as i8 limit 1) as ss3;
17531753

1754+
--
1755+
-- check a case where we formerly generated invalid parameterized paths
1756+
--
1757+
1758+
begin;
1759+
1760+
create temp table t (a int unique);
1761+
1762+
explain (costs off)
1763+
select 1 from t t1
1764+
join lateral (select t1.a from (select 1) foo offset 0) as s1 on true
1765+
join
1766+
(select 1 from t t2
1767+
inner join (t t3
1768+
left join (t t4 left join t t5 on t4.a = 1)
1769+
on t3.a = t4.a)
1770+
on false
1771+
where t3.a = coalesce(t5.a,1)) as s2
1772+
on true;
1773+
1774+
rollback;
1775+
17541776
--
17551777
-- check a case in which a PlaceHolderVar forces join order
17561778
--

0 commit comments

Comments
 (0)