Fix reporting of column typmods for multi-row VALUES constructs.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 8 Dec 2016 16:40:02 +0000 (11:40 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 8 Dec 2016 16:40:02 +0000 (11:40 -0500)
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.  It's cheap for transformValuesClause
to determine the common typmod while transforming a multi-row VALUES, but
it'd be less cheap for expandRTE() and get_rte_attribute_type() to
re-determine that info every time they're asked --- possibly a lot less
cheap, if the VALUES has many rows.  Therefore, the best fix is to record
the common typmods explicitly in a list in the VALUES RTE, as we were
already doing for column collations.  This looks quite a bit like what
we're doing for CTE RTEs, so we can save a little bit of space and code by
unifying the representation for those two RTE types.  They both now share
coltypes/coltypmods/colcollations fields.  (At some point it might seem
desirable to populate those fields for all RTE types; but right now it
looks like constructing them for other RTE types would add more code and
cycles than it would save.)

The RTE change requires a catversion bump, so this fix is only usable
in HEAD.  If we fix this at all in the back branches, the patch will
need to look quite different.

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

12 files changed:
src/backend/nodes/copyfuncs.c
src/backend/nodes/equalfuncs.c
src/backend/nodes/outfuncs.c
src/backend/nodes/readfuncs.c
src/backend/optimizer/plan/setrefs.c
src/backend/parser/analyze.c
src/backend/parser/parse_relation.c
src/include/catalog/catversion.h
src/include/nodes/parsenodes.h
src/include/parser/parse_relation.h
src/test/regress/expected/create_view.out
src/test/regress/sql/create_view.sql

index e30c57e86be341f32c02dab1fbeddfef2fc990f7..d9732259ae826b6e07aff2d207fc765c2cdeefaf 100644 (file)
@@ -2149,13 +2149,12 @@ _copyRangeTblEntry(const RangeTblEntry *from)
    COPY_NODE_FIELD(functions);
    COPY_SCALAR_FIELD(funcordinality);
    COPY_NODE_FIELD(values_lists);
-   COPY_NODE_FIELD(values_collations);
    COPY_STRING_FIELD(ctename);
    COPY_SCALAR_FIELD(ctelevelsup);
    COPY_SCALAR_FIELD(self_reference);
-   COPY_NODE_FIELD(ctecoltypes);
-   COPY_NODE_FIELD(ctecoltypmods);
-   COPY_NODE_FIELD(ctecolcollations);
+   COPY_NODE_FIELD(coltypes);
+   COPY_NODE_FIELD(coltypmods);
+   COPY_NODE_FIELD(colcollations);
    COPY_NODE_FIELD(alias);
    COPY_NODE_FIELD(eref);
    COPY_SCALAR_FIELD(lateral);
index b7a109cfb046ac11f9fc92f28a305d70b55cd65e..edc1797c424dff113e6e6d90cfeac0d1124abf6e 100644 (file)
@@ -2460,13 +2460,12 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b)
    COMPARE_NODE_FIELD(functions);
    COMPARE_SCALAR_FIELD(funcordinality);
    COMPARE_NODE_FIELD(values_lists);
-   COMPARE_NODE_FIELD(values_collations);
    COMPARE_STRING_FIELD(ctename);
    COMPARE_SCALAR_FIELD(ctelevelsup);
    COMPARE_SCALAR_FIELD(self_reference);
-   COMPARE_NODE_FIELD(ctecoltypes);
-   COMPARE_NODE_FIELD(ctecoltypmods);
-   COMPARE_NODE_FIELD(ctecolcollations);
+   COMPARE_NODE_FIELD(coltypes);
+   COMPARE_NODE_FIELD(coltypmods);
+   COMPARE_NODE_FIELD(colcollations);
    COMPARE_NODE_FIELD(alias);
    COMPARE_NODE_FIELD(eref);
    COMPARE_SCALAR_FIELD(lateral);
index 0d858f592067a2ea471149ed21e05b1b262d5075..7258c0357dbb06028f0758f37faf532c7f5163dc 100644 (file)
@@ -2841,15 +2841,17 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
            break;
        case RTE_VALUES:
            WRITE_NODE_FIELD(values_lists);
-           WRITE_NODE_FIELD(values_collations);
+           WRITE_NODE_FIELD(coltypes);
+           WRITE_NODE_FIELD(coltypmods);
+           WRITE_NODE_FIELD(colcollations);
            break;
        case RTE_CTE:
            WRITE_STRING_FIELD(ctename);
            WRITE_UINT_FIELD(ctelevelsup);
            WRITE_BOOL_FIELD(self_reference);
-           WRITE_NODE_FIELD(ctecoltypes);
-           WRITE_NODE_FIELD(ctecoltypmods);
-           WRITE_NODE_FIELD(ctecolcollations);
+           WRITE_NODE_FIELD(coltypes);
+           WRITE_NODE_FIELD(coltypmods);
+           WRITE_NODE_FIELD(colcollations);
            break;
        default:
            elog(ERROR, "unrecognized RTE kind: %d", (int) node->rtekind);
index c587d4e1d7257ab1fc5982f80c6dfbbe15413bff..d608530c6e211cbd599926425489b36365f93146 100644 (file)
@@ -1314,15 +1314,17 @@ _readRangeTblEntry(void)
            break;
        case RTE_VALUES:
            READ_NODE_FIELD(values_lists);
-           READ_NODE_FIELD(values_collations);
+           READ_NODE_FIELD(coltypes);
+           READ_NODE_FIELD(coltypmods);
+           READ_NODE_FIELD(colcollations);
            break;
        case RTE_CTE:
            READ_STRING_FIELD(ctename);
            READ_UINT_FIELD(ctelevelsup);
            READ_BOOL_FIELD(self_reference);
-           READ_NODE_FIELD(ctecoltypes);
-           READ_NODE_FIELD(ctecoltypmods);
-           READ_NODE_FIELD(ctecolcollations);
+           READ_NODE_FIELD(coltypes);
+           READ_NODE_FIELD(coltypmods);
+           READ_NODE_FIELD(colcollations);
            break;
        default:
            elog(ERROR, "unrecognized RTE kind: %d",
index d91bc3b30de5c80f9112f52003cb794de72688fe..2fe1c8cb9d05705a84140379ac53efaca2fbd903 100644 (file)
@@ -396,10 +396,9 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte)
    newrte->joinaliasvars = NIL;
    newrte->functions = NIL;
    newrte->values_lists = NIL;
-   newrte->values_collations = NIL;
-   newrte->ctecoltypes = NIL;
-   newrte->ctecoltypmods = NIL;
-   newrte->ctecolcollations = NIL;
+   newrte->coltypes = NIL;
+   newrte->coltypmods = NIL;
+   newrte->colcollations = NIL;
    newrte->securityQuals = NIL;
 
    glob->finalrtable = lappend(glob->finalrtable, newrte);
index 73643461672d9b431e5cdbbba68ad2efd979bfa5..5e65fe75bd53c747399ccce66c198cd895f6b0b1 100644 (file)
@@ -633,10 +633,11 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
         * RTE.
         */
        List       *exprsLists = NIL;
-       List       *collations = NIL;
+       List       *coltypes = NIL;
+       List       *coltypmods = NIL;
+       List       *colcollations = NIL;
        int         sublist_length = -1;
        bool        lateral = false;
-       int         i;
 
        Assert(selectStmt->intoClause == NULL);
 
@@ -703,11 +704,20 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
        }
 
        /*
-        * Although we don't really need collation info, let's just make sure
-        * we provide a correctly-sized list in the VALUES RTE.
+        * Construct column type/typmod/collation lists for the VALUES RTE.
+        * Every expression in each column has been coerced to the type/typmod
+        * of the corresponding target column or subfield, so it's sufficient
+        * to look at the exprType/exprTypmod of the first row.  We don't care
+        * about the collation labeling, so just fill in InvalidOid for that.
         */
-       for (i = 0; i < sublist_length; i++)
-           collations = lappend_oid(collations, InvalidOid);
+       foreach(lc, (List *) linitial(exprsLists))
+       {
+           Node       *val = (Node *) lfirst(lc);
+
+           coltypes = lappend_oid(coltypes, exprType(val));
+           coltypmods = lappend_int(coltypmods, exprTypmod(val));
+           colcollations = lappend_oid(colcollations, InvalidOid);
+       }
 
        /*
         * Ordinarily there can't be any current-level Vars in the expression
@@ -722,7 +732,8 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
        /*
         * Generate the VALUES RTE
         */
-       rte = addRangeTableEntryForValues(pstate, exprsLists, collations,
+       rte = addRangeTableEntryForValues(pstate, exprsLists,
+                                         coltypes, coltypmods, colcollations,
                                          NULL, lateral, true);
        rtr = makeNode(RangeTblRef);
        /* assume new rte is at end */
@@ -1274,7 +1285,9 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
 {
    Query      *qry = makeNode(Query);
    List       *exprsLists;
-   List       *collations;
+   List       *coltypes = NIL;
+   List       *coltypmods = NIL;
+   List       *colcollations = NIL;
    List      **colexprs = NULL;
    int         sublist_length = -1;
    bool        lateral = false;
@@ -1360,8 +1373,8 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
 
    /*
     * Now resolve the common types of the columns, and coerce everything to
-    * those types.  Then identify the common collation, if any, of each
-    * column.
+    * those types.  Then identify the common typmod and common collation, if
+    * any, of each column.
     *
     * We must do collation processing now because (1) assign_query_collations
     * doesn't process rangetable entries, and (2) we need to label the VALUES
@@ -1372,11 +1385,12 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
     *
     * Note we modify the per-column expression lists in-place.
     */
-   collations = NIL;
    for (i = 0; i < sublist_length; i++)
    {
        Oid         coltype;
+       int32       coltypmod = -1;
        Oid         colcoll;
+       bool        first = true;
 
        coltype = select_common_type(pstate, colexprs[i], "VALUES", NULL);
 
@@ -1386,11 +1400,24 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
 
            col = coerce_to_common_type(pstate, col, coltype, "VALUES");
            lfirst(lc) = (void *) col;
+           if (first)
+           {
+               coltypmod = exprTypmod(col);
+               first = false;
+           }
+           else
+           {
+               /* As soon as we see a non-matching typmod, fall back to -1 */
+               if (coltypmod >= 0 && coltypmod != exprTypmod(col))
+                   coltypmod = -1;
+           }
        }
 
        colcoll = select_common_collation(pstate, colexprs[i], true);
 
-       collations = lappend_oid(collations, colcoll);
+       coltypes = lappend_oid(coltypes, coltype);
+       coltypmods = lappend_int(coltypmods, coltypmod);
+       colcollations = lappend_oid(colcollations, colcoll);
    }
 
    /*
@@ -1432,7 +1459,8 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
    /*
     * Generate the VALUES RTE
     */
-   rte = addRangeTableEntryForValues(pstate, exprsLists, collations,
+   rte = addRangeTableEntryForValues(pstate, exprsLists,
+                                     coltypes, coltypmods, colcollations,
                                      NULL, lateral, true);
    addRTEtoQuery(pstate, rte, true, true, true);
 
index 1e3ecbc51ef288a9c4462662614bc463600028b0..58f70508feae15b2dde2a5e987672763c7d73078 100644 (file)
@@ -1635,7 +1635,9 @@ addRangeTableEntryForFunction(ParseState *pstate,
 RangeTblEntry *
 addRangeTableEntryForValues(ParseState *pstate,
                            List *exprs,
-                           List *collations,
+                           List *coltypes,
+                           List *coltypmods,
+                           List *colcollations,
                            Alias *alias,
                            bool lateral,
                            bool inFromCl)
@@ -1652,7 +1654,9 @@ addRangeTableEntryForValues(ParseState *pstate,
    rte->relid = InvalidOid;
    rte->subquery = NULL;
    rte->values_lists = exprs;
-   rte->values_collations = collations;
+   rte->coltypes = coltypes;
+   rte->coltypmods = coltypmods;
+   rte->colcollations = colcollations;
    rte->alias = alias;
 
    eref = alias ? copyObject(alias) : makeAlias(refname, NIL);
@@ -1822,9 +1826,9 @@ addRangeTableEntryForCTE(ParseState *pstate,
                     parser_errposition(pstate, rv->location)));
    }
 
-   rte->ctecoltypes = cte->ctecoltypes;
-   rte->ctecoltypmods = cte->ctecoltypmods;
-   rte->ctecolcollations = cte->ctecolcollations;
+   rte->coltypes = cte->ctecoltypes;
+   rte->coltypmods = cte->ctecoltypmods;
+   rte->colcollations = cte->ctecolcollations;
 
    rte->alias = alias;
    if (alias)
@@ -2153,46 +2157,6 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
                }
            }
            break;
-       case RTE_VALUES:
-           {
-               /* Values RTE */
-               ListCell   *aliasp_item = list_head(rte->eref->colnames);
-               ListCell   *lcv;
-               ListCell   *lcc;
-
-               varattno = 0;
-               forboth(lcv, (List *) linitial(rte->values_lists),
-                       lcc, rte->values_collations)
-               {
-                   Node       *col = (Node *) lfirst(lcv);
-                   Oid         colcollation = lfirst_oid(lcc);
-
-                   varattno++;
-                   if (colnames)
-                   {
-                       /* Assume there is one alias per column */
-                       char       *label = strVal(lfirst(aliasp_item));
-
-                       *colnames = lappend(*colnames,
-                                           makeString(pstrdup(label)));
-                       aliasp_item = lnext(aliasp_item);
-                   }
-
-                   if (colvars)
-                   {
-                       Var        *varnode;
-
-                       varnode = makeVar(rtindex, varattno,
-                                         exprType(col),
-                                         exprTypmod(col),
-                                         colcollation,
-                                         sublevels_up);
-                       varnode->location = location;
-                       *colvars = lappend(*colvars, varnode);
-                   }
-               }
-           }
-           break;
        case RTE_JOIN:
            {
                /* Join RTE */
@@ -2262,17 +2226,19 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
                }
            }
            break;
+       case RTE_VALUES:
        case RTE_CTE:
            {
+               /* Values or CTE RTE */
                ListCell   *aliasp_item = list_head(rte->eref->colnames);
                ListCell   *lct;
                ListCell   *lcm;
                ListCell   *lcc;
 
                varattno = 0;
-               forthree(lct, rte->ctecoltypes,
-                        lcm, rte->ctecoltypmods,
-                        lcc, rte->ctecolcollations)
+               forthree(lct, rte->coltypes,
+                        lcm, rte->coltypmods,
+                        lcc, rte->colcollations)
                {
                    Oid         coltype = lfirst_oid(lct);
                    int32       coltypmod = lfirst_int(lcm);
@@ -2285,7 +2251,8 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
                        /* Assume there is one alias per output column */
                        char       *label = strVal(lfirst(aliasp_item));
 
-                       *colnames = lappend(*colnames, makeString(pstrdup(label)));
+                       *colnames = lappend(*colnames,
+                                           makeString(pstrdup(label)));
                        aliasp_item = lnext(aliasp_item);
                    }
 
@@ -2296,6 +2263,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);
                    }
                }
@@ -2654,22 +2623,6 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
                                rte->eref->aliasname)));
            }
            break;
-       case RTE_VALUES:
-           {
-               /* Values RTE --- get type info from first sublist */
-               /* collation is stored separately, though */
-               List       *collist = (List *) linitial(rte->values_lists);
-               Node       *col;
-
-               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);
-               *varcollid = list_nth_oid(rte->values_collations, attnum - 1);
-           }
-           break;
        case RTE_JOIN:
            {
                /*
@@ -2685,13 +2638,14 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
                *varcollid = exprCollation(aliasvar);
            }
            break;
+       case RTE_VALUES:
        case RTE_CTE:
            {
-               /* CTE RTE --- get type info from lists in the RTE */
-               Assert(attnum > 0 && attnum <= list_length(rte->ctecoltypes));
-               *vartype = list_nth_oid(rte->ctecoltypes, attnum - 1);
-               *vartypmod = list_nth_int(rte->ctecoltypmods, attnum - 1);
-               *varcollid = list_nth_oid(rte->ctecolcollations, attnum - 1);
+               /* VALUES or CTE RTE --- get type info from lists in the RTE */
+               Assert(attnum > 0 && attnum <= list_length(rte->coltypes));
+               *vartype = list_nth_oid(rte->coltypes, attnum - 1);
+               *vartypmod = list_nth_int(rte->coltypmods, attnum - 1);
+               *varcollid = list_nth_oid(rte->colcollations, attnum - 1);
            }
            break;
        default:
index a602ba386167d2ef24c200cea7ee84f4ca89da6f..cfe01f19ac17a9f73af9a9c9b9a971177a577858 100644 (file)
@@ -53,6 +53,6 @@
  */
 
 /*                         yyyymmddN */
-#define CATALOG_VERSION_NO 201612071
+#define CATALOG_VERSION_NO 201612081
 
 #endif
index 6b95c484476b5ce92d77bd6415040a37894e25cb..fc532fbd432cbf35dcc1ae870e1d065b18c70755 100644 (file)
@@ -927,7 +927,6 @@ typedef struct RangeTblEntry
     * Fields valid for a values RTE (else NIL):
     */
    List       *values_lists;   /* list of expression lists */
-   List       *values_collations;      /* OID list of column collation OIDs */
 
    /*
     * Fields valid for a CTE RTE (else NULL/zero):
@@ -935,9 +934,17 @@ typedef struct RangeTblEntry
    char       *ctename;        /* name of the WITH list item */
    Index       ctelevelsup;    /* number of query levels up */
    bool        self_reference; /* is this a recursive self-reference? */
-   List       *ctecoltypes;    /* OID list of column type OIDs */
-   List       *ctecoltypmods;  /* integer list of column typmods */
-   List       *ctecolcollations;       /* OID list of column collation OIDs */
+
+   /*
+    * Fields valid for values and CTE RTEs (else NIL):
+    *
+    * We need these for CTE RTEs so that the types of self-referential
+    * columns are well-defined.  For VALUES RTEs, storing these explicitly
+    * saves having to re-determine the info by scanning the values_lists.
+    */
+   List       *coltypes;       /* OID list of column type OIDs */
+   List       *coltypmods;     /* integer list of column typmods */
+   List       *colcollations;  /* OID list of column collation OIDs */
 
    /*
     * Fields valid in all RTEs:
index 3ef3d7bae5b9d0440b0473f711af9b6c7630ee91..9463f9d7757df27931d9f9922cd7cd83d9927d1b 100644 (file)
@@ -85,7 +85,9 @@ extern RangeTblEntry *addRangeTableEntryForFunction(ParseState *pstate,
                              bool inFromCl);
 extern RangeTblEntry *addRangeTableEntryForValues(ParseState *pstate,
                            List *exprs,
-                           List *collations,
+                           List *coltypes,
+                           List *coltypmods,
+                           List *colcollations,
                            Alias *alias,
                            bool lateral,
                            bool inFromCl);
index 66ed2c8a3e86cada5458aa09a3b957c53372169c..096bfc30c9e884254552566a8a459a4c939ac6b8 100644 (file)
@@ -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         | Collation | Nullable | Default | 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);
index 8bed5a53b3aaa38bda600e131f1299ec17535607..5fe8b94aae0079d91c9d24cf8224a9a3effbd415 100644 (file)
@@ -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);