Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit cf22c8c

Browse files
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 79c89f1 commit cf22c8c

File tree

3 files changed

+147
-4
lines changed

3 files changed

+147
-4
lines changed

src/backend/parser/parse_relation.c

Lines changed: 97 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ static void expandTupleDesc(TupleDesc tupdesc, Alias *eref,
5252
int rtindex, int sublevels_up,
5353
int location, bool include_dropped,
5454
List **colnames, List **colvars);
55+
static int32 *getValuesTypmods(RangeTblEntry *rte);
5556
static int specialAttNum(const char *attname);
5657
static bool isQueryUsingTempRelation_walker(Node *node, void *context);
5758

@@ -2157,9 +2158,22 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
21572158
{
21582159
/* Values RTE */
21592160
ListCell *aliasp_item = list_head(rte->eref->colnames);
2161+
int32 *coltypmods;
21602162
ListCell *lcv;
21612163
ListCell *lcc;
21622164

2165+
/*
2166+
* It's okay to extract column types from the expressions in
2167+
* the first row, since all rows will have been coerced to the
2168+
* same types. Their typmods might not be the same though, so
2169+
* we potentially need to examine all rows to compute those.
2170+
* Column collations are pre-computed in values_collations.
2171+
*/
2172+
if (colvars)
2173+
coltypmods = getValuesTypmods(rte);
2174+
else
2175+
coltypmods = NULL;
2176+
21632177
varattno = 0;
21642178
forboth(lcv, (List *) linitial(rte->values_lists),
21652179
lcc, rte->values_collations)
@@ -2184,13 +2198,15 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
21842198

21852199
varnode = makeVar(rtindex, varattno,
21862200
exprType(col),
2187-
exprTypmod(col),
2201+
coltypmods[varattno - 1],
21882202
colcollation,
21892203
sublevels_up);
21902204
varnode->location = location;
21912205
*colvars = lappend(*colvars, varnode);
21922206
}
21932207
}
2208+
if (coltypmods)
2209+
pfree(coltypmods);
21942210
}
21952211
break;
21962212
case RTE_JOIN:
@@ -2296,6 +2312,8 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
22962312
varnode = makeVar(rtindex, varattno,
22972313
coltype, coltypmod, colcoll,
22982314
sublevels_up);
2315+
varnode->location = location;
2316+
22992317
*colvars = lappend(*colvars, varnode);
23002318
}
23012319
}
@@ -2412,6 +2430,74 @@ expandTupleDesc(TupleDesc tupdesc, Alias *eref, int count, int offset,
24122430
}
24132431
}
24142432

2433+
/*
2434+
* getValuesTypmods -- expandRTE subroutine
2435+
*
2436+
* Identify per-column typmods for the given VALUES RTE. Returns a
2437+
* palloc'd array.
2438+
*/
2439+
static int32 *
2440+
getValuesTypmods(RangeTblEntry *rte)
2441+
{
2442+
int32 *coltypmods;
2443+
List *firstrow;
2444+
int ncolumns,
2445+
nvalid,
2446+
i;
2447+
ListCell *lc;
2448+
2449+
Assert(rte->values_lists != NIL);
2450+
firstrow = (List *) linitial(rte->values_lists);
2451+
ncolumns = list_length(firstrow);
2452+
coltypmods = (int32 *) palloc(ncolumns * sizeof(int32));
2453+
nvalid = 0;
2454+
2455+
/* Collect the typmods from the first VALUES row */
2456+
i = 0;
2457+
foreach(lc, firstrow)
2458+
{
2459+
Node *col = (Node *) lfirst(lc);
2460+
2461+
coltypmods[i] = exprTypmod(col);
2462+
if (coltypmods[i] >= 0)
2463+
nvalid++;
2464+
i++;
2465+
}
2466+
2467+
/*
2468+
* Scan remaining rows; as soon as we have a non-matching typmod for a
2469+
* column, reset that typmod to -1. We can bail out early if all typmods
2470+
* become -1.
2471+
*/
2472+
if (nvalid > 0)
2473+
{
2474+
for_each_cell(lc, lnext(list_head(rte->values_lists)))
2475+
{
2476+
List *thisrow = (List *) lfirst(lc);
2477+
ListCell *lc2;
2478+
2479+
Assert(list_length(thisrow) == ncolumns);
2480+
i = 0;
2481+
foreach(lc2, thisrow)
2482+
{
2483+
Node *col = (Node *) lfirst(lc2);
2484+
2485+
if (coltypmods[i] >= 0 && coltypmods[i] != exprTypmod(col))
2486+
{
2487+
coltypmods[i] = -1;
2488+
nvalid--;
2489+
}
2490+
i++;
2491+
}
2492+
2493+
if (nvalid <= 0)
2494+
break;
2495+
}
2496+
}
2497+
2498+
return coltypmods;
2499+
}
2500+
24152501
/*
24162502
* expandRelAttrs -
24172503
* Workhorse for "*" expansion: produce a list of targetentries
@@ -2656,18 +2742,25 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
26562742
break;
26572743
case RTE_VALUES:
26582744
{
2659-
/* Values RTE --- get type info from first sublist */
2660-
/* collation is stored separately, though */
2745+
/*
2746+
* Values RTE --- we can get type info from first sublist, but
2747+
* typmod may require scanning all sublists, and collation is
2748+
* stored separately. Using getValuesTypmods() is overkill,
2749+
* but this path is taken so seldom for VALUES that it's not
2750+
* worth writing extra code.
2751+
*/
26612752
List *collist = (List *) linitial(rte->values_lists);
26622753
Node *col;
2754+
int32 *coltypmods = getValuesTypmods(rte);
26632755

26642756
if (attnum < 1 || attnum > list_length(collist))
26652757
elog(ERROR, "values list %s does not have attribute %d",
26662758
rte->eref->aliasname, attnum);
26672759
col = (Node *) list_nth(collist, attnum - 1);
26682760
*vartype = exprType(col);
2669-
*vartypmod = exprTypmod(col);
2761+
*vartypmod = coltypmods[attnum - 1];
26702762
*varcollid = list_nth_oid(rte->values_collations, attnum - 1);
2763+
pfree(coltypmods);
26712764
}
26722765
break;
26732766
case RTE_JOIN:

src/test/regress/expected/create_view.out

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,43 @@ 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,
308+
vv.b,
309+
vv.c,
310+
vv.d
311+
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);
312+
313+
SELECT * FROM tt1;
314+
a | b | c | d
315+
------------+------------+-------+------
316+
abc | 0123456789 | 42 | abcd
317+
0123456789 | abc | 42.12 | abc
318+
(2 rows)
319+
320+
SELECT a::varchar(3) FROM tt1;
321+
a
322+
-----
323+
abc
324+
012
325+
(2 rows)
326+
327+
DROP VIEW tt1;
291328
-- Test view decompilation in the face of relation renaming conflicts
292329
CREATE TABLE tt1 (f1 int, f2 int, f3 text);
293330
CREATE TABLE tx1 (x1 int, x2 int, x3 text);

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
-- Test view decompilation in the face of relation renaming conflicts
228241

229242
CREATE TABLE tt1 (f1 int, f2 int, f3 text);

0 commit comments

Comments
 (0)