pgsql: Improve ruleutils.c's heuristics for dealing with rangetable ali

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Improve ruleutils.c's heuristics for dealing with rangetable ali
Date: 2012-09-21 23:03:33
Message-ID: E1TFCFx-0004m6-DV@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

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.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/11e131854f8231a21613f834c40fe9d046926387

Modified Files
--------------
src/backend/commands/explain.c | 155 ++++++++++-
src/backend/utils/adt/ruleutils.c | 331 +++++++++++++++---------
src/include/commands/explain.h | 1 +
src/include/utils/builtins.h | 4 +-
src/test/regress/expected/aggregates.out | 22 +-
src/test/regress/expected/alter_table.out | 18 +-
src/test/regress/expected/create_view.out | 371 +++++++++++++++++++++++++-
src/test/regress/expected/inherit.out | 94 ++++----
src/test/regress/expected/select_views.out | 10 +-
src/test/regress/expected/select_views_1.out | 10 +-
src/test/regress/expected/with.out | 34 ++--
src/test/regress/sql/create_view.sql | 62 +++++
12 files changed, 886 insertions(+), 226 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Andrew Dunstan 2012-09-22 17:02:41 pgsql: Allow IF NOT EXISTS when add a new enum label.
Previous Message pgsql 2012-09-21 19:13:25 pgsql: Tag refs/tags/REL9_1_6 was created