pgsql: Fix reporting of column typmods for multi-row VALUES constructs.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Fix reporting of column typmods for multi-row VALUES constructs.
Date: 2016-12-08 16:40:11
Message-ID: E1cF1jv-0005oo-H9@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

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

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/0b78106cd4651ad7867036a463ec743f6d3d2e86

Modified Files
--------------
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(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Stephen Frost 2016-12-08 18:49:31 Re: [COMMITTERS] pgsql: Implement table partitioning.
Previous Message Heikki Linnakangas 2016-12-08 12:13:31 pgsql: Fix quoting and a compiler warning in dumping partitions.