Fix reporting of column typmods for multi-row VALUES constructs.

expandRTE() and get_rte_attribute_type() reported the exprType() and
exprTypmod() values of the expressions in the first row of the VALUES as
being the column type/typmod returned by the VALUES RTE.  That's fine for
the data type, since we coerce all expressions in a column to have the same
common type.  But we don't coerce them to have a common typmod, so it was
possible for rows after the first one to return values that violate the
claimed column typmod.  This leads to the incorrect result seen in bug
#14448 from Hassan Mahmood, as well as some other corner-case misbehaviors.

The desired behavior is the same as we use in other type-unification
cases: report the common typmod if there is one, but otherwise return -1
indicating no particular constraint.

We fixed this in HEAD by deriving the typmods during transformValuesClause
and storing them in the RTE, but that's not a feasible solution in the back
branches.  Instead, just use a brute-force approach of determining the
correct common typmod during expandRTE() and get_rte_attribute_type().
Simple testing says that that doesn't really cost much, at least not in
common cases where expandRTE() is only used once per query.  It turns out
that get_rte_attribute_type() is typically never used at all on VALUES
RTEs, so the inefficiency there is of no great concern.

Report: https://postgr.es/m/20161205143037.4377.60754@wrigleys.postgresql.org
Discussion: https://postgr.es/m/27429.1480968538@sss.pgh.pa.us

diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 795eee24dea28f7e4f094097b3b6d34868e656c5..ae84df308ac32886a1a81fab7f80878c563352ec 100644 (file)
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -52,6 +52,7 @@ static void expandTupleDesc(TupleDesc tupdesc, Alias *eref,
                int rtindex, int sublevels_up,
                int location, bool include_dropped,
                List **colnames, List **colvars);
+static int32 *getValuesTypmods(RangeTblEntry *rte);
 static int specialAttNum(const char *attname);
 static bool isQueryUsingTempRelation_walker(Node *node, void *context);
 
@@ -2157,9 +2158,22 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
            {
                /* Values RTE */
                ListCell   *aliasp_item = list_head(rte->eref->colnames);
+               int32      *coltypmods;
                ListCell   *lcv;
                ListCell   *lcc;
 
+               /*
+                * It's okay to extract column types from the expressions in
+                * the first row, since all rows will have been coerced to the
+                * same types.  Their typmods might not be the same though, so
+                * we potentially need to examine all rows to compute those.
+                * Column collations are pre-computed in values_collations.
+                */
+               if (colvars)
+                   coltypmods = getValuesTypmods(rte);
+               else
+                   coltypmods = NULL;
+
                varattno = 0;
                forboth(lcv, (List *) linitial(rte->values_lists),
                        lcc, rte->values_collations)
@@ -2184,13 +2198,15 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
 
                        varnode = makeVar(rtindex, varattno,
                                          exprType(col),
-                                         exprTypmod(col),
+                                         coltypmods[varattno - 1],
                                          colcollation,
                                          sublevels_up);
                        varnode->location = location;
                        *colvars = lappend(*colvars, varnode);
                    }
                }
+               if (coltypmods)
+                   pfree(coltypmods);
            }
            break;
        case RTE_JOIN:
@@ -2296,6 +2312,8 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
                        varnode = makeVar(rtindex, varattno,
                                          coltype, coltypmod, colcoll,
                                          sublevels_up);
+                       varnode->location = location;
+
                        *colvars = lappend(*colvars, varnode);
                    }
                }
@@ -2412,6 +2430,74 @@ expandTupleDesc(TupleDesc tupdesc, Alias *eref, int count, int offset,
    }
 }
 
+/*
+ * getValuesTypmods -- expandRTE subroutine
+ *
+ * Identify per-column typmods for the given VALUES RTE.  Returns a
+ * palloc'd array.
+ */
+static int32 *
+getValuesTypmods(RangeTblEntry *rte)
+{
+   int32      *coltypmods;
+   List       *firstrow;
+   int         ncolumns,
+               nvalid,
+               i;
+   ListCell   *lc;
+
+   Assert(rte->values_lists != NIL);
+   firstrow = (List *) linitial(rte->values_lists);
+   ncolumns = list_length(firstrow);
+   coltypmods = (int32 *) palloc(ncolumns * sizeof(int32));
+   nvalid = 0;
+
+   /* Collect the typmods from the first VALUES row */
+   i = 0;
+   foreach(lc, firstrow)
+   {
+       Node       *col = (Node *) lfirst(lc);
+
+       coltypmods[i] = exprTypmod(col);
+       if (coltypmods[i] >= 0)
+           nvalid++;
+       i++;
+   }
+
+   /*
+    * Scan remaining rows; as soon as we have a non-matching typmod for a
+    * column, reset that typmod to -1.  We can bail out early if all typmods
+    * become -1.
+    */
+   if (nvalid > 0)
+   {
+       for_each_cell(lc, lnext(list_head(rte->values_lists)))
+       {
+           List       *thisrow = (List *) lfirst(lc);
+           ListCell   *lc2;
+
+           Assert(list_length(thisrow) == ncolumns);
+           i = 0;
+           foreach(lc2, thisrow)
+           {
+               Node       *col = (Node *) lfirst(lc2);
+
+               if (coltypmods[i] >= 0 && coltypmods[i] != exprTypmod(col))
+               {
+                   coltypmods[i] = -1;
+                   nvalid--;
+               }
+               i++;
+           }
+
+           if (nvalid <= 0)
+               break;
+       }
+   }
+
+   return coltypmods;
+}
+
 /*
  * expandRelAttrs -
  *   Workhorse for "*" expansion: produce a list of targetentries
@@ -2656,18 +2742,25 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
            break;
        case RTE_VALUES:
            {
-               /* Values RTE --- get type info from first sublist */
-               /* collation is stored separately, though */
+               /*
+                * Values RTE --- we can get type info from first sublist, but
+                * typmod may require scanning all sublists, and collation is
+                * stored separately.  Using getValuesTypmods() is overkill,
+                * but this path is taken so seldom for VALUES that it's not
+                * worth writing extra code.
+                */
                List       *collist = (List *) linitial(rte->values_lists);
                Node       *col;
+               int32      *coltypmods = getValuesTypmods(rte);
 
                if (attnum < 1 || attnum > list_length(collist))
                    elog(ERROR, "values list %s does not have attribute %d",
                         rte->eref->aliasname, attnum);
                col = (Node *) list_nth(collist, attnum - 1);
                *vartype = exprType(col);
-               *vartypmod = exprTypmod(col);
+               *vartypmod = coltypmods[attnum - 1];
                *varcollid = list_nth_oid(rte->values_collations, attnum - 1);
+               pfree(coltypmods);
            }
            break;
        case RTE_JOIN:

diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 81b4e8d42bb95ba31435be7af6dbd42c21b3ffb1..b1c3cff9315a7d1e98f533ea9f5885bbdffbd180 100644 (file)
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -288,6 +288,43 @@ SELECT relname, relkind, reloptions FROM pg_class
  mysecview4 | v       | {security_barrier=false}
 (4 rows)
 
+-- This test checks that proper typmods are assigned in a multi-row VALUES
+CREATE VIEW tt1 AS
+  SELECT * FROM (
+    VALUES
+       ('abc'::varchar(3), '0123456789', 42, 'abcd'::varchar(4)),
+       ('0123456789', 'abc'::varchar(3), 42.12, 'abc'::varchar(4))
+  ) vv(a,b,c,d);
+\d+ tt1
+                      View "testviewschm2.tt1"
+ Column |         Type         | Modifiers | Storage  | Description 
+--------+----------------------+-----------+----------+-------------
+ a      | character varying    |           | extended | 
+ b      | character varying    |           | extended | 
+ c      | numeric              |           | main     | 
+ d      | character varying(4) |           | extended | 
+View definition:
+ SELECT vv.a,
+    vv.b,
+    vv.c,
+    vv.d
+   FROM ( VALUES ('abc'::character varying(3),'0123456789'::character varying,42,'abcd'::character varying(4)), ('0123456789'::character varying,'abc'::character varying(3),42.12,'abc'::character varying(4))) vv(a, b, c, d);
+
+SELECT * FROM tt1;
+     a      |     b      |   c   |  d   
+------------+------------+-------+------
+ abc        | 0123456789 |    42 | abcd
+ 0123456789 | abc        | 42.12 | abc
+(2 rows)
+
+SELECT a::varchar(3) FROM tt1;
+  a  
+-----
+ abc
+ 012
+(2 rows)
+
+DROP VIEW tt1;
 -- Test view decompilation in the face of relation renaming conflicts
 CREATE TABLE tt1 (f1 int, f2 int, f3 text);
 CREATE TABLE tx1 (x1 int, x2 int, x3 text);

diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index 8bed5a53b3aaa38bda600e131f1299ec17535607..5fe8b94aae0079d91c9d24cf8224a9a3effbd415 100644 (file)
--- a/src/test/regress/sql/create_view.sql
+++ b/src/test/regress/sql/create_view.sql
@@ -224,6 +224,19 @@ SELECT relname, relkind, reloptions FROM pg_class
                      'mysecview3'::regclass, 'mysecview4'::regclass)
        ORDER BY relname;
 
+-- This test checks that proper typmods are assigned in a multi-row VALUES
+
+CREATE VIEW tt1 AS
+  SELECT * FROM (
+    VALUES
+       ('abc'::varchar(3), '0123456789', 42, 'abcd'::varchar(4)),
+       ('0123456789', 'abc'::varchar(3), 42.12, 'abc'::varchar(4))
+  ) vv(a,b,c,d);
+\d+ tt1
+SELECT * FROM tt1;
+SELECT a::varchar(3) FROM tt1;
+DROP VIEW tt1;
+
 -- Test view decompilation in the face of relation renaming conflicts
 
 CREATE TABLE tt1 (f1 int, f2 int, f3 text);