pgsql: Partial fix for dropped columns in functions returning composite - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Partial fix for dropped columns in functions returning composite
Date
Msg-id E1X8ZOI-0007cD-8g@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Partial fix for dropped columns in functions returning composite.

When a view has a function-returning-composite in FROM, and there are
some dropped columns in the underlying composite type, ruleutils.c
printed junk in the column alias list for the reconstructed FROM entry.
Before 9.3, this was prevented by doing get_rte_attribute_is_dropped
tests while printing the column alias list; but that solution is not
currently available to us for reasons I'll explain below.  Instead,
check for empty-string entries in the alias list, which can only exist
if that column position had been dropped at the time the view was made.
(The parser fills in empty strings to preserve the invariant that the
aliases correspond to physical column positions.)

While this is sufficient to handle the case of columns dropped before
the view was made, we have still got issues with columns dropped after
the view was made.  In particular, the view could contain Vars that
explicitly reference such columns!  The dependency machinery really
ought to refuse the column drop attempt in such cases, as it would do
when trying to drop a table column that's explicitly referenced in
views.  However, we currently neglect to store dependencies on columns
of composite types, and fixing that is likely to be too big to be
back-patchable (not to mention that existing views in existing databases
would not have the needed pg_depend entries anyway).  So I'll leave that
for a separate patch.

Pre-9.3, ruleutils would print such Vars normally (with their original
column names) even though it suppressed their entries in the RTE's
column alias list.  This is certainly bogus, since the printed view
definition would fail to reload, but at least it didn't crash.  However,
as of 9.3 the printed column alias list is tightly tied to the names
printed for Vars; so we can't treat columns as dropped for one purpose
and not dropped for the other.  This is why we can't just put back the
get_rte_attribute_is_dropped test: it results in an assertion failure
if the view in fact contains any Vars referencing the dropped column.
Once we've got dependencies preventing such cases, we'll probably want
to do it that way instead of relying on the empty-string test used here.

This fix turned up a very ancient bug in outfuncs/readfuncs, namely
that T_String nodes containing empty strings were not dumped/reloaded
correctly: the node was printed as "<>" which is read as a string
value of <>.  Since (per SQL) we disallow empty-string identifiers,
such nodes don't occur normally, which is why we'd not noticed.
(Such nodes aren't used for literal constants, just identifiers.)

Per report from Marc Schablewski.  Back-patch to 9.3 which is where
the rule printing behavior changed.  The dangling-variable case is
broken all the way back, but that's not what his complaint is about.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/9b35ddce93a2ef336498baa15581b9d10f01db9c

Modified Files
--------------
src/backend/nodes/outfuncs.c              |    8 ++++-
src/backend/utils/adt/ruleutils.c         |   11 +++++-
src/test/regress/expected/create_view.out |   52 +++++++++++++++++++++++++++++
src/test/regress/sql/create_view.sql      |   34 +++++++++++++++++++
4 files changed, 103 insertions(+), 2 deletions(-)


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: pgsql: Partial fix for dropped columns in functions returning composite
Next
From: Tom Lane
Date:
Subject: pgsql: Update time zone data files to tzdata release 2014e.