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

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index c2f211a60d13c32fff1ec1713e9fccd7e183f4dd..f047ad9ba4688601bc24a73d58af6463b362c659 100644 (file)
--- 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 6917faec141943b6d9ef6debeada8476454d8995..9b8638f286ab2f63a5608b2eeceb0e0c2834d186 100644 (file)
--- 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 55080bec9af8953decd58e82411987e1b562d830..3e5032b04dd839550f445017f261ed1cadedae6a 100644 (file)
--- 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
 --