Repository: impala
Updated Branches:
  refs/heads/master 96f976534 -> 2ab57438f


IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Expected results from spillable-bufe-sizing.test and max-row-size.test
included incorrect expressions: the un-anayzed GROUP BY expression with
ordinals represented as (invalid) casts.

SelectStmt is a bit of a mess. There are two copies of the grouping
expressions. Here we want to use the analyzed version with the ordinals
replaced.

Testing:

* Problem found when running PlannerTest. PlannerTest now passes,
  with correct results, after this change.
* Turns out there is another path used when generating SQL for a view
  which does toSql() on an unanalyzed query. Added unit tests for this
  case.

Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Reviewed-on: http://gerrit.cloudera.org:8080/11993
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>


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

Branch: refs/heads/master
Commit: e64261adb780518b5fd03e2702c9b2912a9cd174
Parents: 96f9765
Author: Paul Rogers <[email protected]>
Authored: Mon Nov 26 17:47:37 2018 -0800
Committer: Impala Public Jenkins <[email protected]>
Committed: Thu Nov 29 05:16:15 2018 +0000

----------------------------------------------------------------------
 .../org/apache/impala/analysis/SelectStmt.java  | 11 ++-
 .../apache/impala/analysis/ToSqlUtilsTest.java  | 70 ++++++++++++++++++++
 .../queries/PlannerTest/max-row-size.test       | 13 ++--
 .../PlannerTest/spillable-buffer-sizing.test    |  8 +--
 4 files changed, 88 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/e64261ad/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java 
b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
index f60396a..22df031 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
@@ -1028,9 +1028,14 @@ public class SelectStmt extends QueryStmt {
     // Group By clause
     if (groupingExprs_ != null) {
       strBuilder.append(" GROUP BY ");
-      for (int i = 0; i < groupingExprs_.size(); ++i) {
-        strBuilder.append(groupingExprs_.get(i).toSql(options));
-        strBuilder.append((i+1 != groupingExprs_.size()) ? ", " : "");
+      // Handle both analyzed (multiAggInfo_ != null) and unanalyzed cases.
+      // Unanalyzed case us used to generate SQL such as for views.
+      // See ToSqlUtils.getCreateViewSql().
+      List<Expr> groupingExprs = multiAggInfo_ == null
+          ? groupingExprs_ : multiAggInfo_.getGroupingExprs();
+      for (int i = 0; i < groupingExprs.size(); ++i) {
+        strBuilder.append(groupingExprs.get(i).toSql(options));
+        strBuilder.append((i+1 != groupingExprs.size()) ? ", " : "");
       }
     }
     // Having clause

http://git-wip-us.apache.org/repos/asf/impala/blob/e64261ad/fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java 
b/fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
new file mode 100644
index 0000000..d26db46
--- /dev/null
+++ b/fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
@@ -0,0 +1,70 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.impala.analysis;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.impala.catalog.FeTable;
+import org.apache.impala.catalog.FeView;
+import org.apache.impala.common.FrontendTestBase;
+import org.junit.Test;
+
+public class ToSqlUtilsTest extends FrontendTestBase {
+
+  private FeTable getTable(String dbName, String tableName) {
+    FeTable table = catalog_.getOrLoadTable(dbName, tableName);
+    assertNotNull(table);
+    return table;
+  }
+
+  private FeView getView(String dbName, String tableName) {
+    FeTable table = getTable(dbName, tableName);
+    assertTrue(table instanceof FeView);
+    return (FeView) table;
+  }
+
+  @Test
+  public void testCreateViewSql() {
+    {
+      FeView view = getView("functional", "view_view");
+      String sql = ToSqlUtils.getCreateViewSql(view);
+      String expected =
+          "CREATE VIEW functional.view_view AS\n" +
+          "SELECT * FROM functional.alltypes_view";
+      assertEquals(expected, sql);
+    }
+    {
+      FeView view = getView("functional", "complex_view");
+      String sql = ToSqlUtils.getCreateViewSql(view);
+      String expected =
+          "CREATE VIEW functional.complex_view AS\n" +
+          "SELECT complex_view.`_c0` abc, complex_view.string_col xyz" +
+          " FROM (SELECT count(a.bigint_col), b.string_col" +
+          " FROM functional.alltypesagg a" +
+          " INNER JOIN functional.alltypestiny b ON a.id = b.id" +
+          " WHERE a.bigint_col < 50" +
+          " GROUP BY b.string_col" +
+          " HAVING count(a.bigint_col) > 1" +
+          " ORDER BY b.string_col ASC LIMIT 100) complex_view";
+      assertEquals(expected, sql);
+    }
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/impala/blob/e64261ad/testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
----------------------------------------------------------------------
diff --git 
a/testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test 
b/testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
index 87fcfb4..0ffe8af 100644
--- 
a/testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
+++ 
b/testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
@@ -6,7 +6,7 @@ from tpch_parquet.customer
 ---- DISTRIBUTEDPLAN
 Max Per-Host Resource Reservation: Memory=33.97MB Threads=5
 Per-Host Resource Estimates: Memory=68MB
-Analyzed query: SELECT 
+Analyzed query: SELECT
 -- +straight_join
 * FROM tpch_parquet.customer INNER JOIN tpch_parquet.nation ON c_nationkey =
 n_nationkey
@@ -67,7 +67,7 @@ from tpch_parquet.lineitem
 ---- DISTRIBUTEDPLAN
 Max Per-Host Resource Reservation: Memory=110.00MB Threads=5
 Per-Host Resource Estimates: Memory=410MB
-Analyzed query: SELECT 
+Analyzed query: SELECT
 -- +straight_join
 * FROM tpch_parquet.lineitem LEFT OUTER JOIN tpch_parquet.orders ON l_orderkey 
=
 o_orderkey
@@ -184,11 +184,11 @@ having count(*) = 1
 ---- DISTRIBUTEDPLAN
 Max Per-Host Resource Reservation: Memory=110.00MB Threads=7
 Per-Host Resource Estimates: Memory=272MB
-Analyzed query: SELECT 
+Analyzed query: SELECT
 -- +straight_join
 l_orderkey, o_orderstatus, count(*) FROM tpch_parquet.lineitem INNER JOIN
-tpch_parquet.orders ON o_orderkey = l_orderkey GROUP BY CAST(1 AS 
INVALID_TYPE),
-CAST(2 AS INVALID_TYPE) HAVING count(*) = CAST(1 AS BIGINT)
+tpch_parquet.orders ON o_orderkey = l_orderkey GROUP BY l_orderkey,
+o_orderstatus HAVING count(*) = CAST(1 AS BIGINT)
 
 F04:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.10MB mem-reservation=0B 
thread-reservation=1
@@ -326,8 +326,7 @@ group by 1, 2
 Max Per-Host Resource Reservation: Memory=98.00MB Threads=4
 Per-Host Resource Estimates: Memory=276MB
 Analyzed query: SELECT l_orderkey, l_partkey, group_concat(l_linestatus, ',')
-FROM tpch_parquet.lineitem GROUP BY CAST(1 AS INVALID_TYPE), CAST(2 AS
-INVALID_TYPE)
+FROM tpch_parquet.lineitem GROUP BY l_orderkey, l_partkey
 
 F02:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.09MB mem-reservation=0B 
thread-reservation=1

http://git-wip-us.apache.org/repos/asf/impala/blob/e64261ad/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
----------------------------------------------------------------------
diff --git 
a/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
 
b/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
index 9da9bf1..1ddc976 100644
--- 
a/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
+++ 
b/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
@@ -744,8 +744,8 @@ Per-Host Resource Estimates: Memory=244MB
 Analyzed query: SELECT 
 -- +straight_join
 l_orderkey, o_orderstatus, count(*) FROM tpch_parquet.lineitem INNER JOIN
-tpch_parquet.orders ON o_orderkey = l_orderkey GROUP BY CAST(1 AS 
INVALID_TYPE),
-CAST(2 AS INVALID_TYPE) HAVING count(*) = CAST(1 AS BIGINT)
+tpch_parquet.orders ON o_orderkey = l_orderkey GROUP BY l_orderkey,
+o_orderstatus HAVING count(*) = CAST(1 AS BIGINT)
 
 F04:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.10MB mem-reservation=0B 
thread-reservation=1
@@ -829,8 +829,8 @@ Per-Host Resource Estimates: Memory=400MB
 Analyzed query: SELECT 
 -- +straight_join
 l_orderkey, o_orderstatus, count(*) FROM tpch_parquet.lineitem INNER JOIN
-tpch_parquet.orders ON o_orderkey = l_orderkey GROUP BY CAST(1 AS 
INVALID_TYPE),
-CAST(2 AS INVALID_TYPE) HAVING count(*) = CAST(1 AS BIGINT)
+tpch_parquet.orders ON o_orderkey = l_orderkey GROUP BY l_orderkey,
+o_orderstatus HAVING count(*) = CAST(1 AS BIGINT)
 
 F04:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=10.19MB mem-reservation=0B 
thread-reservation=1

Reply via email to