From 91e16b980612d80de1017e97e9f206239afb9026 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 1 May 2014 20:22:37 -0400
Subject: [PATCH] Fix yet another corner case in dumping rules/views with USING
clauses.
ruleutils.c tries to cope with additions/deletions/renamings of columns in
tables referenced by views, by means of adding machine-generated aliases to
the printed form of a view when needed to preserve the original semantics.
A recent blog post by Marko Tiikkaja pointed out a case I'd missed though:
if one input of a join with USING is itself a join, there is nothing to
stop the user from adding a column of the same name as the USING column to
whichever side of the sub-join didn't provide the USING column. And then
there'll be an error when the view is re-parsed, since now the sub-join
exposes two columns matching the USING specification. We were catching a
lot of related cases, but not this one, so add some logic to cope with it.
Back-patch to 9.3, which is the first release that makes any serious
attempt to cope with such cases (cf commit 2ffa740be and follow-ons).
---
src/backend/utils/adt/ruleutils.c | 49 ++++++++++++++++++-----
src/test/regress/expected/create_view.out | 34 ++++++++++++++++
src/test/regress/sql/create_view.sql | 18 +++++++++
3 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 78b23221b7a..36d9953108b 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -174,11 +174,14 @@ typedef struct
* query to ensure it can be referenced unambiguously.
*
* Another problem is that a JOIN USING clause requires the columns to be
- * merged to have the same aliases in both input RTEs. To handle that, we do
- * USING-column alias assignment in a recursive traversal of the query's
- * jointree. When descending through a JOIN with USING, we preassign the
- * USING column names to the child columns, overriding other rules for column
- * alias assignment.
+ * merged to have the same aliases in both input RTEs, and that no other
+ * columns in those RTEs or their children conflict with the USING names.
+ * To handle that, we do USING-column alias assignment in a recursive
+ * traversal of the query's jointree. When descending through a JOIN with
+ * USING, we preassign the USING column names to the child columns, overriding
+ * other rules for column alias assignment. We also mark each RTE with a list
+ * of all USING column names selected for joins containing that RTE, so that
+ * when we assign other columns' aliases later, we can avoid conflicts.
*
* Another problem is that if a JOIN's input tables have had columns added or
* deleted since the query was parsed, we must generate a column alias list
@@ -229,6 +232,9 @@ typedef struct
/* This flag tells whether we should actually print a column alias list */
bool printaliases;
+ /* This list has all names used as USING names in joins above this RTE */
+ List *parentUsing; /* names assigned to parent merged columns */
+
/*
* If this struct is for a JOIN RTE, we fill these fields during the
* set_using_names() pass to describe its relationship to its child RTEs.
@@ -306,7 +312,8 @@ static void set_deparse_for_query(deparse_namespace *dpns, Query *query,
List *parent_namespaces);
static void set_simple_column_names(deparse_namespace *dpns);
static bool has_dangerous_join_using(deparse_namespace *dpns, Node *jtnode);
-static void set_using_names(deparse_namespace *dpns, Node *jtnode);
+static void set_using_names(deparse_namespace *dpns, Node *jtnode,
+ List *parentUsing);
static void set_relation_column_names(deparse_namespace *dpns,
RangeTblEntry *rte,
deparse_columns *colinfo);
@@ -2726,7 +2733,7 @@ set_deparse_for_query(deparse_namespace *dpns, Query *query,
* Select names for columns merged by USING, via a recursive pass over
* the query jointree.
*/
- set_using_names(dpns, (Node *) query->jointree);
+ set_using_names(dpns, (Node *) query->jointree, NIL);
}
/*
@@ -2860,9 +2867,12 @@ has_dangerous_join_using(deparse_namespace *dpns, Node *jtnode)
*
* Column alias info is saved in the dpns->rtable_columns list, which is
* assumed to be filled with pre-zeroed deparse_columns structs.
+ *
+ * parentUsing is a list of all USING aliases assigned in parent joins of
+ * the current jointree node. (The passed-in list must not be modified.)
*/
static void
-set_using_names(deparse_namespace *dpns, Node *jtnode)
+set_using_names(deparse_namespace *dpns, Node *jtnode, List *parentUsing)
{
if (IsA(jtnode, RangeTblRef))
{
@@ -2874,7 +2884,7 @@ set_using_names(deparse_namespace *dpns, Node *jtnode)
ListCell *lc;
foreach(lc, f->fromlist)
- set_using_names(dpns, (Node *) lfirst(lc));
+ set_using_names(dpns, (Node *) lfirst(lc), parentUsing);
}
else if (IsA(jtnode, JoinExpr))
{
@@ -2954,6 +2964,9 @@ set_using_names(deparse_namespace *dpns, Node *jtnode)
*/
if (j->usingClause)
{
+ /* Copy the input parentUsing list so we don't modify it */
+ parentUsing = list_copy(parentUsing);
+
/* USING names must correspond to the first join output columns */
expand_colnames_array_to(colinfo, list_length(j->usingClause));
i = 0;
@@ -2983,6 +2996,7 @@ set_using_names(deparse_namespace *dpns, Node *jtnode)
/* Remember selected names for use later */
colinfo->usingNames = lappend(colinfo->usingNames, colname);
+ parentUsing = lappend(parentUsing, colname);
/* Push down to left column, unless it's a system column */
if (leftattnos[i] > 0)
@@ -3002,9 +3016,13 @@ set_using_names(deparse_namespace *dpns, Node *jtnode)
}
}
+ /* Mark child deparse_columns structs with correct parentUsing info */
+ leftcolinfo->parentUsing = parentUsing;
+ rightcolinfo->parentUsing = parentUsing;
+
/* Now recursively assign USING column names in children */
- set_using_names(dpns, j->larg);
- set_using_names(dpns, j->rarg);
+ set_using_names(dpns, j->larg, parentUsing);
+ set_using_names(dpns, j->rarg, parentUsing);
}
else
elog(ERROR, "unrecognized node type: %d",
@@ -3471,6 +3489,15 @@ colname_is_unique(char *colname, deparse_namespace *dpns,
return false;
}
+ /* Also check against names already assigned for parent-join USING cols */
+ foreach(lc, colinfo->parentUsing)
+ {
+ char *oldname = (char *) lfirst(lc);
+
+ if (strcmp(oldname, colname) == 0)
+ return false;
+ }
+
return true;
}
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 1b95e90e020..06b203793a3 100644
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -1298,6 +1298,40 @@ select pg_get_viewdef('vv5', true);
JOIN tt10 USING (x);
(1 row)
+--
+-- Another corner case is that we might add a column to a table below a
+-- JOIN USING, and thereby make the USING column name ambiguous
+--
+create table tt11 (x int, y int);
+create table tt12 (x int, z int);
+create table tt13 (z int, q int);
+create view vv6 as select x,y,z,q from
+ (tt11 join tt12 using(x)) join tt13 using(z);
+select pg_get_viewdef('vv6', true);
+ pg_get_viewdef
+---------------------------
+ SELECT tt11.x, +
+ tt11.y, +
+ tt12.z, +
+ tt13.q +
+ FROM tt11 +
+ JOIN tt12 USING (x) +
+ JOIN tt13 USING (z);
+(1 row)
+
+alter table tt11 add column z int;
+select pg_get_viewdef('vv6', true);
+ pg_get_viewdef
+------------------------------
+ SELECT tt11.x, +
+ tt11.y, +
+ tt12.z, +
+ tt13.q +
+ FROM tt11 tt11(x, y, z_1)+
+ JOIN tt12 USING (x) +
+ JOIN tt13 USING (z);
+(1 row)
+
-- clean up all the random objects we made above
set client_min_messages = warning;
DROP SCHEMA temp_view_test CASCADE;
diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index 234a4214b27..e09bc1a2799 100644
--- a/src/test/regress/sql/create_view.sql
+++ b/src/test/regress/sql/create_view.sql
@@ -417,6 +417,24 @@ alter table tt9 drop column xx;
select pg_get_viewdef('vv5', true);
+--
+-- Another corner case is that we might add a column to a table below a
+-- JOIN USING, and thereby make the USING column name ambiguous
+--
+
+create table tt11 (x int, y int);
+create table tt12 (x int, z int);
+create table tt13 (z int, q int);
+
+create view vv6 as select x,y,z,q from
+ (tt11 join tt12 using(x)) join tt13 using(z);
+
+select pg_get_viewdef('vv6', true);
+
+alter table tt11 add column z int;
+
+select pg_get_viewdef('vv6', true);
+
-- clean up all the random objects we made above
set client_min_messages = warning;
DROP SCHEMA temp_view_test CASCADE;
--
GitLab