8000 Fix reporting of column typmods for multi-row VALUES constructs. · iakio/postgres@082d1fb · GitHub
[go: up one dir, main page]
Skip to content

Commit 082d1fb

Browse files
tglsfdctglsfdc
committed
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
1 parent a00ac62 commit 082d1fb

File tree

3 files changed

+144
-4
lines changed

3 files changed

+144
-4
lines changed

‎src/backend/parser/parse_relation.c

Lines changed: 97 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ static void expandTupleDesc(TupleDesc tupdesc, Alias *eref,
4747
int rtindex, int sublevels_up,
4848
int location, bool include_dropped,
4949
List **colnames, List **colvars);
50+
static int32 *getValuesTypmods(RangeTblEntry *rte);
5051
static int specialAttNum(const char *attname);
5152

5253

@@ -1708,9 +1709,22 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
17081709
{
17091710
/* Values RTE */
17101711
ListCell *aliasp_item = list_head(rte->eref->colnames);
1712+
int32 *coltypmods;
17111713
ListCell *lcv;
17121714
ListCell *lcc;
17131715

1716+
/*
1717+
* It's okay to extract column types from the expressions in
1718+
* the first row, since all rows will have been coerced to the
1719+
* same types. Their typmods might not be the same though, so
1720+
* we potentially need to examine all rows to compute those.
1721+
* Column collations are pre-computed in values_collations.
1722+
*/
1723+
if (colvars)
1724+
coltypmods = getValuesTypmods(rte);
1725+
else
1726+
coltypmods = NULL;
1727+
17141728
varattno = 0;
17151729
forboth(lcv, (List *) linitial(rte->values_lists),
17161730
lcc, rte->values_collations)
@@ -1735,13 +1749,15 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
17351749

17361750
varnode = makeVar(rtindex, varattno,
17371751
exprType(col),
1738-
exprTypmod(col),
1752+
coltypmods[varattno - 1],
17391753
colcollation,
17401754
sublevels_up);
17411755
varnode->location = location;
17421756
*colvars = lappend(*colvars, varnode);
17431757
}
17441758
}
1759+
if (coltypmods)
1760+
pfree(coltypmods);
17451761
}
17461762
break;
17471763
case RTE_JOIN:
@@ -1847,6 +1863,8 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
18471863
varnode = makeVar(rtindex, varattno,
18481864
coltype, coltypmod, colcoll,
18491865
sublevels_up);
1866+
varnode->location = location;
1867+
18501868
*colvars = lappend(*colvars, varnode);
18511869
}
18521870
}
@@ -1937,6 +1955,74 @@ expandTupleDesc(TupleDesc tupdesc, Alias *eref,
19371955
}
19381956
}
19391957

1958+
/*
1959+
* getValuesTypmods -- expandRTE subroutine
1960+
*
1961+
* Identify per-column typmods for the given VALUES RTE. Returns a
1962+
* palloc'd array.
1963+
*/
1964+
static int32 *
1965+
getValuesTypmods(RangeTblEntry *rte)
1966+
{
1967+
int32 *coltypmods;
1968+
List *firstrow;
1969+
int ncolumns,
1970+
nvalid,
1971+
i;
1972+
ListCell *lc;
1973+
1974+
Assert(rte->values_lists != NIL);
1975+
firstrow = (List *) linitial(rte->values_lists);
1976+
ncolumns = list_length(firstrow);
1977+
coltypmods = (int32 *) palloc(ncolumns * sizeof(int32));
1978+
nvalid = 0;
1979+
1980+
/* Collect the typmods from the first VALUES row */
1981+
i = 0;
1982+
foreach(lc, firstrow)
1983+
{
1984+
Node *col = (Node *) lfirst(lc);
1985+
1986+
coltypmods[i] = exprTypmod(col);
1987+
if (coltypmods[i] >= 0)
1988+
nvalid++;
1989+
i++;
1990+
}
1991+
1992+
/*
1993+
* Scan remaining rows; as soon as we have a non-matching typmod for a
1994+
* column, reset that typmod to -1. We can bail out early if all typmods
1995+
* become -1.
1996+
*/
1997+
if (nvalid > 0)
1998+
{
1999+
for_each_cell(lc, lnext(list_head(rte->values_lists)))
2000+
{
2001+
List *thisrow = (List *) lfirst(lc);
2002+
ListCell *lc2;
2003+
2004+
Assert(list_length(thisrow) == ncolumns);
2005+
i = 0;
2006+
foreach(lc2, thisrow)
2007+
{
2008+
Node *col = (Node *) lfirst(lc2);
2009+
2010+
if (coltypmods[i] >= 0 && coltypmods[i] != exprTypmod(col))
2011+
{
2012+
coltypmods[i] = -1;
2013+
nvalid--;
2014+
}
2015+
i++;
2016+
}
2017+
2018+
if (nvalid <= 0)
2019+
break;
2020+
}
2021+
}
2022+
2023+
return coltypmods;
2024+
}
2025+
19402026
/*
19412027 F438
* expandRelAttrs -
19422028
* Workhorse for "*" expansion: produce a list of targetentries
@@ -2150,18 +2236,25 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
21502236
break;
21512237
case RTE_VALUES:
21522238
{
2153-
/* Values RTE --- get type info from first sublist */
2154-
/* collation is stored separately, though */
2239+
/*
2240+
* Values RTE --- we can get type info from first sublist, but
2241+
* typmod may require scanning all sublists, and collation is
2242+
* stored separately. Using getValuesTypmods() is overkill,
2243+
* but this path is taken so seldom for VALUES that it's not
2244+
* worth writing extra code.
2245+
*/
21552246
List *collist = (List *) linitial(rte->values_lists);
21562247
Node *col;
2248+
int32 *coltypmods = getValuesTypmods(rte);
21572249

21582250
if (attnum < 1 || attnum > list_length(collist))
21592251
elog(ERROR, "values list %s does not have attribute %d",
21602252
rte->eref->aliasname, attnum);
21612253
col = (Node *) list_nth(collist, attnum - 1);
21622254
*vartype = exprType(col);
2163-
*vartypmod = exprTypmod(col);
2255+
*vartypmod = coltypmods[attnum - 1];
21642256
*varcollid = list_nth_oid(rte->values_collations, attnum - 1);
2257+
pfree(coltypmods);
21652258
}
21662259
break;
21672260
case RTE_JOIN:

‎src/test/regress/expected/create_view.out

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,40 @@ SELECT relname, relkind, reloptions FROM pg_class
288288
mysecview4 | v | {security_barrier=false}
289289
(4 rows)
290290

291+
-- This test checks that proper typmods are assigned in a multi-row VALUES
292+
CREATE VIEW tt1 AS
293+
SELECT * FROM (
294+
VALUES
295+
('abc'::varchar(3), '0123456789', 42, 'abcd'::varchar(4)),
296+
('0123456789', 'abc'::varchar(3), 42.12, 'abc'::varchar(4))
297+
) vv(a,b,c,d);
298+
\d+ tt1
299+
View "testviewschm2.tt1"
300+
Column | Type | Modifiers | Storage | Description
301+
--------+----------------------+-----------+----------+-------------
302+
a | character varying | | extended |
303+
b | character varying | | extended |
304+
c | numeric | | main |
305+
d | character varying(4) | | extended |
306+
View definition:
307+
SELECT vv.a, vv.b, vv.c, vv.d
308+
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);
309+
310+
SELECT * FROM tt1;
311+
a | b | c | d
312+
------------+------------+-------+------
313+
abc | 0123456789 | 42 | abcd
314+
0123456789 | abc | 42.12 | abc
315+
(2 rows)
316+
317+
SELECT a::varchar(3) FROM tt1;
318+
a
319+
-----
320+
abc
321+
012
322+
(2 rows)
323+
324+
DROP VIEW tt1;
291325
-- check display of whole-row variables in some corner cases
292326
create type nestedcomposite as (x int8_tbl);
293327
create view tt15v as select row(i)::nestedcomposite from int8_tbl i;

‎src/test/regress/sql/create_view.sql

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,19 @@ SELECT relname, relkind, reloptions FROM pg_class
224224
'mysecview3'::regclass, 'mysecview4'::regclass)
225225
ORDER BY relname;
226226

227+
-- This test checks that proper typmods are assigned in a multi-row VALUES
228+
229+
CREATE VIEW tt1 AS
230+
SELECT * FROM (
231+
VALUES
232+
('abc'::varchar(3), '0123456789', 42, 'abcd'::varchar(4)),
233+
('0123456789', 'abc'::varchar(3), 42.12, 'abc'::varchar(4))
234+
) vv(a,b,c,d);
235+
\d+ tt1
236+
SELECT * FROM tt1;
237+
SELECT a::varchar(3) FROM tt1;
238+
DROP VIEW tt1;
239+
227240
-- check display of whole-row variables in some corner cases
228241

229242
create type nestedcomposite as (x int8_tbl);

0 commit comments

Comments
 (0)
0