From 0b78106cd4651ad7867036a463ec743f6d3d2e86 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 8 Dec 2016 11:40:02 -0500
Subject: [PATCH] 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. 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
---
src/backend/nodes/copyfuncs.c | 7 +-
src/backend/nodes/equalfuncs.c | 7 +-
src/backend/nodes/outfuncs.c | 10 ++-
src/backend/nodes/readfuncs.c | 10 ++-
src/backend/optimizer/plan/setrefs.c | 7 +-
src/backend/parser/analyze.c | 54 +++++++++----
src/backend/parser/parse_relation.c | 94 ++++++-----------------
src/include/catalog/catversion.h | 2 +-
src/include/nodes/parsenodes.h | 15 +++-
src/include/parser/parse_relation.h | 4 +-
src/test/regress/expected/create_view.out | 37 +++++++++
src/test/regress/sql/create_view.sql | 13 ++++
12 files changed, 151 insertions(+), 109 deletions(-)
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index e30c57e86b..d9732259ae 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -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);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index b7a109cfb0..edc1797c42 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -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);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 0d858f5920..7258c0357d 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -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);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index c587d4e1d7..d608530c6e 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -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",
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index d91bc3b30d..2fe1c8cb9d 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -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);
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 7364346167..5e65fe75bd 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -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);
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 1e3ecbc51e..58f70508fe 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -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:
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index a602ba3861..cfe01f19ac 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 201612071
+#define CATALOG_VERSION_NO 201612081
#endif
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 6b95c48447..fc532fbd43 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -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:
diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h
index 3ef3d7bae5..9463f9d775 100644
--- a/src/include/parser/parse_relation.h
+++ b/src/include/parser/parse_relation.h
@@ -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);
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 66ed2c8a3e..096bfc30c9 100644
--- 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 | 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);
diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index 8bed5a53b3..5fe8b94aae 100644
--- 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);