Fix "wrong varnullingrels" for subquery nestloop parameters.

If we apply outer join identity 3 when relation C is a subquery
having lateral references to relation B, then the lateral references
within C continue to bear the original syntactically-correct
varnullingrels marks, but that won't match what is available from
the outer side of the nestloop.  Compensate for that in
process_subquery_nestloop_params().  This is a slightly hacky fix,
but we certainly don't want to re-plan C in toto for each possible
outer join order, so there's not a lot of better alternatives.

Richard Guo and Tom Lane, per report from Markus Winand

Discussion: https://postgr.es/m/DFBB2D25-DE97-49CA-A60E-07C881EA59A7@winand.at

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 1ca26baa2599f0b9b9c3ce86d0b6f155257880a5..3585a703fbd66703ea112c6c85bbcf49d8110b0e 100644 (file)
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -2289,9 +2289,11 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
             * the outer-join level at which they are used, Vars seen in the
             * NestLoopParam expression may have nullingrels that are just a
             * subset of those in the Vars actually available from the outer
-            * side.  Not checking this exactly is a bit grotty, but the work
-            * needed to make things match up perfectly seems well out of
-            * proportion to the value.
+            * side.  Another case that can cause that to happen is explained
+            * in the comments for process_subquery_nestloop_params.  Not
+            * checking this exactly is a bit grotty, but the work needed to
+            * make things match up perfectly seems well out of proportion to
+            * the value.
             */
            nlp->paramval = (Var *) fix_upper_expr(root,
                                                   (Node *) nlp->paramval,

diff --git a/src/backend/optimizer/util/paramassign.c b/src/backend/optimizer/util/paramassign.c
index 66534c0a7820fd173ed7e814ab2b0536516ffcaa..e94f7e7563d578033823c256d2a6e0ad6bd296e8 100644 (file)
--- a/src/backend/optimizer/util/paramassign.c
+++ b/src/backend/optimizer/util/paramassign.c
@@ -421,8 +421,26 @@ replace_nestloop_param_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
  * provide these values.  This differs from replace_nestloop_param_var in
  * that the PARAM_EXEC slots to use have already been determined.
  *
+ * An additional complication is that the subplan_params may contain
+ * nullingrel markers that need adjustment.  This occurs if we have applied
+ * outer join identity 3,
+ *     (A leftjoin B on (Pab)) leftjoin C on (Pb*c)
+ *     = A leftjoin (B leftjoin C on (Pbc)) on (Pab)
+ * and C is a subquery containing lateral references to B.  It's still safe
+ * to apply the identity, but the parser will have created those references
+ * in the form "b*" (i.e., with varnullingrels listing the A/B join), while
+ * what we will have available from the nestloop's outer side is just "b".
+ * We deal with that here by stripping the nullingrels down to what is
+ * available from the outer side according to root->curOuterRels.
+ * That fixes matters for the case of forward application of identity 3.
+ * If the identity was applied in the reverse direction, we will have
+ * subplan_params containing too few nullingrel bits rather than too many.
+ * Currently, that causes no problems because setrefs.c applies only a
+ * subset check to nullingrels in NestLoopParams, but we'd have to work
+ * harder if we ever want to tighten that check.
+ *
  * Note that we also use root->curOuterRels as an implicit parameter for
- * sanity checks.
+ * sanity checks and nullingrel adjustments.
  */
 void
 process_subquery_nestloop_params(PlannerInfo *root, List *subplan_params)
@@ -449,17 +467,19 @@ process_subquery_nestloop_params(PlannerInfo *root, List *subplan_params)
                nlp = (NestLoopParam *) lfirst(lc2);
                if (nlp->paramno == pitem->paramId)
                {
-                   Assert(equal(var, nlp->paramval));
                    /* Present, so nothing to do */
                    break;
                }
            }
            if (lc2 == NULL)
            {
-               /* No, so add it */
+               /* No, so add it after adjusting its nullingrels */
+               var = copyObject(var);
+               var->varnullingrels = bms_intersect(var->varnullingrels,
+                                                   root->curOuterRels);
                nlp = makeNode(NestLoopParam);
                nlp->paramno = pitem->paramId;
-               nlp->paramval = copyObject(var);
+               nlp->paramval = var;
                root->curOuterParams = lappend(root->curOuterParams, nlp);
            }
        }
@@ -480,17 +500,19 @@ process_subquery_nestloop_params(PlannerInfo *root, List *subplan_params)
                nlp = (NestLoopParam *) lfirst(lc2);
                if (nlp->paramno == pitem->paramId)
                {
-                   Assert(equal(phv, nlp->paramval));
                    /* Present, so nothing to do */
                    break;
                }
            }
            if (lc2 == NULL)
            {
-               /* No, so add it */
+               /* No, so add it after adjusting its nullingrels */
+               phv = copyObject(phv);
+               phv->phnullingrels = bms_intersect(phv->phnullingrels,
+                                                  root->curOuterRels);
                nlp = makeNode(NestLoopParam);
                nlp->paramno = pitem->paramId;
-               nlp->paramval = (Var *) copyObject(phv);
+               nlp->paramval = (Var *) phv;
                root->curOuterParams = lappend(root->curOuterParams, nlp);
            }
        }

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index d04648df3fde4fa04a72d1915463cc98bfb7e51e..4999c99f3bcde046e38bf2fda4433063e98f1cbd 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2589,6 +2589,24 @@ on t2.q2 = 123;
                ->  Seq Scan on int8_tbl t5
 (12 rows)
 
+explain (costs off)
+select * from int8_tbl t1
+    left join int8_tbl t2 on true
+    left join lateral
+      (select * from int8_tbl t3 where t3.q1 = t2.q1 offset 0) s
+      on t2.q1 = 1;
+                QUERY PLAN                 
+-------------------------------------------
+ Nested Loop Left Join
+   ->  Seq Scan on int8_tbl t1
+   ->  Materialize
+         ->  Nested Loop Left Join
+               Join Filter: (t2.q1 = 1)
+               ->  Seq Scan on int8_tbl t2
+               ->  Seq Scan on int8_tbl t3
+                     Filter: (q1 = t2.q1)
+(8 rows)
+
 --
 -- check a case where we formerly got confused by conflicting sort orders
 -- in redundant merge join path keys

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 0308258a917e417db8e0972d922ab789d7902a2f..56ca759772b1b933e8b99c908b1afe13387d313a 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -514,6 +514,13 @@ select * from int8_tbl t1 left join
     left join int8_tbl t5 on t2.q1 = t5.q1
 on t2.q2 = 123;
 
+explain (costs off)
+select * from int8_tbl t1
+    left join int8_tbl t2 on true
+    left join lateral
+      (select * from int8_tbl t3 where t3.q1 = t2.q1 offset 0) s
+      on t2.q1 = 1;
+
 --
 -- check a case where we formerly got confused by conflicting sort orders
 -- in redundant merge join path keys