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

3 files changed, 78 insertions, 0 deletions

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
@@ -699,6 +699,17 @@ try_nestloop_path(PlannerInfo *root,
 	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.
 	 */
@@ -909,6 +920,17 @@ try_mergejoin_path(PlannerInfo *root,
 	}
 
 	/*
+	 * 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.
 	 */
@@ -1055,6 +1077,17 @@ try_hashjoin_path(PlannerInfo *root,
 	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
@@ -5064,6 +5064,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
 --
 explain (verbose, costs off)
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
@@ -1752,6 +1752,28 @@ select 1 from
   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
 --