Re: BUG #15669: Error with unnest in PG 11 (ERROR: 0A000)

I wrote:
> I've already got a mostly-working patch.  It's causing one plan change
> in select_parallel that I've not quite figured out the reason for, or
> I should say that it's not obvious why the existing code appears to
> work...

And here 'tis.  I spent some time improving the existing comments, because
it's not very clear what some of this is doing or why it has to be done
that way.

I renamed IS_DUMMY_PATH to IS_DUMMY_APPEND, because almost anything that
might've been using it is probably wrong now; there's only one valid
use-case left in the core code, other than is_dummy_rel itself.

I noticed that there's no reason any more for set_dummy_rel_pathlist to
be global: it's not called from outside allpaths.c, and anything that
might want to call it would be better off using mark_dummy_rel anyway.
Also, there's no reason for is_dummy_plan to exist at all: that's a
leftover from days when recursing into a subquery produced a completed
Plan rather than some Paths.  So this patch includes those cleanups,
but I don't propose back-patching those changes.

I have to leave for today, but unless somebody complains, I'm intending
to push this tomorrow.

            regards, tom lane

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 0debac7..f1eed32 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -105,6 +105,7 @@ static Path *get_cheapest_parameterized_child_path(PlannerInfo *root,
                                       Relids required_outer);
 static void accumulate_append_subpath(Path *path,
                           List **subpaths, List **special_subpaths);
+static void set_dummy_rel_pathlist(RelOptInfo *rel);
 static void set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
                       Index rti, RangeTblEntry *rte);
 static void set_function_pathlist(PlannerInfo *root, RelOptInfo *rel,
@@ -1557,7 +1558,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
      * Consider an append of unordered, unparameterized partial paths.  Make
      * it parallel-aware if possible.
      */
-    if (partial_subpaths_valid)
+    if (partial_subpaths_valid && partial_subpaths != NIL)
     {
         AppendPath *appendpath;
         ListCell   *lc;
@@ -1954,11 +1955,16 @@ accumulate_append_subpath(Path *path, List **subpaths, List **special_subpaths)
  *      Build a dummy path for a relation that's been excluded by constraints
  *
  * Rather than inventing a special "dummy" path type, we represent this as an
- * AppendPath with no members (see also IS_DUMMY_PATH/IS_DUMMY_REL macros).
- *
- * This is exported because inheritance_planner() has need for it.
+ * AppendPath with no members (see also IS_DUMMY_APPEND/IS_DUMMY_REL macros).
+ * This is a convenient representation because it means that when we build
+ * an appendrel and find that all its children have been excluded, no extra
+ * action is needed to recognize the rel as dummy.
+ *
+ * (See also mark_dummy_rel, which does basically the same thing, but is
+ * typically used to change a rel into dummy state after we already made
+ * paths for it.)
  */
-void
+static void
 set_dummy_rel_pathlist(RelOptInfo *rel)
 {
     /* Set dummy size estimates --- we leave attr_widths[] as zeroes */
@@ -1969,6 +1975,12 @@ set_dummy_rel_pathlist(RelOptInfo *rel)
     rel->pathlist = NIL;
     rel->partial_pathlist = NIL;

+    /* Forget about child partitions, too, since they must all be dummy */
+    rel->nparts = 0;
+    rel->boundinfo = NULL;
+    rel->part_rels = NULL;
+
+    /* Set up the dummy path */
     add_path(rel, (Path *) create_append_path(NULL, rel, NIL, NIL, NULL,
                                               0, false, NIL, -1));

@@ -3532,12 +3544,12 @@ generate_partitionwise_join_paths(PlannerInfo *root, RelOptInfo *rel)
         /* Add partitionwise join paths for partitioned child-joins. */
         generate_partitionwise_join_paths(root, child_rel);

+        set_cheapest(child_rel);
+
         /* Dummy children will not be scanned, so ignore those. */
         if (IS_DUMMY_REL(child_rel))
             continue;

-        set_cheapest(child_rel);
-
 #ifdef OPTIMIZER_DEBUG
         debug_print_rel(root, child_rel);
 #endif
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index dfbbfda..36536bf 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -34,7 +34,6 @@ static void make_rels_by_clauseless_joins(PlannerInfo *root,
                               ListCell *other_rels);
 static bool has_join_restriction(PlannerInfo *root, RelOptInfo *rel);
 static bool has_legal_joinclause(PlannerInfo *root, RelOptInfo *rel);
-static bool is_dummy_rel(RelOptInfo *rel);
 static bool restriction_is_constant_false(List *restrictlist,
                               RelOptInfo *joinrel,
                               bool only_pushed_down);
@@ -1196,10 +1195,31 @@ have_dangerous_phv(PlannerInfo *root,
 /*
  * is_dummy_rel --- has relation been proven empty?
  */
-static bool
+bool
 is_dummy_rel(RelOptInfo *rel)
 {
-    return IS_DUMMY_REL(rel);
+    Path       *path = rel->cheapest_total_path;
+
+    /*
+     * Initially, a rel that is known dummy will have a childless Append path.
+     * But in later planning stages we might stick a ProjectSetPath on top,
+     * and it seems possible that a ProjectionPath could end up atop that
+     * (since ProjectSetPath does not accept further layers of projection).
+     * Rather than make assumptions about exactly what combinations can occur,
+     * just descend through whatever we find.
+     */
+    while (path)
+    {
+        if (IsA(path, ProjectionPath))
+            path = ((ProjectionPath *) path)->subpath;
+        else if (IsA(path, ProjectSetPath))
+            path = ((ProjectSetPath *) path)->subpath;
+        else
+            break;
+    }
+    if (path && IS_DUMMY_APPEND(path))
+        return true;
+    return false;
 }

 /*
@@ -1236,6 +1256,11 @@ mark_dummy_rel(RelOptInfo *rel)
     rel->pathlist = NIL;
     rel->partial_pathlist = NIL;

+    /* Forget about child partitions, too, since they must all be dummy */
+    rel->nparts = 0;
+    rel->boundinfo = NULL;
+    rel->part_rels = NULL;
+
     /* Set up the dummy path */
     add_path(rel, (Path *) create_append_path(NULL, rel, NIL, NIL, NULL,
                                               0, false, NIL, -1));
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 236f506..9fbe5b2 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -1072,7 +1072,7 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path)
      *
      * Note that an AppendPath with no members is also generated in certain
      * cases where there was no appending construct at all, but we know the
-     * relation is empty (see set_dummy_rel_pathlist).
+     * relation is empty (see set_dummy_rel_pathlist and mark_dummy_rel).
      */
     if (best_path->subpaths == NIL)
     {
@@ -6585,12 +6585,11 @@ is_projection_capable_path(Path *path)
         case T_Append:

             /*
-             * Append can't project, but if it's being used to represent a
-             * dummy path, claim that it can project.  This prevents us from
-             * converting a rel from dummy to non-dummy status by applying a
-             * projection to its dummy path.
+             * Append can't project, but if an AppendPath is being used to
+             * represent a dummy path, what will actually be generated is a
+             * Result which can project.
              */
-            return IS_DUMMY_PATH(path);
+            return IS_DUMMY_APPEND(path);
         case T_ProjectSet:

             /*
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index bc81535..df9c398 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1517,7 +1517,7 @@ inheritance_planner(PlannerInfo *root)
          * If this child rel was excluded by constraint exclusion, exclude it
          * from the result plan.
          */
-        if (IS_DUMMY_PATH(subpath))
+        if (IS_DUMMY_REL(sub_final_rel))
             continue;

         /*
@@ -2508,38 +2508,6 @@ remap_to_groupclause_idx(List *groupClause,
 }


-
-/*
- * Detect whether a plan node is a "dummy" plan created when a relation
- * is deemed not to need scanning due to constraint exclusion.
- *
- * Currently, such dummy plans are Result nodes with constant FALSE
- * filter quals (see set_dummy_rel_pathlist and create_append_plan).
- *
- * XXX this probably ought to be somewhere else, but not clear where.
- */
-bool
-is_dummy_plan(Plan *plan)
-{
-    if (IsA(plan, Result))
-    {
-        List       *rcqual = (List *) ((Result *) plan)->resconstantqual;
-
-        if (list_length(rcqual) == 1)
-        {
-            Const       *constqual = (Const *) linitial(rcqual);
-
-            if (constqual && IsA(constqual, Const))
-            {
-                if (!constqual->constisnull &&
-                    !DatumGetBool(constqual->constvalue))
-                    return true;
-            }
-        }
-    }
-    return false;
-}
-
 /*
  * preprocess_rowmarks - set up PlanRowMarks if needed
  */
@@ -6901,27 +6869,44 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
                                bool scanjoin_target_parallel_safe,
                                bool tlist_same_exprs)
 {
-    ListCell   *lc;
+    bool        rel_is_partitioned = (rel->part_scheme && rel->part_rels);
     PathTarget *scanjoin_target;
-    bool        is_dummy_rel = IS_DUMMY_REL(rel);
+    ListCell   *lc;

+    /* This recurses, so be paranoid. */
     check_stack_depth();

     /*
+     * If the rel is partitioned, we're going to generate all-new paths here,
+     * and there's no point in doing work on the existing paths.  So we want
+     * to drop those paths.  Care is needed, though, because we have to allow
+     * generate_gather_paths to see the old partial paths in the next stanza.
+     * Hence, zap the main pathlist here, then allow generate_gather_paths to
+     * add path(s) to the main list, and finally zap the partial pathlist.
+     */
+    if (rel_is_partitioned)
+        rel->pathlist = NIL;
+
+    /*
      * If the scan/join target is not parallel-safe, partial paths cannot
      * generate it.
      */
     if (!scanjoin_target_parallel_safe)
     {
         /*
-         * Since we can't generate the final scan/join target, this is our
-         * last opportunity to use any partial paths that exist.  We don't do
-         * this if the case where the target is parallel-safe, since we will
-         * be able to generate superior paths by doing it after the final
-         * scan/join target has been applied.
+         * Since we can't generate the final scan/join target in parallel
+         * workers, this is our last opportunity to use any partial paths that
+         * exist; so build Gather path(s) that use them and emit whatever the
+         * current reltarget is.  We don't do this in the case where the
+         * target is parallel-safe, since we will be able to generate superior
+         * paths by doing it after the final scan/join target has been
+         * applied.
          *
          * Note that this may invalidate rel->cheapest_total_path, so we must
-         * not rely on it after this point without first calling set_cheapest.
+         * not rely on that path being valid until we call set_cheapest.  In
+         * particular, this means that IS_DUMMY_REL(rel) might not work during
+         * this routine.  We don't currently need that, but if in future we
+         * do, save its value at the start of the function.
          */
         generate_gather_paths(root, rel, false);

@@ -6930,61 +6915,27 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
         rel->consider_parallel = false;
     }

-    /*
-     * Update the reltarget.  This may not be strictly necessary in all cases,
-     * but it is at least necessary when create_append_path() gets called
-     * below directly or indirectly, since that function uses the reltarget as
-     * the pathtarget for the resulting path.  It seems like a good idea to do
-     * it unconditionally.
-     */
-    rel->reltarget = llast_node(PathTarget, scanjoin_targets);
-
-    /* Special case: handle dummy relations separately. */
-    if (is_dummy_rel)
-    {
-        /*
-         * Since this is a dummy rel, it's got a single Append path with no
-         * child paths.  Replace it with a new path having the final scan/join
-         * target.  (Note that since Append is not projection-capable, it
-         * would be bad to handle this using the general purpose code below;
-         * we'd end up putting a ProjectionPath on top of the existing Append
-         * node, which would cause this relation to stop appearing to be a
-         * dummy rel.)
-         */
-        rel->pathlist = list_make1(create_append_path(root, rel, NIL, NIL,
-                                                      NULL, 0, false, NIL,
-                                                      -1));
+    /* Finish dropping old paths for a partitioned rel, per comment above */
+    if (rel_is_partitioned)
         rel->partial_pathlist = NIL;
-        set_cheapest(rel);
-        Assert(IS_DUMMY_REL(rel));
-
-        /*
-         * Forget about any child relations.  There's no point in adjusting
-         * them and no point in using them for later planning stages (in
-         * particular, partitionwise aggregate).
-         */
-        rel->nparts = 0;
-        rel->part_rels = NULL;
-        rel->boundinfo = NULL;
-
-        return;
-    }

     /* Extract SRF-free scan/join target. */
     scanjoin_target = linitial_node(PathTarget, scanjoin_targets);

     /*
-     * Adjust each input path.  If the tlist exprs are the same, we can just
-     * inject the sortgroupref information into the existing pathtarget.
-     * Otherwise, replace each path with a projection path that generates the
-     * SRF-free scan/join target.  This can't change the ordering of paths
-     * within rel->pathlist, so we just modify the list in place.
+     * Apply the SRF-free scan/join target to each existing path.
+     *
+     * If the tlist exprs are the same, we can just inject the sortgroupref
+     * information into the existing pathtargets.  Otherwise, replace each
+     * path with a projection path that generates the SRF-free scan/join
+     * target.  This can't change the ordering of paths within rel->pathlist,
+     * so we just modify the list in place.
      */
     foreach(lc, rel->pathlist)
     {
         Path       *subpath = (Path *) lfirst(lc);
-        Path       *newpath;

+        /* Shouldn't have any parameterized paths anymore */
         Assert(subpath->param_info == NULL);

         if (tlist_same_exprs)
@@ -6992,17 +6943,18 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
                 scanjoin_target->sortgrouprefs;
         else
         {
+            Path       *newpath;
+
             newpath = (Path *) create_projection_path(root, rel, subpath,
                                                       scanjoin_target);
             lfirst(lc) = newpath;
         }
     }

-    /* Same for partial paths. */
+    /* Likewise adjust the targets for any partial paths. */
     foreach(lc, rel->partial_pathlist)
     {
         Path       *subpath = (Path *) lfirst(lc);
-        Path       *newpath;

         /* Shouldn't have any parameterized paths anymore */
         Assert(subpath->param_info == NULL);
@@ -7012,39 +6964,53 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
                 scanjoin_target->sortgrouprefs;
         else
         {
-            newpath = (Path *) create_projection_path(root,
-                                                      rel,
-                                                      subpath,
+            Path       *newpath;
+
+            newpath = (Path *) create_projection_path(root, rel, subpath,
                                                       scanjoin_target);
             lfirst(lc) = newpath;
         }
     }

-    /* Now fix things up if scan/join target contains SRFs */
+    /*
+     * Now, if final scan/join target contains SRFs, insert ProjectSetPath(s)
+     * atop each existing path.  (Note that this function doesn't look at the
+     * cheapest-path fields, which is a good thing because they're bogus right
+     * now.)
+     */
     if (root->parse->hasTargetSRFs)
         adjust_paths_for_srfs(root, rel,
                               scanjoin_targets,
                               scanjoin_targets_contain_srfs);

     /*
-     * If the relation is partitioned, recursively apply the same changes to
-     * all partitions and generate new Append paths.  Since Append is not
-     * projection-capable, that might save a separate Result node, and it also
-     * is important for partitionwise aggregate.
+     * Update the rel's target to be the final (with SRFs) scan/join target.
+     * This now matches the actual output of all the paths, and we'll get
+     * confused in createplan.c if they don't agree.  We must do this now so
+     * that any append paths made in the next part will use the correct
+     * pathtarget (cf. create_append_path).
+     */
+    rel->reltarget = llast_node(PathTarget, scanjoin_targets);
+
+    /*
+     * If the relation is partitioned, recursively apply the scan/join target
+     * to all partitions and generate brand-new Append paths.  Since Append is
+     * not projection-capable, that might save a separate Result node, and it
+     * also is important for partitionwise aggregate.
      */
-    if (rel->part_scheme && rel->part_rels)
+    if (rel_is_partitioned)
     {
-        int            partition_idx;
         List       *live_children = NIL;
+        int            partition_idx;

         /* Adjust each partition. */
         for (partition_idx = 0; partition_idx < rel->nparts; partition_idx++)
         {
             RelOptInfo *child_rel = rel->part_rels[partition_idx];
-            ListCell   *lc;
             AppendRelInfo **appinfos;
             int            nappinfos;
             List       *child_scanjoin_targets = NIL;
+            ListCell   *lc;

             /* Translate scan/join targets for this child. */
             appinfos = find_appinfos_by_relids(root, child_rel->relids,
@@ -7076,8 +7042,7 @@ apply_scanjoin_target_to_paths(PlannerInfo *root,
         }

         /* Build new paths for this relation by appending child paths. */
-        if (live_children != NIL)
-            add_paths_to_append_rel(root, rel, live_children);
+        add_paths_to_append_rel(root, rel, live_children);
     }

     /*
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index a008ae0..7a8c3f7 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1361,13 +1361,16 @@ typedef struct AppendPath
     int            first_partial_path;
 } AppendPath;

-#define IS_DUMMY_PATH(p) \
+#define IS_DUMMY_APPEND(p) \
     (IsA((p), AppendPath) && ((AppendPath *) (p))->subpaths == NIL)

-/* A relation that's been proven empty will have one path that is dummy */
-#define IS_DUMMY_REL(r) \
-    ((r)->cheapest_total_path != NULL && \
-     IS_DUMMY_PATH((r)->cheapest_total_path))
+/*
+ * A relation that's been proven empty will have one path that is dummy
+ * (but might have projection paths on top).  For historical reasons,
+ * this is provided as a macro that wraps is_dummy_rel().
+ */
+#define IS_DUMMY_REL(r) is_dummy_rel(r)
+extern bool is_dummy_rel(RelOptInfo *rel);

 /*
  * MergeAppendPath represents a MergeAppend plan, ie, the merging of sorted
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 040335a..36d12bc 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -49,7 +49,6 @@ extern PGDLLIMPORT join_search_hook_type join_search_hook;


 extern RelOptInfo *make_one_rel(PlannerInfo *root, List *joinlist);
-extern void set_dummy_rel_pathlist(RelOptInfo *rel);
 extern RelOptInfo *standard_join_search(PlannerInfo *root, int levels_needed,
                      List *initial_rels);

diff --git a/src/include/optimizer/planner.h b/src/include/optimizer/planner.h
index cb41e40..830557e 100644
--- a/src/include/optimizer/planner.h
+++ b/src/include/optimizer/planner.h
@@ -44,8 +44,6 @@ extern PlannerInfo *subquery_planner(PlannerGlobal *glob, Query *parse,
                  PlannerInfo *parent_root,
                  bool hasRecursion, double tuple_fraction);

-extern bool is_dummy_plan(Plan *plan);
-
 extern RowMarkType select_rowmark_type(RangeTblEntry *rte,
                     LockClauseStrength strength);

diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out
index 25be6b9..d47b5f6 100644
--- a/src/test/regress/expected/tsrf.out
+++ b/src/test/regress/expected/tsrf.out
@@ -83,6 +83,39 @@ SELECT generate_series(1, generate_series(1, 3)), generate_series(2, 4);

 CREATE TABLE few(id int, dataa text, datab text);
 INSERT INTO few VALUES(1, 'a', 'foo'),(2, 'a', 'bar'),(3, 'b', 'bar');
+-- SRF with a provably-dummy relation
+explain (verbose, costs off)
+SELECT unnest(ARRAY[1, 2]) FROM few WHERE false;
+              QUERY PLAN
+--------------------------------------
+ ProjectSet
+   Output: unnest('{1,2}'::integer[])
+   ->  Result
+         One-Time Filter: false
+(4 rows)
+
+SELECT unnest(ARRAY[1, 2]) FROM few WHERE false;
+ unnest
+--------
+(0 rows)
+
+-- SRF shouldn't prevent upper query from recognizing lower as dummy
+explain (verbose, costs off)
+SELECT * FROM few f1,
+  (SELECT unnest(ARRAY[1,2]) FROM few f2 WHERE false OFFSET 0) ss;
+                   QUERY PLAN
+------------------------------------------------
+ Result
+   Output: f1.id, f1.dataa, f1.datab, ss.unnest
+   One-Time Filter: false
+(3 rows)
+
+SELECT * FROM few f1,
+  (SELECT unnest(ARRAY[1,2]) FROM few f2 WHERE false OFFSET 0) ss;
+ id | dataa | datab | unnest
+----+-------+-------+--------
+(0 rows)
+
 -- SRF output order of sorting is maintained, if SRF is not referenced
 SELECT few.id, generate_series(1,3) g FROM few ORDER BY id DESC;
  id | g
diff --git a/src/test/regress/sql/tsrf.sql b/src/test/regress/sql/tsrf.sql
index 0a1e8e5..7c22529 100644
--- a/src/test/regress/sql/tsrf.sql
+++ b/src/test/regress/sql/tsrf.sql
@@ -28,6 +28,18 @@ SELECT generate_series(1, generate_series(1, 3)), generate_series(2, 4);
 CREATE TABLE few(id int, dataa text, datab text);
 INSERT INTO few VALUES(1, 'a', 'foo'),(2, 'a', 'bar'),(3, 'b', 'bar');

+-- SRF with a provably-dummy relation
+explain (verbose, costs off)
+SELECT unnest(ARRAY[1, 2]) FROM few WHERE false;
+SELECT unnest(ARRAY[1, 2]) FROM few WHERE false;
+
+-- SRF shouldn't prevent upper query from recognizing lower as dummy
+explain (verbose, costs off)
+SELECT * FROM few f1,
+  (SELECT unnest(ARRAY[1,2]) FROM few f2 WHERE false OFFSET 0) ss;
+SELECT * FROM few f1,
+  (SELECT unnest(ARRAY[1,2]) FROM few f2 WHERE false OFFSET 0) ss;
+
 -- SRF output order of sorting is maintained, if SRF is not referenced
 SELECT few.id, generate_series(1,3) g FROM few ORDER BY id DESC;