Skip to content

Commit 0b78106

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. 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
1 parent 2560d24 commit 0b78106

File tree

12 files changed

+151
-109
lines changed

12 files changed

+151
-109
lines changed

‎src/backend/nodes/copyfuncs.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2149,13 +2149,12 @@ _copyRangeTblEntry(const RangeTblEntry *from)
21492149
COPY_NODE_FIELD(functions);
21502150
COPY_SCALAR_FIELD(funcordinality);
21512151
COPY_NODE_FIELD(values_lists);
2152-
COPY_NODE_FIELD(values_collations);
21532152
COPY_STRING_FIELD(ctename);
21542153
COPY_SCALAR_FIELD(ctelevelsup);
21552154
COPY_SCALAR_FIELD(self_reference);
2156-
COPY_NODE_FIELD(ctecoltypes);
2157-
COPY_NODE_FIELD(ctecoltypmods);
2158-
COPY_NODE_FIELD(ctecolcollations);
2155+
COPY_NODE_FIELD(coltypes);
2156+
COPY_NODE_FIELD(coltypmods);
2157+
COPY_NODE_FIELD(colcollations);
21592158
COPY_NODE_FIELD(alias);
21602159
COPY_NODE_FIELD(eref);
21612160
COPY_SCALAR_FIELD(lateral);

‎src/backend/nodes/equalfuncs.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2460,13 +2460,12 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b)
24602460
COMPARE_NODE_FIELD(functions);
24612461
COMPARE_SCALAR_FIELD(funcordinality);
24622462
COMPARE_NODE_FIELD(values_lists);
2463-
COMPARE_NODE_FIELD(values_collations);
24642463
COMPARE_STRING_FIELD(ctename);
24652464
COMPARE_SCALAR_FIELD(ctelevelsup);
24662465
COMPARE_SCALAR_FIELD(self_reference);
2467-
COMPARE_NODE_FIELD(ctecoltypes);
2468-
COMPARE_NODE_FIELD(ctecoltypmods);
2469-
COMPARE_NODE_FIELD(ctecolcollations);
2466+
COMPARE_NODE_FIELD(coltypes);
2467+
COMPARE_NODE_FIELD(coltypmods);
2468+
COMPARE_NODE_FIELD(colcollations);
24702469
COMPARE_NODE_FIELD(alias);
24712470
COMPARE_NODE_FIELD(eref);
24722471
COMPARE_SCALAR_FIELD(lateral);

‎src/backend/nodes/outfuncs.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2841,15 +2841,17 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
28412841
break;
28422842
case RTE_VALUES:
28432843
WRITE_NODE_FIELD(values_lists);
2844-
WRITE_NODE_FIELD(values_collations);
2844+
WRITE_NODE_FIELD(coltypes);
2845+
WRITE_NODE_FIELD(coltypmods);
2846+
WRITE_NODE_FIELD(colcollations);
28452847
break;
28462848
case RTE_CTE:
28472849
WRITE_STRING_FIELD(ctename);
28482850
WRITE_UINT_FIELD(ctelevelsup);
28492851
WRITE_BOOL_FIELD(self_reference);
2850-
WRITE_NODE_FIELD(ctecoltypes);
2851-
WRITE_NODE_FIELD(ctecoltypmods);
2852-
WRITE_NODE_FIELD(ctecolcollations);
2852+
WRITE_NODE_FIELD(coltypes);
2853+
WRITE_NODE_FIELD(coltypmods);
2854+
WRITE_NODE_FIELD(colcollations);
28532855
break;
28542856
default:
28552857
elog(ERROR, "unrecognized RTE kind: %d", (int) node->rtekind);

‎src/backend/nodes/readfuncs.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,15 +1314,17 @@ _readRangeTblEntry(void)
13141314
break;
13151315
case RTE_VALUES:
13161316
READ_NODE_FIELD(values_lists);
1317-
READ_NODE_FIELD(values_collations);
1317+
READ_NODE_FIELD(coltypes);
1318+
READ_NODE_FIELD(coltypmods);
1319+
READ_NODE_FIELD(colcollations);
13181320
break;
13191321
case RTE_CTE:
13201322
READ_STRING_FIELD(ctename);
13211323
READ_UINT_FIELD(ctelevelsup);
13221324
READ_BOOL_FIELD(self_reference);
1323-
READ_NODE_FIELD(ctecoltypes);
1324-
READ_NODE_FIELD(ctecoltypmods);
1325-
READ_NODE_FIELD(ctecolcollations);
1325+
READ_NODE_FIELD(coltypes);
1326+
READ_NODE_FIELD(coltypmods);
1327+
READ_NODE_FIELD(colcollations);
13261328
break;
13271329
default:
13281330
elog(ERROR, "unrecognized RTE kind: %d",

‎src/backend/optimizer/plan/setrefs.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -396,10 +396,9 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte)
396396
newrte->joinaliasvars = NIL;
397397
newrte->functions = NIL;
398398
newrte->values_lists = NIL;
399-
newrte->values_collations = NIL;
400-
newrte->ctecoltypes = NIL;
401-
newrte->ctecoltypmods = NIL;
402-
newrte->ctecolcollations = NIL;
399+
newrte->coltypes = NIL;
400+
newrte->coltypmods = NIL;
401+
newrte->colcollations = NIL;
403402
newrte->securityQuals = NIL;
404403

405404
glob->finalrtable = lappend(glob->finalrtable, newrte);

‎src/backend/parser/analyze.c

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -633,10 +633,11 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
633633
* RTE.
634634
*/
635635
List *exprsLists = NIL;
636-
List *collations = NIL;
636+
List *coltypes = NIL;
637+
List *coltypmods = NIL;
638+
List *colcollations = NIL;
637639
int sublist_length = -1;
638640
bool lateral = false;
639-
int i;
640641

641642
Assert(selectStmt->intoClause == NULL);
642643

@@ -703,11 +704,20 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
703704
}
704705

705706
/*
706-
* Although we don't really need collation info, let's just make sure
707-
* we provide a correctly-sized list in the VALUES RTE.
707+
* Construct column type/typmod/collation lists for the VALUES RTE.
708+
* Every expression in each column has been coerced to the type/typmod
709+
* of the corresponding target column or subfield, so it's sufficient
710+
* to look at the exprType/exprTypmod of the first row. We don't care
711+
* about the collation labeling, so just fill in InvalidOid for that.
708712
*/
709-
for (i = 0; i < sublist_length; i++)
710-
collations = lappend_oid(collations, InvalidOid);
713+
foreach(lc, (List *) linitial(exprsLists))
714+
{
715+
Node *val = (Node *) lfirst(lc);
716+
717+
coltypes = lappend_oid(coltypes, exprType(val));
718+
coltypmods = lappend_int(coltypmods, exprTypmod(val));
719+
colcollations = lappend_oid(colcollations, InvalidOid);
720+
}
711721

712722
/*
713723
* Ordinarily there can't be any current-level Vars in the expression
@@ -722,7 +732,8 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
722732
/*
723733
* Generate the VALUES RTE
724734
*/
725-
rte = addRangeTableEntryForValues(pstate, exprsLists, collations,
735+
rte = addRangeTableEntryForValues(pstate, exprsLists,
736+
coltypes, coltypmods, colcollations,
726737
NULL, lateral, true);
727738
rtr = makeNode(RangeTblRef);
728739
/* assume new rte is at end */
@@ -1274,7 +1285,9 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
12741285
{
12751286
Query *qry = makeNode(Query);
12761287
List *exprsLists;
1277-
List *collations;
1288+
List *coltypes = NIL;
1289+
List *coltypmods = NIL;
1290+
List *colcollations = NIL;
12781291
List **colexprs = NULL;
12791292
int sublist_length = -1;
12801293
bool lateral = false;
@@ -1360,8 +1373,8 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
13601373

13611374
/*
13621375
* Now resolve the common types of the columns, and coerce everything to
1363-
* those types. Then identify the common collation, if any, of each
1364-
* column.
1376+
* those types. Then identify the common typmod and common collation, if
1377+
* any, of each column.
13651378
*
13661379
* We must do collation processing now because (1) assign_query_collations
13671380
* doesn't process rangetable entries, and (2) we need to label the VALUES
@@ -1372,11 +1385,12 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
13721385
*
13731386
* Note we modify the per-column expression lists in-place.
13741387
*/
1375-
collations = NIL;
13761388
for (i = 0; i < sublist_length; i++)
13771389
{
13781390
Oid coltype;
1391+
int32 coltypmod = -1;
13791392
Oid colcoll;
1393+
bool first = true;
13801394

13811395
coltype = select_common_type(pstate, colexprs[i], "VALUES", NULL);
13821396

@@ -1386,11 +1400,24 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
13861400

13871401
col = coerce_to_common_type(pstate, col, coltype, "VALUES");
13881402
lfirst(lc) = (void *) col;
1403+
if (first)
1404+
{
1405+
coltypmod = exprTypmod(col);
1406+
first = false;
1407+
}
1408+
else
1409+
{
1410+
/* As soon as we see a non-matching typmod, fall back to -1 */
1411+
if (coltypmod >= 0 && coltypmod != exprTypmod(col))
1412+
coltypmod = -1;
1413+
}
13891414
}
13901415

13911416
colcoll = select_common_collation(pstate, colexprs[i], true);
13921417

1393-
collations = lappend_oid(collations, colcoll);
1418+
coltypes = lappend_oid(coltypes, coltype);
1419+
coltypmods = lappend_int(coltypmods, coltypmod);
1420+
colcollations = lappend_oid(colcollations, colcoll);
13941421
}
13951422

13961423
/*
@@ -1432,7 +1459,8 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
14321459
/*
14331460
* Generate the VALUES RTE
14341461
*/
1435-
rte = addRangeTableEntryForValues(pstate, exprsLists, collations,
1462+
rte = addRangeTableEntryForValues(pstate, exprsLists,
1463+
coltypes, coltypmods, colcollations,
14361464
NULL, lateral, true);
14371465
addRTEtoQuery(pstate, rte, true, true, true);
14381466

‎src/backend/parser/parse_relation.c

Lines changed: 24 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1635,7 +1635,9 @@ addRangeTableEntryForFunction(ParseState *pstate,
16351635
RangeTblEntry *
16361636
addRangeTableEntryForValues(ParseState *pstate,
16371637
List *exprs,
1638-
List *collations,
1638+
List *coltypes,
1639+
List *coltypmods,
1640+
List *colcollations,
16391641
Alias *alias,
16401642
bool lateral,
16411643
bool inFromCl)
@@ -1652,7 +1654,9 @@ addRangeTableEntryForValues(ParseState *pstate,
16521654
rte->relid = InvalidOid;
16531655
rte->subquery = NULL;
16541656
rte->values_lists = exprs;
1655-
rte->values_collations = collations;
1657+
rte->coltypes = coltypes;
1658+
rte->coltypmods = coltypmods;
1659+
rte->colcollations = colcollations;
16561660
rte->alias = alias;
16571661

16581662
eref = alias ? copyObject(alias) : makeAlias(refname, NIL);
@@ -1822,9 +1826,9 @@ addRangeTableEntryForCTE(ParseState *pstate,
18221826
parser_errposition(pstate, rv->location)));
18231827
}
18241828

1825-
rte->ctecoltypes = cte->ctecoltypes;
1826-
rte->ctecoltypmods = cte->ctecoltypmods;
1827-
rte->ctecolcollations = cte->ctecolcollations;
1829+
rte->coltypes = cte->ctecoltypes;
1830+
rte->coltypmods = cte->ctecoltypmods;
1831+
rte->colcollations = cte->ctecolcollations;
18281832

18291833
rte->alias = alias;
18301834
if (alias)
@@ -2153,46 +2157,6 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
21532157
}
21542158
}
21552159
break;
2156-
case RTE_VALUES:
2157-
{
2158-
/* Values RTE */
2159-
ListCell *aliasp_item = list_head(rte->eref->colnames);
2160-
ListCell *lcv;
2161-
ListCell *lcc;
2162-
2163-
varattno = 0;
2164-
forboth(lcv, (List *) linitial(rte->values_lists),
2165-
lcc, rte->values_collations)
2166-
{
2167-
Node *col = (Node *) lfirst(lcv);
2168-
Oid colcollation = lfirst_oid(lcc);
2169-
2170-
varattno++;
2171-
if (colnames)
2172-
{
2173-
/* Assume there is one alias per column */
2174-
char *label = strVal(lfirst(aliasp_item));
2175-
2176-
*colnames = lappend(*colnames,
2177-
makeString(pstrdup(label)));
2178-
aliasp_item = lnext(aliasp_item);
2179-
}
2180-
2181-
if (colvars)
2182-
{
2183-
Var *varnode;
2184-
2185-
varnode = makeVar(rtindex, varattno,
2186-
exprType(col),
2187-
exprTypmod(col),
2188-
colcollation,
2189-
sublevels_up);
2190-
varnode->location = location;
2191-
*colvars = lappend(*colvars, varnode);
2192-
}
2193-
}
2194-
}
2195-
break;
21962160
case RTE_JOIN:
21972161
{
21982162
/* Join RTE */
@@ -2262,17 +2226,19 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
22622226
}
22632227
}
22642228
break;
2229+
case RTE_VALUES:
22652230
case RTE_CTE:
22662231
{
2232+
/* Values or CTE RTE */
22672233
ListCell *aliasp_item = list_head(rte->eref->colnames);
22682234
ListCell *lct;
22692235
ListCell *lcm;
22702236
ListCell *lcc;
22712237

22722238
varattno = 0;
2273-
forthree(lct, rte->ctecoltypes,
2274-
lcm, rte->ctecoltypmods,
2275-
lcc, rte->ctecolcollations)
2239+
forthree(lct, rte->coltypes,
2240+
lcm, rte->coltypmods,
2241+
lcc, rte->colcollations)
22762242
{
22772243
Oid coltype = lfirst_oid(lct);
22782244
int32 coltypmod = lfirst_int(lcm);
@@ -2285,7 +2251,8 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
22852251
/* Assume there is one alias per output column */
22862252
char *label = strVal(lfirst(aliasp_item));
22872253

2288-
*colnames = lappend(*colnames, makeString(pstrdup(label)));
2254+
*colnames = lappend(*colnames,
2255+
makeString(pstrdup(label)));
22892256
aliasp_item = lnext(aliasp_item);
22902257
}
22912258

@@ -2296,6 +2263,8 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
22962263
varnode = makeVar(rtindex, varattno,
22972264
coltype, coltypmod, colcoll,
22982265
sublevels_up);
2266+
varnode->location = location;
2267+
22992268
*colvars = lappend(*colvars, varnode);
23002269
}
23012270
}
@@ -2654,22 +2623,6 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
26542623
rte->eref->aliasname)));
26552624
}
26562625
break;
2657-
case RTE_VALUES:
2658-
{
2659-
/* Values RTE --- get type info from first sublist */
2660-
/* collation is stored separately, though */
2661-
List *collist = (List *) linitial(rte->values_lists);
2662-
Node *col;
2663-
2664-
if (attnum < 1 || attnum > list_length(collist))
2665-
elog(ERROR, "values list %s does not have attribute %d",
2666-
rte->eref->aliasname, attnum);
2667-
col = (Node *) list_nth(collist, attnum - 1);
2668-
*vartype = exprType(col);
2669-
*vartypmod = exprTypmod(col);
2670-
*varcollid = list_nth_oid(rte->values_collations, attnum - 1);
2671-
}
2672-
break;
26732626
case RTE_JOIN:
26742627
{
26752628
/*
@@ -2685,13 +2638,14 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
26852638
*varcollid = exprCollation(aliasvar);
26862639
}
26872640
break;
2641+
case RTE_VALUES:
26882642
case RTE_CTE:
26892643
{
2690-
/* CTE RTE --- get type info from lists in the RTE */
2691-
Assert(attnum > 0 && attnum <= list_length(rte->ctecoltypes));
2692-
*vartype = list_nth_oid(rte->ctecoltypes, attnum - 1);
2693-
*vartypmod = list_nth_int(rte->ctecoltypmods, attnum - 1);
2694-
*varcollid = list_nth_oid(rte->ctecolcollations, attnum - 1);
2644+
/* VALUES or CTE RTE --- get type info from lists in the RTE */
2645+
Assert(attnum > 0 && attnum <= list_length(rte->coltypes));
2646+
*vartype = list_nth_oid(rte->coltypes, attnum - 1);
2647+
*vartypmod = list_nth_int(rte->coltypmods, attnum - 1);
2648+
*varcollid = list_nth_oid(rte->colcollations, attnum - 1);
26952649
}
26962650
break;
26972651
default:

‎src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/* yyyymmddN */
56-
#define CATALOG_VERSION_NO 201612071
56+
#define CATALOG_VERSION_NO 201612081
5757

5858
#endif

0 commit comments

Comments
 (0)