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 1e3ecbc51ef288a9c4462662614bc463600028b0..c51fd81b63f967a766d2986a0f8a5f88efe382a9 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);