From 91e16b980612d80de1017e97e9f206239afb9026 Mon Sep 17 00:00:00 2001 From: Tom Lane 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 78b23221b7..36d9953108 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 1b95e90e02..06b203793a 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 234a4214b2..e09bc1a279 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; -- 2.40.0