Fix incorrect printing of queries with duplicated join names.

tglsfdc · tglsfdc · commit 8ace51a6b114 · 2019-06-12T19:42:39.000-04:00
Given a query in which multiple JOIN nodes used the same alias (which'd necessarily be in different sub-SELECTs), ruleutils.c would assign the JOIN nodes distinct aliases for clarity ... but then it forgot to print the modified aliases when dumping the JOIN nodes themselves. This results in a dump/reload hazard for views, because the emitted query is flat-out incorrect: Vars will be printed with table names that have no referent. This has been wrong for a long time, so back-patch to all supported branches. Philip Dubé Discussion: https://postgr.es/m/CY4PR2101MB080246F2955FF58A6ED1FEAC98140@CY4PR2101MB0802.namprd21.prod.outlook.com
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
@@ -8889,8 +8889,16 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
 		/* Yes, it's correct to put alias after the right paren ... */
 		if (j->alias != NULL)
 		{
+			/*
+			 * Note that it's correct to emit an alias clause if and only if
+			 * there was one originally.  Otherwise we'd be converting a named
+			 * join to unnamed or vice versa, which creates semantic
+			 * subtleties we don't want.  However, we might print a different
+			 * alias name than was there originally.
+			 */
 			appendStringInfo(buf, " %s",
-							 quote_identifier(j->alias->aliasname));
+							 quote_identifier(get_rtable_name(j->rtindex,
+															  context)));
 			get_column_alias_list(colinfo, context);
 		}
 	}
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
@@ -743,6 +743,41 @@ View definition:
            FROM temp_view_test.tx1 tx1_1
           WHERE tx1.y1 = tx1_1.f1));
 
+-- Test aliasing of joins
+create view view_of_joins as
+select * from
+  (select * from (tbl1 cross join tbl2) same) ss,
+  (tbl3 cross join tbl4) same;
+\d+ view_of_joins
+          View "testviewschm2.view_of_joins"
+ Column |  Type   | Modifiers | Storage | Description 
+--------+---------+-----------+---------+-------------
+ a      | integer |           | plain   | 
+ b      | integer |           | plain   | 
+ c      | integer |           | plain   | 
+ d      | integer |           | plain   | 
+ e      | integer |           | plain   | 
+ f      | integer |           | plain   | 
+ g      | integer |           | plain   | 
+ h      | integer |           | plain   | 
+View definition:
+ SELECT ss.a,
+    ss.b,
+    ss.c,
+    ss.d,
+    same.e,
+    same.f,
+    same.g,
+    same.h
+   FROM ( SELECT same_1.a,
+            same_1.b,
+            same_1.c,
+            same_1.d
+           FROM (tbl1
+             CROSS JOIN tbl2) same_1) ss,
+    (tbl3
+     CROSS JOIN tbl4) same;
+
 -- Test view decompilation in the face of column addition/deletion/renaming
 create table tt2 (a int, b int, c int);
 create table tt3 (ax int8, b int2, c numeric);
diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
@@ -311,6 +311,15 @@ ALTER TABLE tmp1 RENAME TO tx1;
 \d+ aliased_view_3
 \d+ aliased_view_4
 
+-- Test aliasing of joins
+
+create view view_of_joins as
+select * from
+  (select * from (tbl1 cross join tbl2) same) ss,
+  (tbl3 cross join tbl4) same;
+
+\d+ view_of_joins
+
 -- Test view decompilation in the face of column addition/deletion/renaming
 
 create table tt2 (a int, b int, c int);

Original file line number	Diff line number	Diff line change
`@@ -8889,8 +8889,16 @@ get_from_clause_item(Node jtnode, Query query, deparse_context *context)`
`8889`	`8889`	`/* Yes, it's correct to put alias after the right paren ... */`
`8890`	`8890`	`if (j->alias != NULL)`
`8891`	`8891`	`{`
	`8892`	`+ /*`
	`8893`	`+ * Note that it's correct to emit an alias clause if and only if`
	`8894`	`+ * there was one originally. Otherwise we'd be converting a named`
	`8895`	`+ * join to unnamed or vice versa, which creates semantic`
	`8896`	`+ * subtleties we don't want. However, we might print a different`
	`8897`	`+ * alias name than was there originally.`
	`8898`	`+ */`
`8892`	`8899`	`appendStringInfo(buf, " %s",`
`8893`		`- quote_identifier(j->alias->aliasname));`
	`8900`	`+ quote_identifier(get_rtable_name(j->rtindex,`
	`8901`	`+ context)));`
`8894`	`8902`	`get_column_alias_list(colinfo, context);`
`8895`	`8903`	`}`
`8896`	`8904`	`}`