postgres.git / summary / log / commit / refs
commit a798660ebe3ff1feb310db13b957c5cda4c8c50d
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu Jun 29 16:12:52 2023 +0000
First released in: 16.0
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
src/backend/optimizer/path/joinpath.c | 33 +++++++++++++++++++++++++++++++++
src/test/regress/expected/join.out | 23 +++++++++++++++++++++++
src/test/regress/sql/join.sql | 22 ++++++++++++++++++++++
3 files changed, 78 insertions(+)
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index c2f211a60d1..f047ad9ba46 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -698,6 +698,17 @@ try_nestloop_path(PlannerInfo *root,
Relids inner_paramrels = PATH_REQ_OUTER(inner_path);
Relids outer_paramrels = PATH_REQ_OUTER(outer_path);
+ /*
+ * If we are forming an outer join at this join, it's nonsensical to use
+ * an input path that uses the outer join as part of its parameterization.
+ * (This can happen despite our join order restrictions, since those apply
+ * to what is in an input relation not what its parameters are.)
+ */
+ if (extra->sjinfo->ojrelid != 0 &&
+ (bms_is_member(extra->sjinfo->ojrelid, inner_paramrels) ||
+ bms_is_member(extra->sjinfo->ojrelid, outer_paramrels)))
+ return;
+
/*
* Paths are parameterized by top-level parents, so run parameterization
* tests on the parent relids.
@@ -908,6 +919,17 @@ try_mergejoin_path(PlannerInfo *root,
return;
}
+ /*
+ * If we are forming an outer join at this join, it's nonsensical to use
+ * an input path that uses the outer join as part of its parameterization.
+ * (This can happen despite our join order restrictions, since those apply
+ * to what is in an input relation not what its parameters are.)
+ */
+ if (extra->sjinfo->ojrelid != 0 &&
+ (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) ||
+ bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path))))
+ return;
+
/*
* Check to see if proposed path is still parameterized, and reject if the
* parameterization wouldn't be sensible.
@@ -1054,6 +1076,17 @@ try_hashjoin_path(PlannerInfo *root,
Relids required_outer;
JoinCostWorkspace workspace;
+ /*
+ * If we are forming an outer join at this join, it's nonsensical to use
+ * an input path that uses the outer join as part of its parameterization.
+ * (This can happen despite our join order restrictions, since those apply
+ * to what is in an input relation not what its parameters are.)
+ */
+ if (extra->sjinfo->ojrelid != 0 &&
+ (bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(inner_path)) ||
+ bms_is_member(extra->sjinfo->ojrelid, PATH_REQ_OUTER(outer_path))))
+ return;
+
/*
* Check to see if proposed path is still parameterized, and reject if the
* parameterization wouldn't be sensible.
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 6917faec141..9b8638f286a 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -5063,6 +5063,29 @@ select 1 from
----------
(0 rows)
+--
+-- check a case where we formerly generated invalid parameterized paths
+--
+begin;
+create temp table t (a int unique);
+explain (costs off)
+select 1 from t t1
+ join lateral (select t1.a from (select 1) foo offset 0) as s1 on true
+ join
+ (select 1 from t t2
+ inner join (t t3
+ left join (t t4 left join t t5 on t4.a = 1)
+ on t3.a = t4.a)
+ on false
+ where t3.a = coalesce(t5.a,1)) as s2
+ on true;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+rollback;
--
-- check a case in which a PlaceHolderVar forces join order
--
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 55080bec9af..3e5032b04dd 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1751,6 +1751,28 @@ select 1 from
on false,
lateral (select i4.f1, ss1.n from int8_tbl as i8 limit 1) as ss3;
+--
+-- check a case where we formerly generated invalid parameterized paths
+--
+
+begin;
+
+create temp table t (a int unique);
+
+explain (costs off)
+select 1 from t t1
+ join lateral (select t1.a from (select 1) foo offset 0) as s1 on true
+ join
+ (select 1 from t t2
+ inner join (t t3
+ left join (t t4 left join t t5 on t4.a = 1)
+ on t3.a = t4.a)
+ on false
+ where t3.a = coalesce(t5.a,1)) as s2
+ on true;
+
+rollback;
+
--
-- check a case in which a PlaceHolderVar forces join order
--
[parent: 43af714defa0]