You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/07/11 05:54:01 UTC

[3/4] impala git commit: IMPALA-7216: Fix SQL generated by CreateViewStmt & AlterViewStmt

IMPALA-7216: Fix SQL generated by CreateViewStmt & AlterViewStmt

The toSql functions in CreateViewStmt and AlterViewStmt generated
invalid SQL by appending types to column definitions. This change
appends just the column names to fix it.

Testing: Added tests to ToSqlTest to verify it.

Change-Id: Ie2a834d4d8d11ba18262d2828f4829c80ed05190
Reviewed-on: http://gerrit.cloudera.org:8080/10886
Reviewed-by: Bikramjeet Vig <bi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/c465150b
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/c465150b
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/c465150b

Branch: refs/heads/master
Commit: c465150ba94a5d43e949344b4f4444124a8738a3
Parents: f164366
Author: poojanilangekar <po...@cloudera.com>
Authored: Thu Jul 5 17:47:44 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Jul 11 02:41:16 2018 +0000

----------------------------------------------------------------------
 .../apache/impala/analysis/AlterViewStmt.java   |  3 +-
 .../analysis/CreateOrAlterViewStmtBase.java     | 14 +++++
 .../apache/impala/analysis/CreateViewStmt.java  |  3 +-
 .../org/apache/impala/analysis/ToSqlTest.java   | 65 ++++++++++++++++++++
 4 files changed, 81 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/c465150b/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java b/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
index cad0169..910dd98 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
@@ -28,7 +28,6 @@ import org.apache.impala.thrift.TAccessEvent;
 import org.apache.impala.thrift.TCatalogObjectType;
 import org.apache.impala.service.BackendConfig;
 
-import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 
 /**
@@ -73,7 +72,7 @@ public class AlterViewStmt extends CreateOrAlterViewStmtBase {
       sb.append(tableName_.getDb() + ".");
     }
     sb.append(tableName_.getTbl());
-    if (columnDefs_ != null) sb.append("(" + Joiner.on(", ").join(columnDefs_) + ")");
+    if (columnDefs_ != null) sb.append("(" + getColumnNames() + ")");
     sb.append(" AS " + viewDefStmt_.toSql());
     return sb.toString();
   }

http://git-wip-us.apache.org/repos/asf/impala/blob/c465150b/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java b/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
index 6cdec98..5bb372d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
@@ -30,6 +30,7 @@ import org.slf4j.LoggerFactory;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
+import com.google.common.base.Joiner;
 
 /**
  * Base class for CREATE VIEW and ALTER VIEW AS SELECT statements.
@@ -206,6 +207,19 @@ public abstract class CreateOrAlterViewStmtBase extends StatementBase {
     return owner_;
   }
 
+  /**
+   * Returns the column names in columnDefs_. Should only be called for non-null
+   * columnDefs_.
+   */
+  protected String getColumnNames() {
+    Preconditions.checkNotNull(columnDefs_);
+    List<String> columnNames = Lists.newArrayList();
+    for (ColumnDef colDef : columnDefs_) {
+      columnNames.add(colDef.getColName());
+    }
+    return Joiner.on(", ").join(columnNames);
+  }
+
   public boolean getIfNotExists() { return ifNotExists_; }
   public String getInlineViewDef() { return inlineViewDef_; }
   public String getTbl() { return tableName_.getTbl(); }

http://git-wip-us.apache.org/repos/asf/impala/blob/c465150b/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
index 1a17aa8..89b6278 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
@@ -26,7 +26,6 @@ import org.apache.impala.thrift.TAccessEvent;
 import org.apache.impala.thrift.TCatalogObjectType;
 import org.apache.impala.service.BackendConfig;
 
-import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 
 /**
@@ -72,7 +71,7 @@ public class CreateViewStmt extends CreateOrAlterViewStmtBase {
     if (ifNotExists_) sb.append("IF NOT EXISTS ");
     if (tableName_.getDb() != null) sb.append(tableName_.getDb() + ".");
     sb.append(tableName_.getTbl());
-    if (columnDefs_ != null) sb.append("(" + Joiner.on(", ").join(columnDefs_) + ")");
+    if (columnDefs_ != null) sb.append("(" + getColumnNames() + ")");
     sb.append(" AS ");
     sb.append(viewDefStmt_.toSql());
     return sb.toString();

http://git-wip-us.apache.org/repos/asf/impala/blob/c465150b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
index 0a073eb..7209364 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
@@ -363,6 +363,71 @@ public class ToSqlTest extends FrontendTestBase {
   }
 
   @Test
+  public void TestCreateView() throws AnalysisException {
+    testToSql(
+      "create view foo_new as select int_col, string_col from functional.alltypes",
+      "default",
+      "CREATE VIEW foo_new AS SELECT int_col, string_col FROM functional.alltypes");
+    testToSql("create view if not exists foo as select * from functional.alltypes",
+        "default", "CREATE VIEW IF NOT EXISTS foo AS SELECT * FROM functional.alltypes");
+    testToSql("create view functional.foo (a, b) as select int_col x, double_col y " +
+        "from functional.alltypes", "default",
+        "CREATE VIEW functional.foo(a, b) AS SELECT int_col x, double_col y " +
+        "FROM functional.alltypes");
+    testToSql("create view foo (aaa, bbb) as select * from functional.complex_view",
+        "default", "CREATE VIEW foo(aaa, bbb) AS SELECT * FROM functional.complex_view");
+    testToSql("create view foo as select trim('abc'), 17 * 7", "default",
+        "CREATE VIEW foo AS SELECT trim('abc'), 17 * 7");
+    testToSql("create view foo (cnt) as " +
+        "select count(distinct x.int_col) from functional.alltypessmall x " +
+        "inner join functional.alltypessmall y on (x.id = y.id) group by x.bigint_col",
+        "default", "CREATE VIEW foo(cnt) AS "+
+        "SELECT count(DISTINCT x.int_col) FROM functional.alltypessmall x " +
+        "INNER JOIN functional.alltypessmall y ON (x.id = y.id) GROUP BY x.bigint_col");
+    testToSql("create view foo (a, b) as values(1, 'a'), (2, 'b')", "default",
+        "CREATE VIEW foo(a, b) AS VALUES((1, 'a'), (2, 'b'))");
+    testToSql("create view foo (a, b) as select 1, 'a' union all select 2, 'b'",
+      "default", "CREATE VIEW foo(a, b) AS SELECT 1, 'a' UNION ALL SELECT 2, 'b'");
+    testToSql("create view test_view_with_subquery as " +
+        "select * from functional.alltypestiny t where exists " +
+        "(select * from functional.alltypessmall s where s.id = t.id)", "default",
+        "CREATE VIEW test_view_with_subquery AS " +
+        "SELECT * FROM functional.alltypestiny t WHERE EXISTS " +
+        "(SELECT * FROM functional.alltypessmall s WHERE s.id = t.id)");
+  }
+
+  @Test
+  public void TestAlterView() throws AnalysisException{
+    testToSql("alter view functional.alltypes_view as " +
+        "select * from functional.alltypesagg", "default",
+        "ALTER VIEW functional.alltypes_view AS SELECT * FROM functional.alltypesagg");
+    testToSql("alter view functional.alltypes_view (a, b) as " +
+        "select int_col, string_col from functional.alltypes", "default",
+        "ALTER VIEW functional.alltypes_view(a, b) AS " +
+        "SELECT int_col, string_col FROM functional.alltypes");
+    testToSql("alter view functional.alltypes_view (a, b) as " +
+        "select int_col x, string_col y from functional.alltypes", "default",
+        "ALTER VIEW functional.alltypes_view(a, b) AS " +
+        "SELECT int_col x, string_col y FROM functional.alltypes");
+    testToSql("alter view functional.alltypes_view as select trim('abc'), 17 * 7",
+        "default", "ALTER VIEW functional.alltypes_view AS SELECT trim('abc'), 17 * 7");
+    testToSql("alter view functional.alltypes_view (aaa, bbb) as " +
+        "select * from functional.complex_view", "default",
+        "ALTER VIEW functional.alltypes_view(aaa, bbb) AS " +
+        "SELECT * FROM functional.complex_view");
+    testToSql("alter view functional.complex_view (abc, xyz) as " +
+        "select year, month from functional.alltypes_view", "default",
+        "ALTER VIEW functional.complex_view(abc, xyz) AS " +
+        "SELECT year, month FROM functional.alltypes_view");
+    testToSql("alter view functional.alltypes_view (cnt) as " +
+        "select count(distinct x.int_col) from functional.alltypessmall x " +
+        "inner join functional.alltypessmall y on (x.id = y.id) group by x.bigint_col",
+        "default", "ALTER VIEW functional.alltypes_view(cnt) AS "+
+        "SELECT count(DISTINCT x.int_col) FROM functional.alltypessmall x " +
+        "INNER JOIN functional.alltypessmall y ON (x.id = y.id) GROUP BY x.bigint_col");
+  }
+
+  @Test
   public void TestTableAliases() throws AnalysisException {
     String[] tables = new String[] { "alltypes", "alltypes_view" };
     String[] columns = new String[] { "int_col", "*" };