ruleutils vs. empty targetlists

Thinking some more about bug #8648, it occurred to me that ruleutils.c
isn't exactly prepared for the case either:

regression=# create table nocols();
CREATE TABLE
regression=# create view vv1 as select exists (select * from nocols);
CREATE VIEW
regression=# \d+ vv1
                  View "public.vv1"
 Column |  Type   | Modifiers | Storage | Description
--------+---------+-----------+---------+-------------
 exists | boolean |           | plain   |
View definition:
 SELECT (EXISTS ( SELECT
           FROM nocols)) AS "exists";

which of course is illegal syntax.  I thought at first that this could be
fixed trivially by emitting "*" if get_target_list found no columns.
However, that doesn't quite work; consider

create view vv2 as select exists (select nocols.* from nocols, somecols);

If we regurgitate this as "exists (select * from nocols, somecols)", it
wouldn't mean the same thing.

But on the third hand, at least in the context of an EXISTS() subquery,
it doesn't really matter because the tlist is irrelevant, at least as long
as it only contains Vars.  So you could argue that emitting "*" is plenty
good enough even in the above example.  I'm not certain that that applies
everywhere that a tlist could appear, though.

I experimented with code that would attempt to regurgitate "nocols.*"
in the above example; see attached draft patch.  I don't like this patch
much: it's ugly, it'd be a pain to backport (because it's digging into
data structures that have changed repeatedly), and I'm not sure how much
I trust it anyway.  So I'm leaning towards just doing

+         if (colno == 0)
+             appendStringInfoString(buf, " *");

at least till such time as somebody shows a case where this isn't good
enough.

Note that I wouldn't be thinking of this as something to back-patch
at all, except that if someone did have a view that looked like this,
they'd find that pg_dump output wouldn't restore.  You could construct
scenarios where that could seem like a denial of service, particularly
if the DBA wasn't smart enough to figure out what was wrong with his
dump.  (And who wants to deal with such a thing at 4AM anyway ...)

Comments?

            regards, tom lane

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 348f620..d479276 100644
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
*************** get_target_list(List *targetList, depars
*** 4610,4615 ****
--- 4610,4652 ----
          appendStringInfoString(buf, targetbuf.data);
      }

+     /*
+      * An empty targetlist (or RETURNING list) is syntactically impossible,
+      * but we might nonetheless find no elements in the list, as a result of
+      * constructs such as "SELECT * FROM zero_column_table".  You might think
+      * we could just print a star and be done, but that fails in cases like
+      * "SELECT z.* FROM zero_column_table z, normal_table".  So, dig through
+      * the namespace lists looking for a zero-column RTE and use its alias;
+      * this is valid even if the original syntax was just "*".
+      */
+     if (colno == 0)
+     {
+         foreach(l, context->namespaces)
+         {
+             deparse_namespace *dpns = (deparse_namespace *) lfirst(l);
+             ListCell   *ln,
+                        *lc;
+
+             forboth(ln, dpns->rtable_names, lc, dpns->rtable_columns)
+             {
+                 char       *refname = (char *) lfirst(ln);
+                 deparse_columns *colinfo = (deparse_columns *) lfirst(lc);
+
+                 if (refname && colinfo->num_cols == 0)
+                 {
+                     appendStringInfo(buf, " %s.*", quote_identifier(refname));
+                     colno++;
+                     break;
+                 }
+             }
+             if (colno)
+                 break;
+         }
+         /* If we didn't find a suitable RTE, just print a star. */
+         if (colno == 0)
+             appendStringInfoString(buf, " *");
+     }
+
      /* clean up */
      pfree(targetbuf.data);
  }