Re: [HACKERS] Assignment of valid collation for SET operations on queries with UNKNOWN types.

I wrote:
> I spent some time fooling with this today and came up with the attached
> patch.  I think this is basically the direction we should go in, but
> there are various loose ends:

Here's an updated patch that's rebased against today's HEAD and addresses
most of the loose ends:

> 1. I adjusted a couple of existing regression tests whose results are
> affected, but didn't do anything towards adding new tests showing the
> desirable results of this change (as per the examples in this thread).

Added some regression test cases.  These are mostly designed to prove
that coercion to text occurs where expected, but I did include a couple
of queries that would have failed outright before.

> 2. I didn't do anything about docs, either, though maybe no change
> is needed.  One user-visible change from this is that queries should
> never return any "unknown"-type columns to clients anymore.  But I
> think that is not documented now, so maybe there's nothing to change.

The only thing I could find in the SGML docs that directly addresses
unknown-type columns was a remark in the CREATE VIEW man page, which
I've updated.  I also added a section to typeconv.sgml to document
the behavior.

> 3. We need to look at whether pg_upgrade is affected.  I think we
> might be able to let it just ignore the issue for views, since they'd
> get properly updated during the dump and reload anyway.  But if someone
> had an actual table (or matview) with an "unknown" column, that would
> be a pg_upgrade hazard, because "unknown" doesn't store like "text".

I tested and found that simple views with unknown columns seem to update
sanely if we just let pg_upgrade dump and restore them.  So I suggest we
allow that to happen.  There might be cases where dependent views behave
unexpectedly after such a conversion, but I think that would be rare
enough that it's not worth forcing users to fix these views by hand before
updating.  However, tables with unknown columns would fail the upgrade
(since we'd reject the CREATE TABLE command) while matviews would be
accepted but then the DDL wouldn't match the physical data storage.
So I added code to pg_upgrade to check for those cases and refuse to
proceed.  This is almost a straight copy-and-paste of the existing
pg_upgrade code for checking for "line" columns.


I think this is committable now; the other loose ends can be dealt
with in follow-on patches.  Does anyone want to review it?

            regards, tom lane

diff --git a/doc/src/sgml/ref/create_view.sgml b/doc/src/sgml/ref/create_view.sgml
index 8641e19..a83d956 100644
*** a/doc/src/sgml/ref/create_view.sgml
--- b/doc/src/sgml/ref/create_view.sgml
*************** CREATE VIEW [ <replaceable>schema</> . ]
*** 251,259 ****
  <programlisting>
  CREATE VIEW vista AS SELECT 'Hello World';
  </programlisting>
!     is bad form in two ways: the column name defaults to <literal>?column?</>,
!     and the column data type defaults to <type>unknown</>.  If you want a
!     string literal in a view's result, use something like:
  <programlisting>
  CREATE VIEW vista AS SELECT text 'Hello World' AS hello;
  </programlisting>
--- 251,260 ----
  <programlisting>
  CREATE VIEW vista AS SELECT 'Hello World';
  </programlisting>
!     is bad form because the column name defaults to <literal>?column?</>;
!     also, the column data type defaults to <type>text</>, which might not
!     be what you wanted.  Better style for a string literal in a view's
!     result is something like:
  <programlisting>
  CREATE VIEW vista AS SELECT text 'Hello World' AS hello;
  </programlisting>
diff --git a/doc/src/sgml/typeconv.sgml b/doc/src/sgml/typeconv.sgml
index c031c01..63d41f0 100644
*** a/doc/src/sgml/typeconv.sgml
--- b/doc/src/sgml/typeconv.sgml
*************** domain's base type for all subsequent st
*** 984,990 ****
  <para>
  If all inputs are of type <type>unknown</type>, resolve as type
  <type>text</type> (the preferred type of the string category).
! Otherwise, <type>unknown</type> inputs are ignored.
  </para>
  </step>

--- 984,991 ----
  <para>
  If all inputs are of type <type>unknown</type>, resolve as type
  <type>text</type> (the preferred type of the string category).
! Otherwise, <type>unknown</type> inputs are ignored for the purposes
! of the remaining rules.
  </para>
  </step>

*************** but <type>integer</> can be implicitly c
*** 1076,1081 ****
--- 1077,1129 ----
  result type is resolved as <type>real</>.
  </para>
  </example>
+ </sect1>
+
+ <sect1 id="typeconv-select">
+ <title><literal>SELECT</literal> Output Columns</title>
+
+ <indexterm zone="typeconv-select">
+  <primary>SELECT</primary>
+  <secondary>determination of result type</secondary>
+ </indexterm>
+
+ <para>
+ The rules given in the preceding sections will result in assignment
+ of non-<type>unknown</> data types to all expressions in a SQL query,
+ except for unspecified-type literals that appear as simple output
+ columns of a <command>SELECT</> command.  For example, in
+
+ <screen>
+ SELECT 'Hello World';
+ </screen>
+
+ there is nothing to identify what type the string literal should be
+ taken as.  In this situation <productname>PostgreSQL</> will fall back
+ to resolving the literal's type as <type>text</>.
+ </para>
+
+ <para>
+ When the <command>SELECT</> is one arm of a <literal>UNION</>
+ (or <literal>INTERSECT</> or <literal>EXCEPT</>) construct, or when it
+ appears within <command>INSERT ... SELECT</>, this rule is not applied
+ since rules given in preceding sections take precedence.  The type of an
+ unspecified-type literal can be taken from the other <literal>UNION</> arm
+ in the first case, or from the destination column in the second case.
+ </para>
+
+ <para>
+ <literal>RETURNING</> lists are treated the same as <command>SELECT</>
+ output lists for this purpose.
+ </para>
+
+ <note>
+  <para>
+   Prior to <productname>PostgreSQL</> 10, this rule did not exist, and
+   unspecified-type literals in a <command>SELECT</> output list were
+   left as type <type>unknown</>.  That had assorted bad consequences,
+   so it's been changed.
+  </para>
+ </note>

  </sect1>
  </chapter>
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index bfc54a8..af6ba47 100644
*** a/src/backend/catalog/heap.c
--- b/src/backend/catalog/heap.c
*************** CheckAttributeType(const char *attname,
*** 490,507 ****
      char        att_typtype = get_typtype(atttypid);
      Oid            att_typelem;

!     if (atttypid == UNKNOWNOID)
!     {
!         /*
!          * Warn user, but don't fail, if column to be created has UNKNOWN type
!          * (usually as a result of a 'retrieve into' - jolly)
!          */
!         ereport(WARNING,
!                 (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
!                  errmsg("column \"%s\" has type %s", attname, "unknown"),
!                  errdetail("Proceeding with relation creation anyway.")));
!     }
!     else if (att_typtype == TYPTYPE_PSEUDO)
      {
          /*
           * Refuse any attempt to create a pseudo-type column, except for a
--- 490,497 ----
      char        att_typtype = get_typtype(atttypid);
      Oid            att_typelem;

!     if (atttypid == UNKNOWNOID ||
!         att_typtype == TYPTYPE_PSEUDO)
      {
          /*
           * Refuse any attempt to create a pseudo-type column, except for a
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index a02a77a..f954dc1 100644
*** a/src/backend/parser/analyze.c
--- b/src/backend/parser/analyze.c
*************** parse_analyze_varparams(RawStmt *parseTr
*** 156,168 ****
  Query *
  parse_sub_analyze(Node *parseTree, ParseState *parentParseState,
                    CommonTableExpr *parentCTE,
!                   bool locked_from_parent)
  {
      ParseState *pstate = make_parsestate(parentParseState);
      Query       *query;

      pstate->p_parent_cte = parentCTE;
      pstate->p_locked_from_parent = locked_from_parent;

      query = transformStmt(pstate, parseTree);

--- 156,170 ----
  Query *
  parse_sub_analyze(Node *parseTree, ParseState *parentParseState,
                    CommonTableExpr *parentCTE,
!                   bool locked_from_parent,
!                   bool resolve_unknowns)
  {
      ParseState *pstate = make_parsestate(parentParseState);
      Query       *query;

      pstate->p_parent_cte = parentCTE;
      pstate->p_locked_from_parent = locked_from_parent;
+     pstate->p_resolve_unknowns = resolve_unknowns;

      query = transformStmt(pstate, parseTree);

*************** transformInsertStmt(ParseState *pstate,
*** 570,579 ****
--- 572,588 ----
           * otherwise the behavior of SELECT within INSERT might be different
           * from a stand-alone SELECT. (Indeed, Postgres up through 6.5 had
           * bugs of just that nature...)
+          *
+          * The sole exception is that we prevent resolving unknown-type
+          * outputs as TEXT.  This does not change the semantics since if the
+          * column type matters semantically, it would have been resolved to
+          * something else anyway.  Doing this lets us resolve such outputs as
+          * the target column's type, which we handle below.
           */
          sub_pstate->p_rtable = sub_rtable;
          sub_pstate->p_joinexprs = NIL;    /* sub_rtable has no joins */
          sub_pstate->p_namespace = sub_namespace;
+         sub_pstate->p_resolve_unknowns = false;

          selectQuery = transformStmt(sub_pstate, stmt->selectStmt);

*************** transformSelectStmt(ParseState *pstate,
*** 1269,1274 ****
--- 1278,1287 ----
                                                     pstate->p_windowdefs,
                                                     &qry->targetList);

+     /* resolve any still-unresolved output columns as being type text */
+     if (pstate->p_resolve_unknowns)
+         resolveTargetListUnknowns(pstate, qry->targetList);
+
      qry->rtable = pstate->p_rtable;
      qry->jointree = makeFromExpr(pstate->p_joinlist, qual);

*************** transformSetOperationTree(ParseState *ps
*** 1843,1853 ****
          /*
           * Transform SelectStmt into a Query.
           *
           * Note: previously transformed sub-queries don't affect the parsing
           * of this sub-query, because they are not in the toplevel pstate's
           * namespace list.
           */
!         selectQuery = parse_sub_analyze((Node *) stmt, pstate, NULL, false);

          /*
           * Check for bogus references to Vars on the current query level (but
--- 1856,1874 ----
          /*
           * Transform SelectStmt into a Query.
           *
+          * This works the same as SELECT transformation normally would, except
+          * that we prevent resolving unknown-type outputs as TEXT.  This does
+          * not change the subquery's semantics since if the column type
+          * matters semantically, it would have been resolved to something else
+          * anyway.  Doing this lets us resolve such outputs using
+          * select_common_type(), below.
+          *
           * Note: previously transformed sub-queries don't affect the parsing
           * of this sub-query, because they are not in the toplevel pstate's
           * namespace list.
           */
!         selectQuery = parse_sub_analyze((Node *) stmt, pstate,
!                                         NULL, false, false);

          /*
           * Check for bogus references to Vars on the current query level (but
*************** transformReturningList(ParseState *pstat
*** 2350,2355 ****
--- 2371,2380 ----
      /* mark column origins */
      markTargetListOrigins(pstate, rlist);

+     /* resolve any still-unresolved output columns as being type text */
+     if (pstate->p_resolve_unknowns)
+         resolveTargetListUnknowns(pstate, rlist);
+
      /* restore state */
      pstate->p_next_resno = save_next_resno;

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 624ab41..4769e78 100644
*** a/src/backend/parser/parse_clause.c
--- b/src/backend/parser/parse_clause.c
*************** transformRangeSubselect(ParseState *psta
*** 471,477 ****
       * Analyze and transform the subquery.
       */
      query = parse_sub_analyze(r->subquery, pstate, NULL,
!                               isLockedRefname(pstate, r->alias->aliasname));

      /* Restore state */
      pstate->p_lateral_active = false;
--- 471,478 ----
       * Analyze and transform the subquery.
       */
      query = parse_sub_analyze(r->subquery, pstate, NULL,
!                               isLockedRefname(pstate, r->alias->aliasname),
!                               true);

      /* Restore state */
      pstate->p_lateral_active = false;
diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c
index fc8c15b..dfbcaa2 100644
*** a/src/backend/parser/parse_cte.c
--- b/src/backend/parser/parse_cte.c
*************** analyzeCTE(ParseState *pstate, CommonTab
*** 241,247 ****
      /* Analysis not done already */
      Assert(!IsA(cte->ctequery, Query));

!     query = parse_sub_analyze(cte->ctequery, pstate, cte, false);
      cte->ctequery = (Node *) query;

      /*
--- 241,247 ----
      /* Analysis not done already */
      Assert(!IsA(cte->ctequery, Query));

!     query = parse_sub_analyze(cte->ctequery, pstate, cte, false, true);
      cte->ctequery = (Node *) query;

      /*
*************** analyzeCTETargetList(ParseState *pstate,
*** 393,403 ****

          /*
           * If the CTE is recursive, force the exposed column type of any
!          * "unknown" column to "text".  This corresponds to the fact that
!          * SELECT 'foo' UNION SELECT 'bar' will ultimately produce text. We
!          * might see "unknown" as a result of an untyped literal in the
!          * non-recursive term's select list, and if we don't convert to text
!          * then we'll have a mismatch against the UNION result.
           *
           * The column might contain 'foo' COLLATE "bar", so don't override
           * collation if it's already set.
--- 393,402 ----

          /*
           * If the CTE is recursive, force the exposed column type of any
!          * "unknown" column to "text".  We must deal with this here because
!          * we're called on the non-recursive term before there's been any
!          * attempt to force unknown output columns to some other type.  We
!          * have to resolve unknowns before looking at the recursive term.
           *
           * The column might contain 'foo' COLLATE "bar", so don't override
           * collation if it's already set.
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index add3be6..c43ef19 100644
*** a/src/backend/parser/parse_expr.c
--- b/src/backend/parser/parse_expr.c
*************** transformSubLink(ParseState *pstate, Sub
*** 1846,1852 ****
      /*
       * OK, let's transform the sub-SELECT.
       */
!     qtree = parse_sub_analyze(sublink->subselect, pstate, NULL, false);

      /*
       * Check that we got a SELECT.  Anything else should be impossible given
--- 1846,1852 ----
      /*
       * OK, let's transform the sub-SELECT.
       */
!     qtree = parse_sub_analyze(sublink->subselect, pstate, NULL, false, true);

      /*
       * Check that we got a SELECT.  Anything else should be impossible given
diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index 73e7d65..2a5f147 100644
*** a/src/backend/parser/parse_node.c
--- b/src/backend/parser/parse_node.c
*************** make_parsestate(ParseState *parentParseS
*** 51,56 ****
--- 51,57 ----

      /* Fill in fields that don't start at null/false/zero */
      pstate->p_next_resno = 1;
+     pstate->p_resolve_unknowns = true;

      if (parentParseState)
      {
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 081a8dd..2576e31 100644
*** a/src/backend/parser/parse_target.c
--- b/src/backend/parser/parse_target.c
*************** transformExpressionList(ParseState *psta
*** 289,300 ****


  /*
   * markTargetListOrigins()
   *        Mark targetlist columns that are simple Vars with the source
   *        table's OID and column number.
   *
!  * Currently, this is done only for SELECT targetlists, since we only
!  * need the info if we are going to send it to the frontend.
   */
  void
  markTargetListOrigins(ParseState *pstate, List *targetlist)
--- 289,329 ----


  /*
+  * resolveTargetListUnknowns()
+  *        Convert any unknown-type targetlist entries to type TEXT.
+  *
+  * We do this after we've exhausted all other ways of identifying the output
+  * column types of a query.
+  */
+ void
+ resolveTargetListUnknowns(ParseState *pstate, List *targetlist)
+ {
+     ListCell   *l;
+
+     foreach(l, targetlist)
+     {
+         TargetEntry *tle = (TargetEntry *) lfirst(l);
+         Oid            restype = exprType((Node *) tle->expr);
+
+         if (restype == UNKNOWNOID)
+         {
+             tle->expr = (Expr *) coerce_type(pstate, (Node *) tle->expr,
+                                              restype, TEXTOID, -1,
+                                              COERCION_IMPLICIT,
+                                              COERCE_IMPLICIT_CAST,
+                                              -1);
+         }
+     }
+ }
+
+
+ /*
   * markTargetListOrigins()
   *        Mark targetlist columns that are simple Vars with the source
   *        table's OID and column number.
   *
!  * Currently, this is done only for SELECT targetlists and RETURNING lists,
!  * since we only need the info if we are going to send it to the frontend.
   */
  void
  markTargetListOrigins(ParseState *pstate, List *targetlist)
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index b6efad4..ff1d7a2 100644
*** a/src/bin/pg_upgrade/check.c
--- b/src/bin/pg_upgrade/check.c
*************** check_and_dump_old_cluster(bool live_che
*** 99,104 ****
--- 99,108 ----
      check_for_reg_data_type_usage(&old_cluster);
      check_for_isn_and_int8_passing_mismatch(&old_cluster);

+     /* Pre-PG 10 allowed tables with 'unknown' type columns */
+     if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
+         old_9_6_check_for_unknown_data_type_usage(&old_cluster);
+
      /* 9.5 and below should not have roles starting with pg_ */
      if (GET_MAJOR_VERSION(old_cluster.major_version) <= 905)
          check_for_pg_role_prefix(&old_cluster);
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 42e7aeb..968ab8a 100644
*** a/src/bin/pg_upgrade/pg_upgrade.h
--- b/src/bin/pg_upgrade/pg_upgrade.h
*************** void        pg_putenv(const char *var, const c
*** 442,447 ****
--- 442,448 ----
  void new_9_0_populate_pg_largeobject_metadata(ClusterInfo *cluster,
                                           bool check_mode);
  void        old_9_3_check_for_line_data_type_usage(ClusterInfo *cluster);
+ void        old_9_6_check_for_unknown_data_type_usage(ClusterInfo *cluster);

  /* parallel.c */
  void parallel_exec_prog(const char *log_file, const char *opt_log_file,
diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
index fb56fea..aa462da 100644
*** a/src/bin/pg_upgrade/version.c
--- b/src/bin/pg_upgrade/version.c
*************** old_9_3_check_for_line_data_type_usage(C
*** 185,187 ****
--- 185,284 ----
      else
          check_ok();
  }
+
+
+ /*
+  * old_9_6_check_for_unknown_data_type_usage()
+  *    9.6 -> 10
+  *    It's no longer allowed to create tables or views with "unknown"-type
+  *    columns.  We do not complain about views with such columns, because
+  *    they should get silently converted to "text" columns during the DDL
+  *    dump and reload; it seems unlikely to be worth making users do that
+  *    by hand.  However, if there's a table with such a column, the DDL
+  *    reload will fail, so we should pre-detect that rather than failing
+  *    mid-upgrade.  Worse, if there's a matview with such a column, the
+  *    DDL reload will silently change it to "text" which won't match the
+  *    on-disk storage (which is like "cstring").  So we *must* reject that.
+  *    Also check composite types, in case they are used for table columns.
+  *    We needn't check indexes, because "unknown" has no opclasses.
+  */
+ void
+ old_9_6_check_for_unknown_data_type_usage(ClusterInfo *cluster)
+ {
+     int            dbnum;
+     FILE       *script = NULL;
+     bool        found = false;
+     char        output_path[MAXPGPATH];
+
+     prep_status("Checking for invalid \"unknown\" user columns");
+
+     snprintf(output_path, sizeof(output_path), "tables_using_unknown.txt");
+
+     for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
+     {
+         PGresult   *res;
+         bool        db_used = false;
+         int            ntups;
+         int            rowno;
+         int            i_nspname,
+                     i_relname,
+                     i_attname;
+         DbInfo       *active_db = &cluster->dbarr.dbs[dbnum];
+         PGconn       *conn = connectToServer(cluster, active_db->db_name);
+
+         res = executeQueryOrDie(conn,
+                                 "SELECT n.nspname, c.relname, a.attname "
+                                 "FROM    pg_catalog.pg_class c, "
+                                 "        pg_catalog.pg_namespace n, "
+                                 "        pg_catalog.pg_attribute a "
+                                 "WHERE    c.oid = a.attrelid AND "
+                                 "        NOT a.attisdropped AND "
+                                 "        a.atttypid = 'pg_catalog.unknown'::pg_catalog.regtype AND "
+                            "        c.relkind IN ('r', 'c', 'm') AND "
+                                 "        c.relnamespace = n.oid AND "
+         /* exclude possible orphaned temp tables */
+                                 "        n.nspname !~ '^pg_temp_' AND "
+                          "        n.nspname !~ '^pg_toast_temp_' AND "
+                                 "        n.nspname NOT IN ('pg_catalog', 'information_schema')");
+
+         ntups = PQntuples(res);
+         i_nspname = PQfnumber(res, "nspname");
+         i_relname = PQfnumber(res, "relname");
+         i_attname = PQfnumber(res, "attname");
+         for (rowno = 0; rowno < ntups; rowno++)
+         {
+             found = true;
+             if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+                 pg_fatal("could not open file \"%s\": %s\n", output_path,
+                          strerror(errno));
+             if (!db_used)
+             {
+                 fprintf(script, "Database: %s\n", active_db->db_name);
+                 db_used = true;
+             }
+             fprintf(script, "  %s.%s.%s\n",
+                     PQgetvalue(res, rowno, i_nspname),
+                     PQgetvalue(res, rowno, i_relname),
+                     PQgetvalue(res, rowno, i_attname));
+         }
+
+         PQclear(res);
+
+         PQfinish(conn);
+     }
+
+     if (script)
+         fclose(script);
+
+     if (found)
+     {
+         pg_log(PG_REPORT, "fatal\n");
+         pg_fatal("Your installation contains the \"unknown\" data type in user tables.  This\n"
+                  "data type is no longer allowed in tables, so this cluster cannot currently\n"
+                  "be upgraded.  You can remove the problem tables and restart the upgrade.\n"
+                  "A list of the problem columns is in the file:\n"
+                  "    %s\n\n", output_path);
+     }
+     else
+         check_ok();
+ }
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index a7e5c55..1725940 100644
*** a/src/include/parser/analyze.h
--- b/src/include/parser/analyze.h
*************** extern Query *parse_analyze_varparams(Ra
*** 29,35 ****

  extern Query *parse_sub_analyze(Node *parseTree, ParseState *parentParseState,
                    CommonTableExpr *parentCTE,
!                   bool locked_from_parent);

  extern Query *transformTopLevelStmt(ParseState *pstate, RawStmt *parseTree);
  extern Query *transformStmt(ParseState *pstate, Node *parseTree);
--- 29,36 ----

  extern Query *parse_sub_analyze(Node *parseTree, ParseState *parentParseState,
                    CommonTableExpr *parentCTE,
!                   bool locked_from_parent,
!                   bool resolve_unknowns);

  extern Query *transformTopLevelStmt(ParseState *pstate, RawStmt *parseTree);
  extern Query *transformStmt(ParseState *pstate, Node *parseTree);
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index bc3eea9..3a25d95 100644
*** a/src/include/parser/parse_node.h
--- b/src/include/parser/parse_node.h
*************** typedef Node *(*CoerceParamHook) (ParseS
*** 150,155 ****
--- 150,158 ----
   * p_locked_from_parent: true if parent query level applies FOR UPDATE/SHARE
   * to this subquery as a whole.
   *
+  * p_resolve_unknowns: resolve unknown-type SELECT output columns as type TEXT
+  * (this is true by default).
+  *
   * p_hasAggs, p_hasWindowFuncs, etc: true if we've found any of the indicated
   * constructs in the query.
   *
*************** struct ParseState
*** 182,187 ****
--- 185,192 ----
      List       *p_locking_clause;        /* raw FOR UPDATE/FOR SHARE info */
      bool        p_locked_from_parent;    /* parent has marked this subquery
                                           * with FOR UPDATE/FOR SHARE */
+     bool        p_resolve_unknowns;        /* resolve unknown-type SELECT outputs
+                                          * as type text */

      /* Flags telling about things found in the query: */
      bool        p_hasAggs;
diff --git a/src/include/parser/parse_target.h b/src/include/parser/parse_target.h
index e035aac..d06a235 100644
*** a/src/include/parser/parse_target.h
--- b/src/include/parser/parse_target.h
*************** extern List *transformTargetList(ParseSt
*** 21,26 ****
--- 21,27 ----
                      ParseExprKind exprKind);
  extern List *transformExpressionList(ParseState *pstate, List *exprlist,
                          ParseExprKind exprKind, bool allowDefault);
+ extern void resolveTargetListUnknowns(ParseState *pstate, List *targetlist);
  extern void markTargetListOrigins(ParseState *pstate, List *targetlist);
  extern TargetEntry *transformTargetEntry(ParseState *pstate,
                       Node *node, Node *expr, ParseExprKind exprKind,
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 6caa9c2..36266f0 100644
*** a/src/test/regress/expected/create_table.out
--- b/src/test/regress/expected/create_table.out
*************** CREATE TABLE array_index_op_test (
*** 199,204 ****
--- 199,212 ----
  CREATE TABLE testjsonb (
         j jsonb
  );
+ CREATE TABLE unknowntab (
+     u unknown    -- fail
+ );
+ ERROR:  column "u" has pseudo-type unknown
+ CREATE TYPE unknown_comptype AS (
+     u unknown    -- fail
+ );
+ ERROR:  column "u" has pseudo-type unknown
  CREATE TABLE IF NOT EXISTS test_tsvector(
      t text,
      a tsvector
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 096bfc3..ce0c8ce 100644
*** a/src/test/regress/expected/create_view.out
--- b/src/test/regress/expected/create_view.out
*************** SELECT relname, relkind, reloptions FROM
*** 288,293 ****
--- 288,319 ----
   mysecview4 | v       | {security_barrier=false}
  (4 rows)

+ -- Check that unknown literals are converted to "text" in CREATE VIEW,
+ -- so that we don't end up with unknown-type columns.
+ CREATE VIEW unspecified_types AS
+   SELECT 42 as i, 42.5 as num, 'foo' as u, 'foo'::unknown as u2, null as n;
+ \d+ unspecified_types
+                    View "testviewschm2.unspecified_types"
+  Column |  Type   | Collation | Nullable | Default | Storage  | Description
+ --------+---------+-----------+----------+---------+----------+-------------
+  i      | integer |           |          |         | plain    |
+  num    | numeric |           |          |         | main     |
+  u      | text    |           |          |         | extended |
+  u2     | text    |           |          |         | extended |
+  n      | text    |           |          |         | extended |
+ View definition:
+  SELECT 42 AS i,
+     42.5 AS num,
+     'foo'::text AS u,
+     'foo'::text AS u2,
+     NULL::text AS n;
+
+ SELECT * FROM unspecified_types;
+  i  | num  |  u  | u2  | n
+ ----+------+-----+-----+---
+  42 | 42.5 | foo | foo |
+ (1 row)
+
  -- This test checks that proper typmods are assigned in a multi-row VALUES
  CREATE VIEW tt1 AS
    SELECT * FROM (
diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out
index 7a2eaa0..4ae4460 100644
*** a/src/test/regress/expected/matview.out
--- b/src/test/regress/expected/matview.out
*************** DETAIL:  drop cascades to materialized v
*** 508,513 ****
--- 508,540 ----
  drop cascades to materialized view mvtest_mv_v_2
  drop cascades to materialized view mvtest_mv_v_3
  drop cascades to materialized view mvtest_mv_v_4
+ -- Check that unknown literals are converted to "text" in CREATE MATVIEW,
+ -- so that we don't end up with unknown-type columns.
+ CREATE MATERIALIZED VIEW mv_unspecified_types AS
+   SELECT 42 as i, 42.5 as num, 'foo' as u, 'foo'::unknown as u2, null as n;
+ \d+ mv_unspecified_types
+                       Materialized view "public.mv_unspecified_types"
+  Column |  Type   | Collation | Nullable | Default | Storage  | Stats target | Description
+ --------+---------+-----------+----------+---------+----------+--------------+-------------
+  i      | integer |           |          |         | plain    |              |
+  num    | numeric |           |          |         | main     |              |
+  u      | text    |           |          |         | extended |              |
+  u2     | text    |           |          |         | extended |              |
+  n      | text    |           |          |         | extended |              |
+ View definition:
+  SELECT 42 AS i,
+     42.5 AS num,
+     'foo'::text AS u,
+     'foo'::text AS u2,
+     NULL::text AS n;
+
+ SELECT * FROM mv_unspecified_types;
+  i  | num  |  u  | u2  | n
+ ----+------+-----+-----+---
+  42 | 42.5 | foo | foo |
+ (1 row)
+
+ DROP MATERIALIZED VIEW mv_unspecified_types;
  -- make sure that create WITH NO DATA does not plan the query (bug #13907)
  create materialized view mvtest_error as select 1/0 as x;  -- fail
  ERROR:  division by zero
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index abd3217..47afdc3 100644
*** a/src/test/regress/expected/subselect.out
--- b/src/test/regress/expected/subselect.out
*************** SELECT '' AS five, f1 AS "Correlated Fie
*** 196,201 ****
--- 196,232 ----
        |                3
  (5 rows)

+ -- Unspecified-type literals in output columns should resolve as text
+ SELECT *, pg_typeof(f1) FROM
+   (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1;
+  f1  | pg_typeof
+ -----+-----------
+  foo | text
+  foo | text
+  foo | text
+ (3 rows)
+
+ -- ... unless there's context to suggest differently
+ explain verbose select '42' union all select '43';
+                    QUERY PLAN
+ -------------------------------------------------
+  Append  (cost=0.00..0.04 rows=2 width=32)
+    ->  Result  (cost=0.00..0.01 rows=1 width=32)
+          Output: '42'::text
+    ->  Result  (cost=0.00..0.01 rows=1 width=32)
+          Output: '43'::text
+ (5 rows)
+
+ explain verbose select '42' union all select 43;
+                    QUERY PLAN
+ ------------------------------------------------
+  Append  (cost=0.00..0.04 rows=2 width=4)
+    ->  Result  (cost=0.00..0.01 rows=1 width=4)
+          Output: 42
+    ->  Result  (cost=0.00..0.01 rows=1 width=4)
+          Output: 43
+ (5 rows)
+
  --
  -- Use some existing tables in the regression test
  --
diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out
index 02fa08e..3b7f689 100644
*** a/src/test/regress/expected/with.out
--- b/src/test/regress/expected/with.out
*************** SELECT * FROM t LIMIT 10;
*** 133,141 ****

  -- Test behavior with an unknown-type literal in the WITH
  WITH q AS (SELECT 'foo' AS x)
! SELECT x, x IS OF (unknown) as is_unknown FROM q;
!   x  | is_unknown
! -----+------------
   foo | t
  (1 row)

--- 133,141 ----

  -- Test behavior with an unknown-type literal in the WITH
  WITH q AS (SELECT 'foo' AS x)
! SELECT x, x IS OF (text) AS is_text FROM q;
!   x  | is_text
! -----+---------
   foo | t
  (1 row)

*************** WITH RECURSIVE t(n) AS (
*** 144,150 ****
  UNION ALL
      SELECT n || ' bar' FROM t WHERE length(n) < 20
  )
! SELECT n, n IS OF (text) as is_text FROM t;
              n            | is_text
  -------------------------+---------
   foo                     | t
--- 144,150 ----
  UNION ALL
      SELECT n || ' bar' FROM t WHERE length(n) < 20
  )
! SELECT n, n IS OF (text) AS is_text FROM t;
              n            | is_text
  -------------------------+---------
   foo                     | t
*************** SELECT n, n IS OF (text) as is_text FROM
*** 155,160 ****
--- 155,172 ----
   foo bar bar bar bar bar | t
  (6 rows)

+ -- In a perfect world, this would work and resolve the literal as int ...
+ -- but for now, we have to be content with resolving to text too soon.
+ WITH RECURSIVE t(n) AS (
+     SELECT '7'
+ UNION ALL
+     SELECT n+1 FROM t WHERE n < 10
+ )
+ SELECT n, n IS OF (int) AS is_int FROM t;
+ ERROR:  operator does not exist: text + integer
+ LINE 4:     SELECT n+1 FROM t WHERE n < 10
+                     ^
+ HINT:  No operator matches the given name and argument type(s). You might need to add explicit type casts.
  --
  -- Some examples with a tree
  --
diff --git a/src/test/regress/output/create_function_1.source b/src/test/regress/output/create_function_1.source
index 30c2936..957595c 100644
*** a/src/test/regress/output/create_function_1.source
--- b/src/test/regress/output/create_function_1.source
*************** CREATE FUNCTION test_atomic_ops()
*** 59,65 ****
  CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
      AS 'SELECT ''not an integer'';';
  ERROR:  return type mismatch in function declared to return integer
! DETAIL:  Actual return type is unknown.
  CONTEXT:  SQL function "test1"
  CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
      AS 'not even SQL';
--- 59,65 ----
  CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
      AS 'SELECT ''not an integer'';';
  ERROR:  return type mismatch in function declared to return integer
! DETAIL:  Actual return type is text.
  CONTEXT:  SQL function "test1"
  CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
      AS 'not even SQL';
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index 8242e73..6314aa4 100644
*** a/src/test/regress/sql/create_table.sql
--- b/src/test/regress/sql/create_table.sql
*************** CREATE TABLE testjsonb (
*** 236,241 ****
--- 236,249 ----
         j jsonb
  );

+ CREATE TABLE unknowntab (
+     u unknown    -- fail
+ );
+
+ CREATE TYPE unknown_comptype AS (
+     u unknown    -- fail
+ );
+
  CREATE TABLE IF NOT EXISTS test_tsvector(
      t text,
      a tsvector
diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index 5fe8b94..c27f103 100644
*** a/src/test/regress/sql/create_view.sql
--- b/src/test/regress/sql/create_view.sql
*************** SELECT relname, relkind, reloptions FROM
*** 224,229 ****
--- 224,237 ----
                       'mysecview3'::regclass, 'mysecview4'::regclass)
         ORDER BY relname;

+ -- Check that unknown literals are converted to "text" in CREATE VIEW,
+ -- so that we don't end up with unknown-type columns.
+
+ CREATE VIEW unspecified_types AS
+   SELECT 42 as i, 42.5 as num, 'foo' as u, 'foo'::unknown as u2, null as n;
+ \d+ unspecified_types
+ SELECT * FROM unspecified_types;
+
  -- This test checks that proper typmods are assigned in a multi-row VALUES

  CREATE VIEW tt1 AS
diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql
index 65a743c..1164b4c 100644
*** a/src/test/regress/sql/matview.sql
--- b/src/test/regress/sql/matview.sql
*************** SELECT * FROM mvtest_mv_v_3;
*** 198,203 ****
--- 198,211 ----
  SELECT * FROM mvtest_mv_v_4;
  DROP TABLE mvtest_v CASCADE;

+ -- Check that unknown literals are converted to "text" in CREATE MATVIEW,
+ -- so that we don't end up with unknown-type columns.
+ CREATE MATERIALIZED VIEW mv_unspecified_types AS
+   SELECT 42 as i, 42.5 as num, 'foo' as u, 'foo'::unknown as u2, null as n;
+ \d+ mv_unspecified_types
+ SELECT * FROM mv_unspecified_types;
+ DROP MATERIALIZED VIEW mv_unspecified_types;
+
  -- make sure that create WITH NO DATA does not plan the query (bug #13907)
  create materialized view mvtest_error as select 1/0 as x;  -- fail
  create materialized view mvtest_error as select 1/0 as x with no data;
diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index 08eb825..9c2a73d 100644
*** a/src/test/regress/sql/subselect.sql
--- b/src/test/regress/sql/subselect.sql
*************** SELECT '' AS five, f1 AS "Correlated Fie
*** 80,85 ****
--- 80,95 ----
    WHERE (f1, f2) IN (SELECT f2, CAST(f3 AS int4) FROM SUBSELECT_TBL
                       WHERE f3 IS NOT NULL);

+ -- Unspecified-type literals in output columns should resolve as text
+
+ SELECT *, pg_typeof(f1) FROM
+   (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1;
+
+ -- ... unless there's context to suggest differently
+
+ explain verbose select '42' union all select '43';
+ explain verbose select '42' union all select 43;
+
  --
  -- Use some existing tables in the regression test
  --
diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql
index 7ee32ba..08ddc8b 100644
*** a/src/test/regress/sql/with.sql
--- b/src/test/regress/sql/with.sql
*************** SELECT * FROM t LIMIT 10;
*** 69,82 ****

  -- Test behavior with an unknown-type literal in the WITH
  WITH q AS (SELECT 'foo' AS x)
! SELECT x, x IS OF (unknown) as is_unknown FROM q;

  WITH RECURSIVE t(n) AS (
      SELECT 'foo'
  UNION ALL
      SELECT n || ' bar' FROM t WHERE length(n) < 20
  )
! SELECT n, n IS OF (text) as is_text FROM t;

  --
  -- Some examples with a tree
--- 69,91 ----

  -- Test behavior with an unknown-type literal in the WITH
  WITH q AS (SELECT 'foo' AS x)
! SELECT x, x IS OF (text) AS is_text FROM q;

  WITH RECURSIVE t(n) AS (
      SELECT 'foo'
  UNION ALL
      SELECT n || ' bar' FROM t WHERE length(n) < 20
  )
! SELECT n, n IS OF (text) AS is_text FROM t;
!
! -- In a perfect world, this would work and resolve the literal as int ...
! -- but for now, we have to be content with resolving to text too soon.
! WITH RECURSIVE t(n) AS (
!     SELECT '7'
! UNION ALL
!     SELECT n+1 FROM t WHERE n < 10
! )
! SELECT n, n IS OF (int) AS is_int FROM t;

  --
  -- Some examples with a tree

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers