]> granicus.if.org Git - postgresql/commit
Improve ruleutils.c's heuristics for dealing with rangetable aliases.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 21 Sep 2012 23:03:10 +0000 (19:03 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 21 Sep 2012 23:03:10 +0000 (19:03 -0400)
commit11e131854f8231a21613f834c40fe9d046926387
tree94d012e7181c3a05b00f04a70aa588c6d8abf4af
parent7c45e3a3c682f855ecda7fd671969ee5e91929bf
Improve ruleutils.c's heuristics for dealing with rangetable aliases.

The previous scheme had bugs in some corner cases involving tables that had
been renamed since a view was made.  This could result in dumped views that
failed to reload or reloaded incorrectly, as seen in bug #7553 from Lloyd
Albin, as well as in some pgsql-hackers discussion back in January.  Also,
its behavior for printing EXPLAIN plans was sometimes confusing because of
willingness to use the same alias for multiple RTEs (it was Ashutosh
Bapat's complaint about that aspect that started the January thread).

To fix, ensure that each RTE in the query has a unique unqualified alias,
by modifying the alias if necessary (we add "_" and digits as needed to
create a non-conflicting name).  Then we can just print its variables with
that alias, avoiding the confusing and bug-prone scheme of sometimes
schema-qualifying variable names.  In EXPLAIN, it proves to be expedient to
take the further step of only assigning such aliases to RTEs that are
actually referenced in the query, since the planner has a habit of
generating extra RTEs with the same alias in situations such as
inheritance-tree expansion.

Although this fixes a bug of very long standing, I'm hesitant to back-patch
such a noticeable behavioral change.  My experiments while creating a
regression test convinced me that actually incorrect output (as opposed to
confusing output) occurs only in very narrow cases, which is backed up by
the lack of previous complaints from the field.  So we may be better off
living with it in released branches; and in any case it'd be smart to let
this ripen awhile in HEAD before we consider back-patching it.
12 files changed:
src/backend/commands/explain.c
src/backend/utils/adt/ruleutils.c
src/include/commands/explain.h
src/include/utils/builtins.h
src/test/regress/expected/aggregates.out
src/test/regress/expected/alter_table.out
src/test/regress/expected/create_view.out
src/test/regress/expected/inherit.out
src/test/regress/expected/select_views.out
src/test/regress/expected/select_views_1.out
src/test/regress/expected/with.out
src/test/regress/sql/create_view.sql