Re: BUG #18576: Using EXPLAIN (VERBOSE) in information_schema.element_types returns ERROR: failed to find plan for - Mailing list pgsql-bugs

I wrote:
> I wonder if we shouldn't change what the planner puts into the tlist
> of a dummy Result.  That is, I'm imagining reducing the tlist of
> such a Result to null Consts that would serve to show the right column
> datatypes and not much else:

>  Result
>    Output: NULL, NULL
>    One-Time Filter: false

I experimented with this, and was interested to find that there's
basically only one place we could do it in a low-level, localized
fashion, namely in set_plan_refs' handling of Result nodes; if you try
to do it earlier then set_upper_references fails when there's an upper
plan node that should copy such a column's value.  And there's
*already* an ugly hack there, which we could replace with a more
general hack.  See 0001 attached.

However, after testing it I got less excited, because it caused
quite a lot of regression test changes (which I didn't bother to
include in 0001).  Many of them seem like significant readability
lossage, for example

  GroupAggregate
-   Group Key: pagg_tab1.y
+   Group Key: (NULL::integer)
    ->  Sort
-         Sort Key: pagg_tab1.y
+         Sort Key: (NULL::integer)
          ->  Result
                One-Time Filter: false

and in a few cases I feel like the change actually means that the
test no longer proves what it intends to, because significant
details vanish into a sea of NULLs.

Another problem, which we could probably work around, is that this
breaks removal of trivial SubqueryScan nodes in some cases, because
that happens even later and the tlists no longer match.

So I feel like my idea is a dead end.  Even without the above
problems, a significant change in EXPLAIN's behavior in back
branches would be a hard sell --- the more so if it's to fix
a case that nobody even noticed for eight years.

So I'm coming around to doing something like the quick hack you
proposed.  I don't like it too much because it seems like it could
make other bugs in this area much harder to notice, but I don't have a
better idea.  We do need some work on the outdated comments though.
Also, I think we should use "f%d" not "col%d" by analogy to the
default field names for RowExprs and anonymous record types.
See 0002 attached.

            regards, tom lane

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 7aed84584c..7c417d4e14 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1013,26 +1013,41 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
                 else
                 {
                     /*
-                     * The tlist of a childless Result could contain
-                     * unresolved ROWID_VAR Vars, in case it's representing a
-                     * target relation which is completely empty because of
-                     * constraint exclusion.  Replace any such Vars by null
-                     * constants, as though they'd been resolved for a leaf
-                     * scan node that doesn't support them.  We could have
-                     * fix_scan_expr do this, but since the case is only
-                     * expected to occur here, it seems safer to special-case
-                     * it here and keep the assertions that ROWID_VARs
-                     * shouldn't be seen by fix_scan_expr.
+                     * The tlist of a dummy Result (one with constant-false
+                     * resconstantqual) could contain Vars, which are
+                     * effectively dangling references and have the potential
+                     * to cause problems in the executor or EXPLAIN.  We could
+                     * not get rid of these earlier without breaking Var
+                     * replacement in upper plan levels, but now let's reduce
+                     * the tlist to a bunch of NULL constants.  Since the
+                     * tlist should never be executed, this has no run-time
+                     * effect.  (One useful side-effect is that this makes it
+                     * safe for fix_scan_expr to assert that it won't see
+                     * ROWID_VARs; without this, unresolved ROWID Vars could
+                     * be found in the tlist of such a result.)
                      */
-                    foreach(l, splan->plan.targetlist)
+                    bool        dummy = false;
+
+                    if (list_length((List *) splan->resconstantqual) == 1)
                     {
-                        TargetEntry *tle = (TargetEntry *) lfirst(l);
-                        Var           *var = (Var *) tle->expr;
+                        Const       *cqual = (Const *) linitial((List *) splan->resconstantqual);
+
+                        dummy = IsA(cqual, Const) &&
+                            (cqual->constisnull ||
+                             !DatumGetBool(cqual->constvalue));
+                    }

-                        if (var && IsA(var, Var) && var->varno == ROWID_VAR)
-                            tle->expr = (Expr *) makeNullConst(var->vartype,
-                                                               var->vartypmod,
-                                                               var->varcollid);
+                    if (dummy)
+                    {
+                        foreach(l, splan->plan.targetlist)
+                        {
+                            TargetEntry *tle = (TargetEntry *) lfirst(l);
+                            Node       *expr = (Node *) tle->expr;
+
+                            tle->expr = (Expr *) makeNullConst(exprType(expr),
+                                                               exprTypmod(expr),
+                                                               exprCollation(expr));
+                        }
                     }

                     splan->plan.targetlist =
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 653685bffc..ab88355d9a 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -7895,17 +7895,29 @@ get_name_for_var_field(Var *var, int fieldno,
                     /*
                      * We're deparsing a Plan tree so we don't have complete
                      * RTE entries (in particular, rte->subquery is NULL). But
-                     * the only place we'd see a Var directly referencing a
-                     * SUBQUERY RTE is in a SubqueryScan plan node, and we can
-                     * look into the child plan's tlist instead.
+                     * the only place we'd normally see a Var directly
+                     * referencing a SUBQUERY RTE is in a SubqueryScan plan
+                     * node, and we can look into the child plan's tlist
+                     * instead.  An exception occurs if the subquery was
+                     * proven empty and optimized away: then we'd find such a
+                     * Var in a childless Result node, and there's nothing in
+                     * the plan tree that would let us figure out what it had
+                     * originally referenced.  In that case, fall back on
+                     * printing "fN", analogously to the default column names
+                     * for RowExprs.
                      */
                     TargetEntry *tle;
                     deparse_namespace save_dpns;
                     const char *result;

                     if (!dpns->inner_plan)
-                        elog(ERROR, "failed to find plan for subquery %s",
-                             rte->eref->aliasname);
+                    {
+                        char       *dummyname = palloc(32);
+
+                        snprintf(dummyname, 32, "f%d", fieldno);
+                        return dummyname;
+                    }
+
                     tle = get_tle_by_resno(dpns->inner_tlist, attnum);
                     if (!tle)
                         elog(ERROR, "bogus varattno for subquery var: %d",
@@ -8014,20 +8026,27 @@ get_name_for_var_field(Var *var, int fieldno,
                 {
                     /*
                      * We're deparsing a Plan tree so we don't have a CTE
-                     * list.  But the only places we'd see a Var directly
-                     * referencing a CTE RTE are in CteScan or WorkTableScan
-                     * plan nodes.  For those cases, set_deparse_plan arranged
-                     * for dpns->inner_plan to be the plan node that emits the
-                     * CTE or RecursiveUnion result, and we can look at its
-                     * tlist instead.
+                     * list.  But the only places we'd normally see a Var
+                     * directly referencing a CTE RTE are in CteScan or
+                     * WorkTableScan plan nodes.  For those cases,
+                     * set_deparse_plan arranged for dpns->inner_plan to be
+                     * the plan node that emits the CTE or RecursiveUnion
+                     * result, and we can look at its tlist instead.  As
+                     * above, this can fail if the CTE has been proven empty,
+                     * in which case fall back to "fN".
                      */
                     TargetEntry *tle;
                     deparse_namespace save_dpns;
                     const char *result;

                     if (!dpns->inner_plan)
-                        elog(ERROR, "failed to find plan for CTE %s",
-                             rte->eref->aliasname);
+                    {
+                        char       *dummyname = palloc(32);
+
+                        snprintf(dummyname, 32, "f%d", fieldno);
+                        return dummyname;
+                    }
+
                     tle = get_tle_by_resno(dpns->inner_tlist, attnum);
                     if (!tle)
                         elog(ERROR, "bogus varattno for subquery var: %d",
diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out
index b400b58f76..9168979a62 100644
--- a/src/test/regress/expected/rowtypes.out
+++ b/src/test/regress/expected/rowtypes.out
@@ -1300,6 +1300,60 @@ select pg_get_viewdef('composite_v', true);
 (1 row)

 drop view composite_v;
+--
+-- Check cases where the composite comes from a proven-dummy rel (bug #18576)
+--
+explain (verbose, costs off)
+select (ss.a).x, (ss.a).n from
+  (select information_schema._pg_expandarray(array[1,2]) AS a) ss;
+                               QUERY PLAN
+------------------------------------------------------------------------
+ Subquery Scan on ss
+   Output: (ss.a).x, (ss.a).n
+   ->  ProjectSet
+         Output: information_schema._pg_expandarray('{1,2}'::integer[])
+         ->  Result
+(5 rows)
+
+explain (verbose, costs off)
+select (ss.a).x, (ss.a).n from
+  (select information_schema._pg_expandarray(array[1,2]) AS a) ss
+where false;
+        QUERY PLAN
+--------------------------
+ Result
+   Output: (a).f1, (a).f2
+   One-Time Filter: false
+(3 rows)
+
+explain (verbose, costs off)
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select (c).f1 from cte2 as t;
+            QUERY PLAN
+-----------------------------------
+ CTE Scan on cte
+   Output: (cte.c).f1
+   CTE cte
+     ->  Result
+           Output: '(1,2)'::record
+(5 rows)
+
+explain (verbose, costs off)
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select (c).f1 from cte2 as t
+where false;
+            QUERY PLAN
+-----------------------------------
+ Result
+   Output: (cte.c).f1
+   One-Time Filter: false
+   CTE cte
+     ->  Result
+           Output: '(1,2)'::record
+(6 rows)
+
 --
 -- Tests for component access / FieldSelect
 --
diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql
index fd47dc9e0f..174b062144 100644
--- a/src/test/regress/sql/rowtypes.sql
+++ b/src/test/regress/sql/rowtypes.sql
@@ -520,6 +520,27 @@ where (select * from (select c as c1) s
 select pg_get_viewdef('composite_v', true);
 drop view composite_v;

+--
+-- Check cases where the composite comes from a proven-dummy rel (bug #18576)
+--
+explain (verbose, costs off)
+select (ss.a).x, (ss.a).n from
+  (select information_schema._pg_expandarray(array[1,2]) AS a) ss;
+explain (verbose, costs off)
+select (ss.a).x, (ss.a).n from
+  (select information_schema._pg_expandarray(array[1,2]) AS a) ss
+where false;
+
+explain (verbose, costs off)
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select (c).f1 from cte2 as t;
+explain (verbose, costs off)
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select (c).f1 from cte2 as t
+where false;
+
 --
 -- Tests for component access / FieldSelect
 --