Обсуждение: ERROR: wrong varnullingrels (b 3) (expected (b)) for Var 2/1

I found an error similar to others before ([1]) that is still persists as of head right now (0bcb3ca3b9).

CREATE TABLE t (
    n INTEGER
);

SELECT *
  FROM (VALUES (1)) t(c)
  LEFT JOIN t ljl1 ON true
  LEFT JOIN LATERAL (WITH cte AS (SELECT * FROM t WHERE t.n = ljl1.n) SELECT * FROM cte) ljl2 ON ljl1.n = 1;

ERROR:  wrong varnullingrels (b 3) (expected (b)) for Var 2/1

Note that the error does **not** occur if the CTE is unwrapped like this:

SELECT *
  FROM (VALUES (1)) t(c)
  LEFT JOIN t ljl1 ON true
  LEFT JOIN LATERAL (SELECT * FROM t WHERE t.n = ljl1.n) ljl2 ON ljl1.n = 1;

-markus

[1] https://www.postgresql.org/message-id/CAHewXNnu7u1aT==WjnCRa+SzKb6s80hvwPP_9eMvvvtdyFdqjw@mail.gmail.com

> On 31.05.2023, at 08:36, Richard Guo <guofenglinux@gmail.com> wrote:
>
> Attached is a patch for that.  Does this make sense?
>
> Thanks
> Richard
> <v1-0001-Fix-nulling-bitmap-for-nestloop-parameters.patch>

All I can say is that it fixes the error for me — also for the non-simplified original query that I have.

-markus

Richard Guo <guofenglinux@gmail.com> writes:
> On Wed, May 31, 2023 at 10:47 AM Richard Guo <guofenglinux@gmail.com> wrote:
>> When we transform the first form of identity 3 to the second form, we've
>> converted Pb*c to Pbc in deconstruct_distribute_oj_quals.  But we
>> neglect to consider that rel C might be a RTE_SUBQUERY and contains
>> quals that have lateral references to B.  So the B vars in such quals
>> have wrong nulling bitmaps and we'd finally notice that when we do
>> fix_upper_expr for the NestLoopParam expressions.

Right.  One question that immediately raises is whether it's even safe
to apply the identity if C has lateral references to B, because that
almost certainly means that C will produce different output when
joined to a nulled B row than when joined to a not-nulled row.
I think that's okay because if the B row will fail Pab then it doesn't
matter what row(s) C produces, but maybe I've missed something.

> We can identify in which form of identity 3 the plan is built up by
> checking the relids of the B/C join's outer rel.  If it's in the first
> form, the outer rel's relids must contain the A/B join.  Otherwise it
> should only contain B's relid.  So I'm considering that maybe we can
> adjust the nulling bitmap for nestloop parameters according to that.
> Attached is a patch for that.  Does this make sense?

Hmm.  I don't really want to do it in identify_current_nestloop_params
because that gets applied to all nestloop params, so it seems like
that risks masking bugs of other kinds.  I'd rather do it in
process_subquery_nestloop_params, which we know is only applied to
subquery LATERAL references.  So more or less as attached.

(I dropped the equal() assertions in process_subquery_nestloop_params
because they've never caught anything and it'd be too complicated to
make them still work.)

Thoughts?

            regards, tom lane

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 1ca26baa25..3585a703fb 100644
--- 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 66534c0a78..e94f7e7563 100644
--- 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 d04648df3f..4999c99f3b 100644
--- 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 0308258a91..56ca759772 100644
--- 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

Richard Guo <guofenglinux@gmail.com> writes:
> Yeah, that makes sense.  process_subquery_nestloop_params is a better
> place to do this adjustments.  +1 to v2 patch.

Pushed, then.

            regards, tom lane

Richard Guo <guofenglinux@gmail.com> writes:
> Oh, wait ... It occurred to me that we may have this same issue with
> Memoize cache keys.  In get_memoize_path we collect the cache keys from
> innerpath's ppi_clauses and innerrel's lateral_vars, and the latter may
> contain nullingrel markers that need adjustment.  As an example,
> consider the query below

> explain (costs off)
> select * from onek t1
>     left join onek t2 on true
>     left join lateral
>       (select * from onek t3 where t3.two = t2.two offset 0) s
>       on t2.unique1 = 1;
> ERROR:  wrong varnullingrels (b 3) (expected (b)) for Var 2/3

Good catch --- I'll take a closer look tomorrow.

            regards, tom lane

I wrote:
> Richard Guo <guofenglinux@gmail.com> writes:
>> Oh, wait ... It occurred to me that we may have this same issue with
>> Memoize cache keys.

> Good catch --- I'll take a closer look tomorrow.

Pushed after a little more fiddling with the comments.

            regards, tom lane

Richard Guo <guofenglinux@gmail.com> writes:
> I just realized that we may still have holes in this area.  Until now
> we're mainly focusing on LATERAL subquery, in which case the lateral
> reference Vars are copied into rel->subplan_params and we've already
> adjusted the nulling bitmaps there.  But what about the lateral
> reference Vars in other cases?

Ugh.

> So it seems that we need to do nullingrel adjustments in a more common
> place.

I agree: this suggests that we fixed it in the wrong place.

> I think it exposes a new issue.  It seems that we extract a problematic
> lateral_relids from lateral references within PlaceHolderVars in
> create_lateral_join_info.  I doubt that we should use ph_lateral
> directly.  It seems more reasonable to me that we strip outer-join
> relids from ph_lateral and then use that for lateral_relids.

Hmm.  I don't have time to think hard about this today, but this
does feel similar to our existing decision that parameterized paths
should be generated with minimal nullingrels bits on their outer
references.  We only thought about pushed-down join clauses when we
did that.  But a lateral ref necessarily gives rise to parameterized
path(s), and what we seem to be seeing is that those need to be
handled just the same as ones generated by pushing down join clauses.

            regards, tom lane

I wrote:
> Richard Guo <guofenglinux@gmail.com> writes:
>> So it seems that we need to do nullingrel adjustments in a more common
>> place.

> I agree: this suggests that we fixed it in the wrong place.

So pursuant to that, 0001 attached reverts the code changes from bfd332b3f
and 63e4f13d2 (keeping the test cases and some unrelated comment fixes).
Then the question is what to do instead.  I've not come up with a better
idea than to hack it in identify_current_nestloop_params (per 0002), as
you proposed upthread.  I don't like this too much, as it's on the hairy
edge of making setrefs.c's nullingrel cross-checks completely useless for
NestLoopParams; but the alternatives aren't attractive either.

>> I think it exposes a new issue.  It seems that we extract a problematic
>> lateral_relids from lateral references within PlaceHolderVars in
>> create_lateral_join_info.  I doubt that we should use ph_lateral
>> directly.  It seems more reasonable to me that we strip outer-join
>> relids from ph_lateral and then use that for lateral_relids.

I experimented with that (0003) and it fixes your example query.
I think it is functionally okay, because the lateral_relids just need
to be a sufficient subset of the lateral references' requirements to
ensure we can evaluate them where needed; other mechanisms should ensure
that the right sorts of joins happen.  It seems a bit unsatisfying
though, especially given that we just largely lobotomized setrefs.c's
cross-checks for these same references.  I don't have a better idea
however, and beta2 is fast approaching.

            regards, tom lane

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index 5ba266fdb6..c2f211a60d 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -430,24 +430,6 @@ have_unsafe_outer_join_ref(PlannerInfo *root,
  * These are returned in parallel lists in *param_exprs and *operators.
  * We also set *binary_mode to indicate whether strict binary matching is
  * required.
- *
- * A complication is that innerrel's lateral_vars 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 contains 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 outerrel->relids.
- * That fixes matters for the case of forward application of identity 3.
- * If the identity was applied in the reverse direction, we will have
- * innerrel's lateral_vars 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.
  */
 static bool
 paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
@@ -551,25 +533,6 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
             return false;
         }

-        /* OK, but adjust its nullingrels before adding it to result */
-        expr = copyObject(expr);
-        if (IsA(expr, Var))
-        {
-            Var           *var = (Var *) expr;
-
-            var->varnullingrels = bms_intersect(var->varnullingrels,
-                                                outerrel->relids);
-        }
-        else if (IsA(expr, PlaceHolderVar))
-        {
-            PlaceHolderVar *phv = (PlaceHolderVar *) expr;
-
-            phv->phnullingrels = bms_intersect(phv->phnullingrels,
-                                               outerrel->relids);
-        }
-        else
-            Assert(false);
-
         *operators = lappend_oid(*operators, typentry->eq_opr);
         *param_exprs = lappend(*param_exprs, expr);

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index ec5552327f..1ca26baa25 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -2289,11 +2289,9 @@ 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.  Lateral references can create the same situation, as
-             * explained in the comments for process_subquery_nestloop_params
-             * and paraminfo_get_equal_hashops.  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.  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 e94f7e7563..66534c0a78 100644
--- a/src/backend/optimizer/util/paramassign.c
+++ b/src/backend/optimizer/util/paramassign.c
@@ -421,26 +421,8 @@ 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 and nullingrel adjustments.
+ * sanity checks.
  */
 void
 process_subquery_nestloop_params(PlannerInfo *root, List *subplan_params)
@@ -467,19 +449,17 @@ 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 after adjusting its nullingrels */
-                var = copyObject(var);
-                var->varnullingrels = bms_intersect(var->varnullingrels,
-                                                    root->curOuterRels);
+                /* No, so add it */
                 nlp = makeNode(NestLoopParam);
                 nlp->paramno = pitem->paramId;
-                nlp->paramval = var;
+                nlp->paramval = copyObject(var);
                 root->curOuterParams = lappend(root->curOuterParams, nlp);
             }
         }
@@ -500,19 +480,17 @@ 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 after adjusting its nullingrels */
-                phv = copyObject(phv);
-                phv->phnullingrels = bms_intersect(phv->phnullingrels,
-                                                   root->curOuterRels);
+                /* No, so add it */
                 nlp = makeNode(NestLoopParam);
                 nlp->paramno = pitem->paramId;
-                nlp->paramval = (Var *) phv;
+                nlp->paramval = (Var *) copyObject(phv);
                 root->curOuterParams = lappend(root->curOuterParams, nlp);
             }
         }
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 1ca26baa25..c63758cb2b 100644
--- 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.  (Lateral references can also cause this, as explained in
+             * the comments for identify_current_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 66534c0a78..d6a923b0b6 100644
--- a/src/backend/optimizer/util/paramassign.c
+++ b/src/backend/optimizer/util/paramassign.c
@@ -503,6 +503,28 @@ process_subquery_nestloop_params(PlannerInfo *root, List *subplan_params)
  * Identify any NestLoopParams that should be supplied by a NestLoop plan
  * node with the specified lefthand rels.  Remove them from the active
  * root->curOuterParams list and return them as the result list.
+ *
+ * XXX Here we also hack up the returned Vars and PHVs so that they do not
+ * contain nullingrel sets exceeding what is available from the outer side.
+ * This is needed 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 contains 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 leftrelids.
+ *
+ * That fixes matters for the case of forward application of identity 3.
+ * If the identity was applied in the reverse direction, we will have
+ * parameter Vars 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.  This is all pretty annoying
+ * because it greatly weakens setrefs.c's cross-check, but the alternative
+ * seems to be to generate multiple versions of each laterally-parameterized
+ * subquery, which'd be unduly expensive.
  */
 List *
 identify_current_nestloop_params(PlannerInfo *root, Relids leftrelids)
@@ -517,13 +539,19 @@ identify_current_nestloop_params(PlannerInfo *root, Relids leftrelids)

         /*
          * We are looking for Vars and PHVs that can be supplied by the
-         * lefthand rels.
+         * lefthand rels.  When we find one, it's okay to modify it in-place
+         * because all the routines above make a fresh copy to put into
+         * curOuterParams.
          */
         if (IsA(nlp->paramval, Var) &&
             bms_is_member(nlp->paramval->varno, leftrelids))
         {
+            Var           *var = (Var *) nlp->paramval;
+
             root->curOuterParams = foreach_delete_current(root->curOuterParams,
                                                           cell);
+            var->varnullingrels = bms_intersect(var->varnullingrels,
+                                                leftrelids);
             result = lappend(result, nlp);
         }
         else if (IsA(nlp->paramval, PlaceHolderVar) &&
@@ -531,8 +559,12 @@ identify_current_nestloop_params(PlannerInfo *root, Relids leftrelids)
                                                      (PlaceHolderVar *) nlp->paramval)->ph_eval_at,
                                leftrelids))
         {
+            PlaceHolderVar *phv = (PlaceHolderVar *) nlp->paramval;
+
             root->curOuterParams = foreach_delete_current(root->curOuterParams,
                                                           cell);
+            phv->phnullingrels = bms_intersect(phv->phnullingrels,
+                                               leftrelids);
             result = lappend(result, nlp);
         }
     }
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index cc4c122fdd..af3e6bba6d 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2607,6 +2607,23 @@ select * from int8_tbl t1
                      Filter: (q1 = t2.q1)
 (8 rows)

+explain (costs off)
+select * from int8_tbl t1
+    left join int8_tbl t2 on true
+    left join lateral
+      (select * from generate_series(t2.q1, 100)) 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
+               ->  Function Scan on generate_series
+(7 rows)
+
 explain (costs off)
 select * from onek t1
     left join onek t2 on true
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index e77e469570..ee19af838f 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -521,6 +521,13 @@ select * from int8_tbl t1
       (select * from int8_tbl t3 where t3.q1 = t2.q1 offset 0) s
       on t2.q1 = 1;

+explain (costs off)
+select * from int8_tbl t1
+    left join int8_tbl t2 on true
+    left join lateral
+      (select * from generate_series(t2.q1, 100)) s
+      on t2.q1 = 1;
+
 explain (costs off)
 select * from onek t1
     left join onek t2 on true
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 69ef483d28..b31d892121 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -580,6 +580,7 @@ create_lateral_join_info(PlannerInfo *root)
     {
         PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
         Relids        eval_at = phinfo->ph_eval_at;
+        Relids        lateral_refs;
         int            varno;

         if (phinfo->ph_lateral == NULL)
@@ -587,6 +588,15 @@ create_lateral_join_info(PlannerInfo *root)

         found_laterals = true;

+        /*
+         * Include only baserels not outer joins in the evaluation sites'
+         * lateral relids.  This avoids problems when outer join order gets
+         * rearranged, and it should still ensure that the lateral values are
+         * available when needed.
+         */
+        lateral_refs = bms_intersect(phinfo->ph_lateral, root->all_baserels);
+        Assert(!bms_is_empty(lateral_refs));
+
         if (bms_get_singleton_member(eval_at, &varno))
         {
             /* Evaluation site is a baserel */
@@ -594,10 +604,10 @@ create_lateral_join_info(PlannerInfo *root)

             brel->direct_lateral_relids =
                 bms_add_members(brel->direct_lateral_relids,
-                                phinfo->ph_lateral);
+                                lateral_refs);
             brel->lateral_relids =
                 bms_add_members(brel->lateral_relids,
-                                phinfo->ph_lateral);
+                                lateral_refs);
         }
         else
         {
@@ -610,7 +620,7 @@ create_lateral_join_info(PlannerInfo *root)
                 if (brel == NULL)
                     continue;    /* ignore outer joins in eval_at */
                 brel->lateral_relids = bms_add_members(brel->lateral_relids,
-                                                       phinfo->ph_lateral);
+                                                       lateral_refs);
             }
         }
     }
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index af3e6bba6d..372147292a 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2624,6 +2624,23 @@ select * from int8_tbl t1
                ->  Function Scan on generate_series
 (7 rows)

+explain (costs off)
+select * from int8_tbl t1
+    left join int8_tbl t2 on true
+    left join lateral
+      (select t2.q1 from int8_tbl t3) 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
+(7 rows)
+
 explain (costs off)
 select * from onek t1
     left join onek t2 on true
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index ee19af838f..84a1d69956 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -528,6 +528,13 @@ select * from int8_tbl t1
       (select * from generate_series(t2.q1, 100)) s
       on t2.q1 = 1;

+explain (costs off)
+select * from int8_tbl t1
+    left join int8_tbl t2 on true
+    left join lateral
+      (select t2.q1 from int8_tbl t3) s
+      on t2.q1 = 1;
+
 explain (costs off)
 select * from onek t1
     left join onek t2 on true