Re: BUG #18953: Planner fails to build plan for complex query with LATERAL references - Mailing list pgsql-bugs

Alexander Lakhin <exclusion@gmail.com> writes:
> 17.06.2025 19:29, Tom Lane wrote:
>> So I'm inclined to leave that code as I had it.  It's notationally
>> a bit simpler and it doesn't require assuming that we can ignore
>> the path's required_outer marking at this stage.  If I'm wrong,
>> someone will eventually find a counterexample and we can fix it
>> then; the changes won't be large.

> Please look at the following (simplified version of a query generated by
> SQLsmith), which produces errors after a16ef313f2:

Hah, that didn't take long!  Your second case is indeed a
counterexample to my argument.  We end up with a PHV having
phrels (b 7 8) which needs to be evaluated here:

         {NESTPATH 
         :jpath.path.pathtype 356 
         :parent_relids (b 1 6 7)
         :required_outer (b 8)
         ...
         :jpath.outerjoinpath 
            {PATH 
            :pathtype 339 
            :parent_relids (b 7)
            :required_outer (b)

Doing things the way Richard wanted to (ie using the nestloop's
required_outer) gets past the "failed to assign all NestLoopParams"
error, but then we get the same "unrecognized node type: 22" error
as in the first example.

That error seems considerably nastier to fix.  I think it is a
pre-existing problem, though I don't currently have an explanation
why we've not seen it reported before.  What is happening is that
we pull up a PHV for "c", containing a SubLink for (SELECT true),
and only some copies of it get put through preprocess_expression,
which is responsible for converting SubLinks to SubPlans among
other essential tasks.  It seems that up to now we've always
managed to use only preprocessed copies in the finished plan.
But now a not-preprocessed copy is managing to get through to
the executor, which promptly spits up because it doesn't know
what to do with a SubLink.

We could probably hack this in a localized way by ensuring that
we push a preprocessed copy of the PHV into the left plan's tlist.
(One way to get that would be to look in the placeholder_list for the
correct phid.)  However, now that I've seen this I have very little
faith that there aren't other bugs of the same ilk, that somehow
we've not seen up to now.  I'm thinking about what we must do to
ensure that all PHVs are preprocessed once and only once, perhaps
at the moment they get put into the placeholder_list.

I don't have a patch for that yet, but attached is something that
gets rid of the "failed to assign all NestLoopParams" problem.

            regards, tom lane

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 8baf36ba4b7..81e16e87d62 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4344,6 +4344,7 @@ create_nestloop_plan(PlannerInfo *root,
     NestLoop   *join_plan;
     Plan       *outer_plan;
     Plan       *inner_plan;
+    Relids        outerrelids;
     List       *tlist = build_path_tlist(root, &best_path->jpath.path);
     List       *joinrestrictclauses = best_path->jpath.joinrestrictinfo;
     List       *joinclauses;
@@ -4374,8 +4375,8 @@ create_nestloop_plan(PlannerInfo *root,
     outer_plan = create_plan_recurse(root, best_path->jpath.outerjoinpath, 0);

     /* For a nestloop, include outer relids in curOuterRels for inner side */
-    root->curOuterRels = bms_union(root->curOuterRels,
-                                   best_path->jpath.outerjoinpath->parent->relids);
+    outerrelids = best_path->jpath.outerjoinpath->parent->relids;
+    root->curOuterRels = bms_union(root->curOuterRels, outerrelids);

     inner_plan = create_plan_recurse(root, best_path->jpath.innerjoinpath, 0);

@@ -4415,7 +4416,8 @@ create_nestloop_plan(PlannerInfo *root,
      * node, and remove them from root->curOuterParams.
      */
     nestParams = identify_current_nestloop_params(root,
-                                                  best_path->jpath.outerjoinpath);
+                                                  outerrelids,
+                                                  PATH_REQ_OUTER((Path *) best_path));

     /*
      * While nestloop parameters that are Vars had better be available from
@@ -4423,6 +4425,10 @@ create_nestloop_plan(PlannerInfo *root,
      * that are PHVs won't be.  In such cases we must add them to the
      * outer_plan's tlist, since the executor's NestLoopParam machinery
      * requires the params to be simple outer-Var references to that tlist.
+     * (This is cheating a little bit, because the outer path's required-outer
+     * relids might not be enough to allow evaluating such a PHV.  But in
+     * practice, if we could have evaluated the PHV at the nestloop node, we
+     * can do so in the outer plan too.)
      */
     outer_tlist = outer_plan->targetlist;
     outer_parallel_safe = outer_plan->parallel_safe;
diff --git a/src/backend/optimizer/util/paramassign.c b/src/backend/optimizer/util/paramassign.c
index 9836abf9479..5403ae89eab 100644
--- a/src/backend/optimizer/util/paramassign.c
+++ b/src/backend/optimizer/util/paramassign.c
@@ -599,9 +599,10 @@ 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 input path.  Remove them from the active
- * root->curOuterParams list and return them as the result list.
+ * Identify any NestLoopParams that should be supplied by a NestLoop
+ * plan node with the specified lefthand rels and required-outer 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.
@@ -626,11 +627,11 @@ process_subquery_nestloop_params(PlannerInfo *root, List *subplan_params)
  * subquery, which'd be unduly expensive.
  */
 List *
-identify_current_nestloop_params(PlannerInfo *root, Path *leftpath)
+identify_current_nestloop_params(PlannerInfo *root,
+                                 Relids leftrelids,
+                                 Relids outerrelids)
 {
     List       *result;
-    Relids        leftrelids = leftpath->parent->relids;
-    Relids        outerrelids = PATH_REQ_OUTER(leftpath);
     Relids        allleftrelids;
     ListCell   *cell;

diff --git a/src/include/optimizer/paramassign.h b/src/include/optimizer/paramassign.h
index d30d20de299..bbf7214289b 100644
--- a/src/include/optimizer/paramassign.h
+++ b/src/include/optimizer/paramassign.h
@@ -30,7 +30,8 @@ extern Param *replace_nestloop_param_placeholdervar(PlannerInfo *root,
 extern void process_subquery_nestloop_params(PlannerInfo *root,
                                              List *subplan_params);
 extern List *identify_current_nestloop_params(PlannerInfo *root,
-                                              Path *leftpath);
+                                              Relids leftrelids,
+                                              Relids outerrelids);
 extern Param *generate_new_exec_param(PlannerInfo *root, Oid paramtype,
                                       int32 paramtypmod, Oid paramcollation);
 extern int    assign_special_exec_param(PlannerInfo *root);