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/01/25 21:46:40 UTC

[1/6] impala git commit: IMPALA-5191: Standardize column alias behavior

Repository: impala
Updated Branches:
  refs/heads/master 92fd7e56e -> 0fdd81682


IMPALA-5191: Standardize column alias behavior

We should not perform alias substitution in the
subexpressions of GROUP BY, HAVING, and ORDER BY
to be more standard conformant.

=== Allowed ===
SELECT int_col / 2 AS x
FROM functional.alltypes
GROUP BY x;

SELECT int_col / 2 AS x
FROM functional.alltypes
ORDER BY x;

SELECT NOT bool_col AS nb
FROM functional.alltypes
GROUP BY nb
HAVING nb;

=== Not allowed ===
SELECT int_col / 2 AS x
FROM functional.alltypes
GROUP BY x / 2;

SELECT int_col / 2 AS x
FROM functional.alltypes
ORDER BY -x;

SELECT int_col / 2 AS x
FROM functional.alltypes
GROUP BY x
HAVING x > 3;

Some extra checks were added to AnalyzeExprsTest.java.

I had to update other tests to make them pass
since the new behavior is more restrictive.

I added alias.test to the end-to-end tests.

Cherry-picks: not for 2.x.

Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Reviewed-on: http://gerrit.cloudera.org:8080/8801
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: 545e60f8329ddd98b5adaff5981b52bbdcf4d69d
Parents: 92fd7e5
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
Authored: Mon Dec 11 18:11:55 2017 +0100
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Wed Jan 24 22:47:18 2018 +0000

----------------------------------------------------------------------
 .../org/apache/impala/analysis/QueryStmt.java   | 59 ++++++-------
 .../org/apache/impala/analysis/SelectStmt.java  | 38 +++++++--
 .../impala/analysis/AnalyzeExprsTest.java       |  6 --
 .../impala/analysis/AnalyzeStmtsTest.java       | 88 +++++++++++++++++++-
 .../queries/PlannerTest/constant-folding.test   |  6 +-
 .../PlannerTest/predicate-propagation.test      |  2 +-
 .../queries/QueryTest/alias.test                | 87 +++++++++++++++++++
 .../queries/QueryTest/decimal-exprs.test        |  4 +-
 .../queries/QueryTest/exprs.test                |  7 +-
 .../queries/QueryTest/nested-types-tpch.test    |  2 +-
 .../primitive_groupby_bigint_highndv.test       |  2 +-
 .../primitive_groupby_bigint_lowndv.test        |  2 +-
 .../queries/primitive_groupby_bigint_pk.test    |  2 +-
 .../primitive_groupby_decimal_highndv.test      |  2 +-
 .../primitive_groupby_decimal_lowndv.test       |  2 +-
 tests/query_test/test_queries.py                |  3 +
 16 files changed, 252 insertions(+), 60 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java b/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
index 6f21a33..178e80c 100644
--- a/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
@@ -197,7 +197,7 @@ public abstract class QueryStmt extends StatementBase {
       isAscOrder.add(Boolean.valueOf(orderByElement.isAsc()));
       nullsFirstParams.add(orderByElement.getNullsFirstParam());
     }
-    substituteOrdinalsAliases(orderingExprs, "ORDER BY", analyzer);
+    substituteOrdinalsAndAliases(orderingExprs, "ORDER BY", analyzer);
 
     if (!analyzer.isRootAnalyzer() && hasOffset() && !hasLimit()) {
       throw new AnalysisException("Order-by with offset without limit not supported" +
@@ -268,41 +268,42 @@ public abstract class QueryStmt extends StatementBase {
   }
 
   /**
-   * Return the first expr in exprs that is a non-unique alias. Return null if none of
-   * exprs is an ambiguous alias.
+   * Substitutes an ordinal or an alias. An ordinal is an integer NumericLiteral
+   * that refers to a select-list expression by ordinal. An alias is a SlotRef
+   * that matches the alias of a select-list expression (tracked by 'aliasMap_').
+   * We should substitute by ordinal or alias but not both to avoid an incorrect
+   * double substitution.
+   * Returns clone() of 'expr' if it is not an ordinal, nor an alias.
+   * The returned expr is analyzed regardless of whether substitution was performed.
    */
-  protected Expr getFirstAmbiguousAlias(List<Expr> exprs) {
-    for (Expr exp: exprs) {
-      if (ambiguousAliasList_.contains(exp)) return exp;
+  protected Expr substituteOrdinalOrAlias(Expr expr, String errorPrefix,
+      Analyzer analyzer) throws AnalysisException {
+    Expr substituteExpr = trySubstituteOrdinal(expr, errorPrefix, analyzer);
+    if (substituteExpr != null) return substituteExpr;
+    if (ambiguousAliasList_.contains(expr)) {
+      throw new AnalysisException("Column '" + expr.toSql() +
+          "' in " + errorPrefix + " clause is ambiguous");
+    }
+    if (expr instanceof SlotRef) {
+      substituteExpr = expr.trySubstitute(aliasSmap_, analyzer, false);
+    } else {
+      expr.analyze(analyzer);
+      substituteExpr = expr;
     }
-    return null;
+    return substituteExpr;
   }
 
   /**
-   * Substitute exprs of the form "<number>"  with the corresponding
-   * expressions and any alias references in aliasSmap_.
-   * Modifies exprs list in-place.
+   * Substitutes top-level ordinals and aliases. Does not substitute ordinals and
+   * aliases in subexpressions.
+   * Modifies the 'exprs' list in-place.
+   * The 'exprs' are all analyzed after this function regardless of whether
+   * substitution was performed.
    */
-  protected void substituteOrdinalsAliases(List<Expr> exprs, String errorPrefix,
+  protected void substituteOrdinalsAndAliases(List<Expr> exprs, String errorPrefix,
       Analyzer analyzer) throws AnalysisException {
-    Expr ambiguousAlias = getFirstAmbiguousAlias(exprs);
-    if (ambiguousAlias != null) {
-      throw new AnalysisException("Column '" + ambiguousAlias.toSql() +
-          "' in " + errorPrefix + " clause is ambiguous");
-    }
-
-    ListIterator<Expr> i = exprs.listIterator();
-    while (i.hasNext()) {
-      Expr expr = i.next();
-      // We can substitute either by ordinal or by alias.
-      // If we substitute by ordinal, we should not replace any aliases, since
-      // the new expression was copied from the select clause context, where
-      // alias substitution is not performed in the same way.
-      Expr substituteExpr = trySubstituteOrdinal(expr, errorPrefix, analyzer);
-      if (substituteExpr == null) {
-        substituteExpr = expr.trySubstitute(aliasSmap_, analyzer, false);
-      }
-      i.set(substituteExpr);
+    for (int i = 0; i < exprs.size(); ++i) {
+      exprs.set(i, substituteOrdinalOrAlias(exprs.get(i), errorPrefix, analyzer));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/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 2ba5105..f9d08b7 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
@@ -541,13 +541,12 @@ public class SelectStmt extends QueryStmt {
     // Analyze the HAVING clause first so we can check if it contains aggregates.
     // We need to analyze/register it even if we are not computing aggregates.
     if (havingClause_ != null) {
-      if (havingClause_.contains(Predicates.instanceOf(Subquery.class))) {
+      havingPred_ = substituteOrdinalOrAlias(havingClause_, "HAVING", analyzer);
+      // can't contain subqueries
+      if (havingPred_.contains(Predicates.instanceOf(Subquery.class))) {
         throw new AnalysisException(
             "Subqueries are not supported in the HAVING clause.");
       }
-      // substitute aliases in place (ordinals not allowed in having clause)
-      havingPred_ = havingClause_.substitute(aliasSmap_, analyzer, false);
-      havingPred_.checkReturnsBool("HAVING clause", true);
       // can't contain analytic exprs
       Expr analyticExpr = havingPred_.findFirstOf(AnalyticExpr.class);
       if (analyticExpr != null) {
@@ -555,6 +554,7 @@ public class SelectStmt extends QueryStmt {
             "HAVING clause must not contain analytic expressions: "
                + analyticExpr.toSql());
       }
+      havingPred_.checkReturnsBool("HAVING clause", true);
     }
 
     if (groupingExprs_ == null && !selectList_.isDistinct()
@@ -615,7 +615,7 @@ public class SelectStmt extends QueryStmt {
       // exprs during analysis (in case we need to print them later)
       groupingExprsCopy = Expr.cloneList(groupingExprs_);
 
-      substituteOrdinalsAliases(groupingExprsCopy, "GROUP BY", analyzer);
+      substituteOrdinalsAndAliases(groupingExprsCopy, "GROUP BY", analyzer);
 
       for (int i = 0; i < groupingExprsCopy.size(); ++i) {
         groupingExprsCopy.get(i).analyze(analyzer);
@@ -887,6 +887,23 @@ public class SelectStmt extends QueryStmt {
     }
   }
 
+  /**
+   * If given expr is rewritten into an integer literal, then return the original expr,
+   * otherwise return the rewritten expr.
+   * Used for GROUP BY, ORDER BY, and HAVING where we don't want to create an ordinal
+   * from a constant arithmetic expr, e.g. 1 * 2 =/=> 2
+   */
+  private Expr rewriteCheckOrdinalResult(ExprRewriter rewriter, Expr expr)
+      throws AnalysisException {
+    Expr rewrittenExpr = rewriter.rewrite(expr, analyzer_);
+    if (rewrittenExpr.isLiteral() && rewrittenExpr.getType().isIntegerType()) {
+      return expr;
+    }
+    else {
+      return rewrittenExpr;
+    }
+  }
+
   @Override
   public void rewriteExprs(ExprRewriter rewriter) throws AnalysisException {
     Preconditions.checkState(isAnalyzed());
@@ -900,12 +917,17 @@ public class SelectStmt extends QueryStmt {
       for (Subquery s: subqueryExprs) s.getStatement().rewriteExprs(rewriter);
     }
     if (havingClause_ != null) {
-      havingClause_ = rewriter.rewrite(havingClause_, analyzer_);
+      havingClause_ = rewriteCheckOrdinalResult(rewriter, havingClause_);
+    }
+    if (groupingExprs_ != null) {
+      for (int i = 0; i < groupingExprs_.size(); ++i) {
+        groupingExprs_.set(i, rewriteCheckOrdinalResult(
+            rewriter, groupingExprs_.get(i)));
+      }
     }
-    if (groupingExprs_ != null) rewriter.rewriteList(groupingExprs_, analyzer_);
     if (orderByElements_ != null) {
       for (OrderByElement orderByElem: orderByElements_) {
-        orderByElem.setExpr(rewriter.rewrite(orderByElem.getExpr(), analyzer_));
+        orderByElem.setExpr(rewriteCheckOrdinalResult(rewriter, orderByElem.getExpr()));
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
index 93a4090..91d1c00 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
@@ -951,11 +951,6 @@ public class AnalyzeExprsTest extends AnalyzerTest {
         "select id, tinyint_col, sum(distinct tinyint_col) over(order by id) "
           + "from functional.alltypes",
         "DISTINCT not allowed in analytic function");
-    // select list alias needs to be ignored
-    AnalysisError(
-        "select min(id) over (order by tinyint_col) as X from functional.alltypes "
-          + "group by id, tinyint_col order by rank() over (order by X)",
-        "Nesting of analytic expressions is not allowed");
     // IGNORE NULLS may only be used with first_value/last_value
     AnalysisError(
         "select sum(id ignore nulls) over (order by id) from functional.alltypes",
@@ -1005,7 +1000,6 @@ public class AnalyzeExprsTest extends AnalyzerTest {
           + "order by rank() over (order by tinyint_col), int_col) "
           + "from functional.alltypes",
         "Nesting of analytic expressions is not allowed");
-
     // lead/lag variants
     AnalyzesOk(
         "select lag(int_col, 10, 5 + 1) over (partition by id, bool_col "

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
index f9d5cf5..1d8d1be 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
@@ -974,7 +974,7 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
   }
 
   @Test
-  public void TestOrdinals() throws AnalysisException {
+  public void TestAliasesAndOrdinals() throws AnalysisException {
     AnalysisError("select * from functional.alltypes group by 1",
         "cannot combine '*' in select list with grouping or aggregation");
     AnalysisError("select * from functional.alltypes order by 14",
@@ -985,6 +985,89 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
     AnalyzesOk("select * from (select max(id) from functional.testtbl) t1 order by 1");
     AnalysisError("select * from (select max(id) from functional.testtbl) t1 order by 2",
         "ORDER BY: ordinal exceeds number of items in select list: 2");
+
+    // IMPALA-5191: In GROUP BY, HAVING, ORDER BY, aliases and ordinals must only be
+    // substituted at the top level
+    // Ambiguous alias in HAVING
+    AnalysisError("select not bool_col m, min(smallint_col) m, max(bigint_col) m "
+        + "from functional.alltypes group by bool_col having m",
+        "Column 'm' in HAVING clause is ambiguous");
+    // GROUP BY, ORDER BY, HAVING contains top-level SlotRef or ordinal
+    AnalyzesOk("select int_col / 2 as x from functional.alltypes group by x");
+    AnalyzesOk("select int_col / 2 as x from functional.alltypes order by x");
+    AnalyzesOk("select not bool_col as nb from functional.alltypes having nb");
+    AnalyzesOk("select int_col / 2 as x from functional.alltypes group by 1");
+    AnalyzesOk("select int_col / 2 as x from functional.alltypes order by 1");
+    AnalyzesOk("select not bool_col as nb from functional.alltypes having 1");
+    // GROUP BY, ORDER BY, HAVING contains alias in subexpression
+    AnalysisError(
+        "select int_col / 2 as x from functional.alltypes group by x / 2",
+        "Could not resolve column/field reference: 'x'");
+    AnalysisError(
+        "select int_col / 2 as x from functional.alltypes order by -x",
+        "Could not resolve column/field reference: 'x'");
+    AnalysisError(
+        "select int_col / 2 as x from functional.alltypes having x > 0",
+        "Could not resolve column/field reference: 'x'");
+    // Alias or ordinal referring to aggregation output in GROUP BY, ORDER BY, HAVING
+    AnalysisError("select count(*) a from functional.alltypes group by a",
+        "GROUP BY expression must not contain aggregate functions: a");
+    AnalyzesOk("select count(*) a from functional.alltypes order by a");
+    AnalysisError("select count(*) a from functional.alltypes having a",
+        "HAVING clause 'count(*)' requires return type 'BOOLEAN'. " +
+        "Actual type is 'BIGINT'.");
+    AnalysisError("select count(*) from functional.alltypes group by 1",
+        "GROUP BY expression must not contain aggregate functions: 1");
+    AnalyzesOk("select count(*) from functional.alltypes order by 1");
+    AnalysisError("select count(*) from functional.alltypes having 1",
+        "HAVING clause 'count(*)' requires return type 'BOOLEAN'. " +
+        "Actual type is 'BIGINT'.");
+    // Alias or ordinal referring to predicate in GROUP BY, ORDER BY, HAVING
+    AnalysisError("select count(*) > 10 a from functional.alltypes group by a",
+        "GROUP BY expression must not contain aggregate functions: a");
+    AnalyzesOk("select count(*) > 10 a from functional.alltypes order by a");
+    AnalyzesOk("select count(*) > 10 a from functional.alltypes having a");
+    AnalysisError("select count(*) > 10 from functional.alltypes group by 1",
+        "GROUP BY expression must not contain aggregate functions: 1");
+    AnalyzesOk("select count(*) > 10 from functional.alltypes order by 1");
+    AnalyzesOk("select count(*) > 10 from functional.alltypes having 1");
+    // Alias or ordinal referring to analytic output in GROUP BY, ORDER BY, HAVING
+    AnalysisError("select sum(id) over(order by id) a " +
+        "from functional.alltypes group by a",
+        "GROUP BY expression must not contain analytic expressions: " +
+        "sum(id) OVER (ORDER BY id ASC)");
+    AnalyzesOk("select sum(id) over(order by id) a from functional.alltypes order by a");
+    AnalysisError("select sum(id) over(order by id) a from functional.alltypes having a",
+        "HAVING clause must not contain analytic expressions: " +
+        "sum(id) OVER (ORDER BY id ASC)");
+    AnalysisError("select sum(id) over(order by id) " +
+        "from functional.alltypes group by 1",
+        "GROUP BY expression must not contain analytic expressions: " +
+        "sum(id) OVER (ORDER BY id ASC)");
+    AnalyzesOk("select sum(id) over(order by id) from functional.alltypes order by 1");
+    AnalysisError("select sum(id) over(order by id) from functional.alltypes having 1",
+        "HAVING clause must not contain analytic expressions: " +
+        "sum(id) OVER (ORDER BY id ASC)");
+    AnalyzesOk("with w_test as (select '1' as one, 2 as two, '3' as three) " +
+        "select one as one, substring(cast(two as string), 1, 1) as two, " +
+        "three as three, count(1) as cnt " +
+        "from w_test " +
+        "group by one, substring(cast(two as string), 1, 1), three");
+    // Constant exprs should not be interpreted as ordinals
+    AnalyzesOk("select int_col, count(*) from functional.alltypes group by 1, 1 * 2");
+    AnalyzesOk("select int_col, bigint_col from functional.alltypes order by 1 + 4");
+    AnalysisError("select int_col, bool_col, count(*) from functional.alltypes " +
+        "group by 1, 2 having 1 + 1",
+        "HAVING clause '1 + 1' requires return type 'BOOLEAN'. " +
+        "Actual type is 'SMALLINT'.");
+    AnalyzesOk("select int_col, count(*) from functional.alltypes group by 1, " +
+        "if(true, 2, int_col)");
+    AnalyzesOk("select int_col, bigint_col from functional.alltypes order by " +
+        "if(true, 7, int_col)");
+    AnalysisError("select int_col, bool_col, count(*) from functional.alltypes " +
+        "group by 1, 2 having if(TRUE, 2, int_col)",
+        "HAVING clause 'if(TRUE, 2, int_col)' requires return type 'BOOLEAN'. " +
+        "Actual type is 'INT'.");
   }
 
   @Test
@@ -1999,7 +2082,8 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
 
     // Test select stmt avg smap.
     AnalyzesOk("select cast(avg(c1) as decimal(10,4)) as c from " +
-        "functional.decimal_tiny group by c3 having c = 5.1106 order by 1");
+        "functional.decimal_tiny group by c3 having cast(avg(c1) as " +
+        "decimal(10,4)) = 5.1106 order by 1");
 
     // check CHAR and VARCHAR aggregates
     checkExprType("select min(cast('foo' as char(5))) from functional.chars_tiny",

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test b/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
index 2b2d5ef..37b3163 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
@@ -98,12 +98,12 @@ predicates: float_col != 0
    tuple-ids=0 row-size=116B cardinality=500
 ====
 # Test aggregation.
-select sum(1 + 1 + id) sm
+select sum(1 + 1 + id)
 from functional.alltypes
 group by timestamp_col = cast('2015-11-15' as timestamp) + interval 1 year
 having 1024 * 1024 * count(*) % 2 = 0
-  and (sm > 1 or sm > 1)
-  and (sm between 5 and 10)
+  and (sum(1 + 1 + id) > 1 or sum(1 + 1 + id) > 1)
+  and (sum(1 + 1 + id) between 5 and 10)
 ---- PLAN
 F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
 |  Per-Host Resources: mem-estimate=138.00MB mem-reservation=1.94MB

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test b/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
index cc29c8c..09a2993 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
@@ -1070,7 +1070,7 @@ inner join (select bigint_col, count(tinyint_col) x, max(smallint_col) y, min(in
             from functional.alltypessmall
             group by bigint_col
 # assigned in agg node and scan of t1
-            having x + y > 10) t2
+            having count(tinyint_col) + max(smallint_col) > 10) t2
   on (t1.id = t2.x and t1.tinyint_col = t2.y)
 inner join (select count(tinyint_col) x, max(smallint_col) y, min(int_col) z
             from functional.alltypestiny) t3

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/functional-query/queries/QueryTest/alias.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/alias.test b/testdata/workloads/functional-query/queries/QueryTest/alias.test
new file mode 100644
index 0000000..e14488f
--- /dev/null
+++ b/testdata/workloads/functional-query/queries/QueryTest/alias.test
@@ -0,0 +1,87 @@
+====
+---- QUERY
+# GROUP BY alias
+select int_col / 2 as x from alltypes group by x
+---- RESULTS
+1.5
+3.5
+4.5
+3
+1
+0.5
+4
+0
+2
+2.5
+---- TYPES
+double
+====
+---- QUERY
+# GROUP BY alias ORDER BY alias
+select int_col / 2 as x from alltypes group by x order by x
+---- RESULTS
+0
+0.5
+1
+1.5
+2
+2.5
+3
+3.5
+4
+4.5
+---- TYPES
+double
+====
+---- QUERY
+# HAVING with bool typed alias
+select int_col / 2 as x, not bool_col as nb
+from alltypes group by x, nb having nb
+---- RESULTS
+0.5,true
+3.5,true
+1.5,true
+4.5,true
+2.5,true
+---- TYPES
+double,boolean
+====
+---- QUERY
+# count(*) alias ORDER BY
+select count(*) a from alltypes order by a
+---- RESULTS
+7300
+---- TYPES
+bigint
+====
+---- QUERY
+# count(*) > 10 alias ORDER BY
+select count(*) > 10 a from alltypes order by a
+---- RESULTS
+true
+---- TYPES
+boolean
+====
+---- QUERY
+# count(*) > 10 alias HAVING
+select count(*) > 10 a from alltypes having a
+---- RESULTS
+true
+---- TYPES
+boolean
+====
+---- QUERY
+# sum(id) over(order by id) alias ORDER BY
+select sum(id) over(order by id) a from alltypestiny order by a
+---- RESULTS
+0
+1
+3
+6
+10
+15
+21
+28
+---- TYPES
+bigint
+====

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test b/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
index be75c23..dafe9ad 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
@@ -295,7 +295,7 @@ DECIMAL,DECIMAL,DECIMAL
 # Test AVG() with DECIMAL_V1
 set decimal_v2=false;
 select avg(l_extendedprice) as a from tpch_parquet.lineitem
-group by l_tax having a > 38247.190 order by 1;
+group by l_tax having avg(l_extendedprice) > 38247.190 order by 1;
 ---- RESULTS
 38250.48
 38251.62
@@ -309,7 +309,7 @@ DECIMAL
 # Test AVG() with DECIMAL_V2
 set decimal_v2=true;
 select avg(l_extendedprice) as a from tpch_parquet.lineitem
-group by l_tax having a > 38247.190 order by 1;
+group by l_tax having avg(l_extendedprice) > 38247.190 order by 1;
 ---- RESULTS
 38247.196745
 38250.487309

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/functional-query/queries/QueryTest/exprs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/exprs.test b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
index a15f3b5..50ee288 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
@@ -2622,7 +2622,8 @@ select count(now()) c, avg(cast('2016-11-22 16:40:00.00' as timestamp)) a,
 from functional_parquet.alltypes
 where timestamp_col < cast('2013-02-18 20:46:00.01' as timestamp)
 group by g
-having a = cast('2016-11-22 16:40:00.00' as timestamp)
+having avg(cast('2016-11-22 16:40:00.00' as timestamp)) =
+           cast('2016-11-22 16:40:00.00' as timestamp)
 order by c, cast('2016-11-22 16:40:00.00' as timestamp)
 ---- RESULTS
 7300,2016-11-22 16:40:00,2016-11-22 16:40:00
@@ -2651,8 +2652,8 @@ BIGINT
 select tinyint_col, count(*) cnt
 from functional_parquet.alltypesagg
 group by 1
-having cnt > 1000 or cnt > 1000
-  and cnt between 1500 and 2500
+having count(*) > 1000 or count(*) > 1000
+  and count(*) between 1500 and 2500
 ---- TYPES
 TINYINT, BIGINT
 ---- RESULTS

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch.test b/testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch.test
index c8a80b2..8ec3753 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/nested-types-tpch.test
@@ -135,7 +135,7 @@ t1.c_orders t3,
 t3.item.o_lineitems t4
 WHERE t1.c_custkey < 100 AND t2.p_partkey < 100
 GROUP BY 1
-HAVING int_col <= 5
+HAVING COALESCE(t2.p_size, t4.pos, o_orderkey) <= 5
 ORDER BY 1
 ---- RESULTS
 1,177

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_highndv.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_highndv.test b/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_highndv.test
index 2830837..3a87d99 100644
--- a/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_highndv.test
+++ b/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_highndv.test
@@ -6,7 +6,7 @@ SELECT l_orderkey,
        count(*) AS cnt
 FROM lineitem
 GROUP BY l_orderkey
-HAVING cnt > 9999999999999;
+HAVING count(*) > 9999999999999;
 ---- RESULTS
 ---- TYPES
 BIGINT,BIGINT

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_lowndv.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_lowndv.test b/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_lowndv.test
index b28d7d6..1856804 100644
--- a/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_lowndv.test
+++ b/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_lowndv.test
@@ -6,7 +6,7 @@ SELECT l_linenumber,
        count(*) AS cnt
 FROM lineitem
 GROUP BY l_linenumber
-HAVING cnt > 9999999999999;
+HAVING count(*) > 9999999999999;
 ---- RESULTS
 ---- TYPES
 ====

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_pk.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_pk.test b/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_pk.test
index f5db9d9..f45caaa 100644
--- a/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_pk.test
+++ b/testdata/workloads/targeted-perf/queries/primitive_groupby_bigint_pk.test
@@ -6,7 +6,7 @@ SELECT l_orderkey, l_partkey,
        count(*) AS cnt
 FROM lineitem
 GROUP BY l_orderkey,l_partkey
-HAVING cnt > 9999999999999;
+HAVING count(*) > 9999999999999;
 ---- RESULTS
 ---- TYPES
 ====

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_highndv.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_highndv.test b/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_highndv.test
index cfb976a..35c5845 100644
--- a/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_highndv.test
+++ b/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_highndv.test
@@ -7,7 +7,7 @@ SELECT l_extendedprice,
        count(*) AS cnt
 FROM lineitem
 GROUP BY l_extendedprice
-HAVING cnt > 9999999999999;
+HAVING count(*) > 9999999999999;
 ---- RESULTS
 ---- TYPES
 DECIMAL,BIGINT

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_lowndv.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_lowndv.test b/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_lowndv.test
index 6398059..7f18b3c 100644
--- a/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_lowndv.test
+++ b/testdata/workloads/targeted-perf/queries/primitive_groupby_decimal_lowndv.test
@@ -7,7 +7,7 @@ SELECT l_discount,
        count(*) AS cnt
 FROM lineitem
 GROUP BY l_discount
-HAVING cnt > 9999999999999;
+HAVING count(*) > 9999999999999;
 ---- RESULTS
 ---- TYPES
 ====

http://git-wip-us.apache.org/repos/asf/impala/blob/545e60f8/tests/query_test/test_queries.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_queries.py b/tests/query_test/test_queries.py
index 997b59a..4667d96 100644
--- a/tests/query_test/test_queries.py
+++ b/tests/query_test/test_queries.py
@@ -110,6 +110,9 @@ class TestQueries(ImpalaTestSuite):
   def test_subquery(self, vector):
     self.run_test_case('QueryTest/subquery', vector)
 
+  def test_alias(self, vector):
+    self.run_test_case('QueryTest/alias', vector)
+
   def test_subquery_in_constant_lhs(self, vector):
     self.run_test_case('QueryTest/subquery-in-constant-lhs', vector)
 


[6/6] impala git commit: IMPALA-6377: Bump breakpad version to include the fix for Breakpad #752

Posted by ta...@apache.org.
IMPALA-6377: Bump breakpad version to include the fix for Breakpad #752

This change bumps the Breakpad version to pull in the fix for
https://bugs.chromium.org/p/google-breakpad/issues/detail?id=752 .

Change-Id: I9b6212ab77eff2df0cb51339c25183ae4f22b07b
Reviewed-on: http://gerrit.cloudera.org:8080/9131
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: 0fdd81682db633ebc69ca415080e7b790ab901ab
Parents: 088d6ad
Author: Lars Volker <lv...@cloudera.com>
Authored: Wed Jan 24 18:31:40 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Jan 25 21:08:58 2018 +0000

----------------------------------------------------------------------
 bin/impala-config.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/0fdd8168/bin/impala-config.sh
----------------------------------------------------------------------
diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index 058dc01..bb1cb15 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -72,7 +72,7 @@ fi
 # moving to a different build of the toolchain, e.g. when a version is bumped or a
 # compile option is changed. The build id can be found in the output of the toolchain
 # build jobs, it is constructed from the build number and toolchain git hash prefix.
-export IMPALA_TOOLCHAIN_BUILD_ID=482-c2361403fc
+export IMPALA_TOOLCHAIN_BUILD_ID=39-a1bbd2851a
 # Versions of toolchain dependencies.
 # -----------------------------------
 export IMPALA_AVRO_VERSION=1.7.4-p4
@@ -81,7 +81,7 @@ export IMPALA_BINUTILS_VERSION=2.26.1
 unset IMPALA_BINUTILS_URL
 export IMPALA_BOOST_VERSION=1.57.0-p3
 unset IMPALA_BOOST_URL
-export IMPALA_BREAKPAD_VERSION=1b704857f1e78a864e6942e613457e55f1aecb60-p3
+export IMPALA_BREAKPAD_VERSION=97a98836768f8f0154f8f86e5e14c2bb7e74132e-p2
 unset IMPALA_BREAKPAD_URL
 export IMPALA_BZIP2_VERSION=1.0.6-p2
 unset IMPALA_BZIP2_URL


[5/6] impala git commit: IMPALA-6410: Use subprocess in compare_branches.py.

Posted by ta...@apache.org.
IMPALA-6410: Use subprocess in compare_branches.py.

Switches bin/compare_branches.py to use 'subprocess' instead
of 'sh'. We often use 'sh' in Impala testing code for its
friendly API, but it has to be installed separately. To avoid
automation that is just doing git operations needing to
either build the Impala python environment or otherwise get
extra libraries, I converted the usages.

As a side-effect, the script outputs the stdout of 'git cherry-pick',
whereas it used to swallow it. I like it better this way.

I tested this by running it in an environment which needed
some cherry-picks.

Change-Id: I509a548a129e7ad67aaf800a8ba03cffad51dd81
Reviewed-on: http://gerrit.cloudera.org:8080/9130
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: 088d6add784619e4f1be84588bafe94faf1693f4
Parents: b38db91
Author: Philip Zeyliger <ph...@cloudera.com>
Authored: Wed Jan 24 15:54:27 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Jan 25 20:27:59 2018 +0000

----------------------------------------------------------------------
 bin/compare_branches.py | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/088d6add/bin/compare_branches.py
----------------------------------------------------------------------
diff --git a/bin/compare_branches.py b/bin/compare_branches.py
index 6a81901..7050924 100755
--- a/bin/compare_branches.py
+++ b/bin/compare_branches.py
@@ -64,7 +64,7 @@ import json
 import logging
 import os
 import re
-import sh
+import subprocess
 import sys
 
 from collections import defaultdict
@@ -136,8 +136,8 @@ def build_commit_map(branch, merge_base):
   fields = ['%H', '%s', '%an', '%cd', '%b']
   pretty_format = '\x1f'.join(fields) + '\x1e'
   result = OrderedDict()
-  for line in sh.git.log(
-      branch, "^" + merge_base, pretty=pretty_format, color='never').split('\x1e'):
+  for line in subprocess.check_output(["git", "log", branch, "^" + merge_base,
+    "--pretty=" + pretty_format, "--color=never"]).split('\x1e'):
     if line == "":
       # if no changes are identified by the git log, we get an empty string
       continue
@@ -174,8 +174,9 @@ def cherrypick(cherry_pick_hashes, full_target_branch_name):
     return
 
   # Cherrypicking only makes sense if we're on the equivalent of the target branch.
-  head_sha = sh.git('rev-parse', 'HEAD').strip()
-  target_branch_sha = sh.git('rev-parse', full_target_branch_name).strip()
+  head_sha = subprocess.check_output(['git', 'rev-parse', 'HEAD']).strip()
+  target_branch_sha = subprocess.check_output(
+      ['git', 'rev-parse', full_target_branch_name]).strip()
   if head_sha != target_branch_sha:
     print "Cannot cherrypick because %s (%s) and HEAD (%s) are divergent." % (
         full_target_branch_name, target_branch_sha, head_sha)
@@ -183,7 +184,8 @@ def cherrypick(cherry_pick_hashes, full_target_branch_name):
 
   cherry_pick_hashes.reverse()
   for cherry_pick_hash in cherry_pick_hashes:
-    sh.git('cherry-pick', '--keep-redundant-commits', cherry_pick_hash)
+    subprocess.check_call(
+        ['git', 'cherry-pick', '--keep-redundant-commits', cherry_pick_hash])
 
 
 def main():
@@ -202,19 +204,19 @@ def main():
   # Ensure all branches are up to date, unless remotes are disabled
   # by specifying them with an empty string.
   if options.source_remote_name != "":
-    sh.git.fetch(options.source_remote_name)
+    subprocess.check_call(['git', 'fetch', options.source_remote_name])
     full_source_branch_name = options.source_remote_name + '/' + options.source_branch
   else:
     full_source_branch_name = options.source_branch
   if options.target_remote_name != "":
     if options.source_remote_name != options.target_remote_name:
-      sh.git.fetch(options.target_remote_name)
+      subprocess.check_call(['git', 'fetch', options.target_remote_name])
     full_target_branch_name = options.target_remote_name + '/' + options.target_branch
   else:
     full_target_branch_name = options.target_branch
 
-  merge_base = sh.git("merge-base",
-      full_source_branch_name, full_target_branch_name).strip()
+  merge_base = subprocess.check_output(["git", "merge-base",
+      full_source_branch_name, full_target_branch_name]).strip()
   source_commits = build_commit_map(full_source_branch_name, merge_base)
   target_commits = build_commit_map(full_target_branch_name, merge_base)
 


[4/6] impala git commit: KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

Posted by ta...@apache.org.
KUDU-2270: Add a flag to control logging in RpczStore::LogTrace()

This change adds a new flag FLAGS_rpc_duration_too_long_ms
which controls the duration above which a RPC is considered
too long and is logged at INFO level in the log. Previously,
this threshold is hardcoded to 1000ms which may be too short
for a busy Impalad demon, leading to massive log spew.

Change-Id: Ie587ee602e83bb65d74f7ee622a9bc47897f2574
Reviewed-on: http://gerrit.cloudera.org:8080/9117
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/9121
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: b38db91b5e463f68efbd7eacf3e719e34d87e2bb
Parents: 0a1d586
Author: Michael Ho <kw...@cloudera.com>
Authored: Wed Jan 24 00:05:11 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Jan 25 04:36:06 2018 +0000

----------------------------------------------------------------------
 be/src/kudu/rpc/rpc-test.cc   | 1 +
 be/src/kudu/rpc/rpcz_store.cc | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/b38db91b/be/src/kudu/rpc/rpc-test.cc
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/rpc-test.cc b/be/src/kudu/rpc/rpc-test.cc
index dc29323..6d9d156 100644
--- a/be/src/kudu/rpc/rpc-test.cc
+++ b/be/src/kudu/rpc/rpc-test.cc
@@ -43,6 +43,7 @@ METRIC_DECLARE_histogram(rpc_incoming_queue_time);
 
 DECLARE_bool(rpc_reopen_outbound_connections);
 DECLARE_int32(rpc_negotiation_inject_delay_ms);
+DECLARE_int32(rpc_duration_too_long_ms);
 
 using std::shared_ptr;
 using std::string;

http://git-wip-us.apache.org/repos/asf/impala/blob/b38db91b/be/src/kudu/rpc/rpcz_store.cc
----------------------------------------------------------------------
diff --git a/be/src/kudu/rpc/rpcz_store.cc b/be/src/kudu/rpc/rpcz_store.cc
index b1a0591..710a57e 100644
--- a/be/src/kudu/rpc/rpcz_store.cc
+++ b/be/src/kudu/rpc/rpcz_store.cc
@@ -40,6 +40,13 @@ DEFINE_bool_hidden(rpc_dump_all_traces, false,
 TAG_FLAG(rpc_dump_all_traces, advanced);
 TAG_FLAG(rpc_dump_all_traces, runtime);
 
+DEFINE_int32_hidden(rpc_duration_too_long_ms, 1000,
+             "Threshold (in milliseconds) above which a RPC is considered too long and its "
+             "duration and method name are logged at INFO level. The time measured is between "
+             "when a RPC is accepted and when its call handler completes.");
+TAG_FLAG(rpc_duration_too_long_ms, advanced);
+TAG_FLAG(rpc_duration_too_long_ms, runtime);
+
 using std::pair;
 using std::vector;
 using std::unique_ptr;
@@ -244,7 +251,7 @@ void RpczStore::LogTrace(InboundCall* call) {
   if (PREDICT_FALSE(FLAGS_rpc_dump_all_traces)) {
     LOG(INFO) << call->ToString() << " took " << duration_ms << "ms. Trace:";
     call->trace()->Dump(&LOG(INFO), true);
-  } else if (duration_ms > 1000) {
+  } else if (duration_ms > FLAGS_rpc_duration_too_long_ms) {
     LOG(INFO) << call->ToString() << " took " << duration_ms << "ms. "
               << "Request Metrics: " << call->trace()->MetricsAsJSON();
   }


[2/6] impala git commit: IMPALA-6383: free memory after skipping parquet row groups

Posted by ta...@apache.org.
IMPALA-6383: free memory after skipping parquet row groups

Before this patch, resources were only flushed after breaking out of
NextRowGroup(). This is a problem because resources can be allocated
for skipped row groups (e.g. for reading dictionaries).

Testing:
Tested in conjunction with a prototype buffer pool patch that was
DCHECKing before the change.

Added DCHECKs to the current version to ensure the streams are cleared
up as expected.

Ran the repro for IMPALA-6419 to confirm this iteration of the patch
fixed the original problem.

Change-Id: I95713675455f7635fa3f72616b166f35e2a46c1a
Reviewed-on: http://gerrit.cloudera.org:8080/9059
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: 1fc40f4a6661e8e9d2550458adae32f40122db28
Parents: 545e60f
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Jan 10 15:35:41 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Jan 25 03:55:31 2018 +0000

----------------------------------------------------------------------
 be/src/exec/hdfs-parquet-scanner.cc | 52 +++++++++++++++++++++-----------
 be/src/exec/hdfs-parquet-scanner.h  |  5 +++
 be/src/exec/scanner-context.h       |  8 ++---
 3 files changed, 44 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/1fc40f4a/be/src/exec/hdfs-parquet-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-parquet-scanner.cc b/be/src/exec/hdfs-parquet-scanner.cc
index 3a17a3b..c14edd7 100644
--- a/be/src/exec/hdfs-parquet-scanner.cc
+++ b/be/src/exec/hdfs-parquet-scanner.cc
@@ -229,6 +229,7 @@ Status HdfsParquetScanner::Open(ScannerContext* context) {
   // Release I/O buffers immediately to make sure they are cleaned up
   // in case we return a non-OK status anywhere below.
   context_->ReleaseCompletedResources(true);
+  context_->ClearStreams();
   RETURN_IF_ERROR(footer_status);
 
   // Parse the file schema into an internal representation for schema resolution.
@@ -264,7 +265,7 @@ void HdfsParquetScanner::Close(RowBatch* row_batch) {
     }
   } else {
     template_tuple_pool_->FreeAll();
-    dictionary_pool_.get()->FreeAll();
+    dictionary_pool_->FreeAll();
     context_->ReleaseCompletedResources(true);
     for (ParquetColumnReader* col_reader : column_readers_) col_reader->Close(nullptr);
     // The scratch batch may still contain tuple data. We can get into this case if
@@ -479,7 +480,6 @@ Status HdfsParquetScanner::GetNextInternal(RowBatch* row_batch) {
     // Transfer resources and clear streams if there is any leftover from the previous
     // row group. We will create new streams for the next row group.
     FlushRowGroupResources(row_batch);
-    context_->ClearStreams();
     if (!advance_row_group_) {
       Status status =
           ValidateEndOfRowGroup(column_readers_, row_group_idx_, row_group_rows_read_);
@@ -620,6 +620,9 @@ Status HdfsParquetScanner::NextRowGroup() {
   while (true) {
     // Reset the parse status for the next row group.
     parse_status_ = Status::OK();
+    // Make sure that we don't have leftover resources from the file metadata scan range
+    // or previous row groups.
+    DCHECK_EQ(0, context_->NumStreams());
 
     ++row_group_idx_;
     if (row_group_idx_ >= file_metadata_.row_groups.size()) {
@@ -672,6 +675,9 @@ Status HdfsParquetScanner::NextRowGroup() {
     // of the column.
     RETURN_IF_ERROR(InitScalarColumns(row_group_idx_, dict_filterable_readers_));
 
+    // InitColumns() may have allocated resources to scan columns. If we skip this row
+    // group below, we must call ReleaseSkippedRowGroupResources() before continuing.
+
     // If there is a dictionary-encoded column where every value is eliminated
     // by a conjunct, the row group can be eliminated. This initializes dictionaries
     // for all columns visited.
@@ -680,10 +686,12 @@ Status HdfsParquetScanner::NextRowGroup() {
     if (!status.ok()) {
       // Either return an error or skip this row group if it is ok to ignore errors
       RETURN_IF_ERROR(state_->LogOrReturnError(status.msg()));
+      ReleaseSkippedRowGroupResources();
       continue;
     }
     if (skip_row_group_on_dict_filters) {
       COUNTER_ADD(num_dict_filtered_row_groups_counter_, 1);
+      ReleaseSkippedRowGroupResources();
       continue;
     }
 
@@ -695,10 +703,11 @@ Status HdfsParquetScanner::NextRowGroup() {
     if (!status.ok()) {
       // Either return an error or skip this row group if it is ok to ignore errors
       RETURN_IF_ERROR(state_->LogOrReturnError(status.msg()));
+      ReleaseSkippedRowGroupResources();
       continue;
     }
 
-    bool seeding_ok = true;
+    bool seeding_failed = false;
     for (ParquetColumnReader* col_reader: column_readers_) {
       // Seed collection and boolean column readers with NextLevel().
       // The ScalarColumnReaders use an optimized ReadValueBatch() that
@@ -707,19 +716,21 @@ Status HdfsParquetScanner::NextRowGroup() {
       // ScalarColumnReader::ReadValueBatch() which does not need seeding. This
       // will allow better sharing of code between the row-wise and column-wise
       // materialization strategies.
-      if (col_reader->NeedsSeedingForBatchedReading()) {
-        if (!col_reader->NextLevels()) {
-          seeding_ok = false;
-          break;
-        }
+      if (col_reader->NeedsSeedingForBatchedReading()
+          && !col_reader->NextLevels()) {
+        seeding_failed = true;
+        break;
       }
-      DCHECK(parse_status_.ok()) << "Invalid parse_status_" << parse_status_.GetDetail();
     }
-
-    if (!parse_status_.ok()) {
-      RETURN_IF_ERROR(state_->LogOrReturnError(parse_status_.msg()));
-    } else if (seeding_ok) {
-      // Found a non-empty row group and successfully initialized the column readers.
+    if (seeding_failed) {
+      if (!parse_status_.ok()) {
+        RETURN_IF_ERROR(state_->LogOrReturnError(parse_status_.msg()));
+      }
+      ReleaseSkippedRowGroupResources();
+      continue;
+    } else {
+      // Seeding succeeded - we're ready to read the row group.
+      DCHECK(parse_status_.ok()) << "Invalid parse_status_" << parse_status_.GetDetail();
       break;
     }
   }
@@ -733,9 +744,16 @@ void HdfsParquetScanner::FlushRowGroupResources(RowBatch* row_batch) {
   row_batch->tuple_data_pool()->AcquireData(dictionary_pool_.get(), false);
   scratch_batch_->ReleaseResources(row_batch->tuple_data_pool());
   context_->ReleaseCompletedResources(true);
-  for (ParquetColumnReader* col_reader : column_readers_) {
-    col_reader->Close(row_batch);
-  }
+  for (ParquetColumnReader* col_reader : column_readers_) col_reader->Close(row_batch);
+  context_->ClearStreams();
+}
+
+void HdfsParquetScanner::ReleaseSkippedRowGroupResources() {
+  dictionary_pool_->FreeAll();
+  scratch_batch_->ReleaseResources(nullptr);
+  context_->ReleaseCompletedResources(true);
+  for (ParquetColumnReader* col_reader : column_readers_) col_reader->Close(nullptr);
+  context_->ClearStreams();
 }
 
 bool HdfsParquetScanner::IsDictFilterable(BaseScalarColumnReader* col_reader) {

http://git-wip-us.apache.org/repos/asf/impala/blob/1fc40f4a/be/src/exec/hdfs-parquet-scanner.h
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-parquet-scanner.h b/be/src/exec/hdfs-parquet-scanner.h
index 2ddf0fc..b1409d7 100644
--- a/be/src/exec/hdfs-parquet-scanner.h
+++ b/be/src/exec/hdfs-parquet-scanner.h
@@ -654,6 +654,11 @@ class HdfsParquetScanner : public HdfsScanner {
   /// Should be called after completing a row group and when returning the last batch.
   void FlushRowGroupResources(RowBatch* row_batch);
 
+  /// Releases resources associated with a row group that was skipped and closes all
+  /// column readers. Should be called after skipping a row group from which no rows
+  /// were returned.
+  void ReleaseSkippedRowGroupResources();
+
   /// Evaluates whether the column reader is eligible for dictionary predicates
   bool IsDictFilterable(ParquetColumnReader* col_reader);
 

http://git-wip-us.apache.org/repos/asf/impala/blob/1fc40f4a/be/src/exec/scanner-context.h
----------------------------------------------------------------------
diff --git a/be/src/exec/scanner-context.h b/be/src/exec/scanner-context.h
index e316063..09a4bdc 100644
--- a/be/src/exec/scanner-context.h
+++ b/be/src/exec/scanner-context.h
@@ -89,7 +89,6 @@ class ScannerContext {
   ScannerContext(RuntimeState*, HdfsScanNodeBase*, HdfsPartitionDescriptor*,
       io::ScanRange* scan_range, const std::vector<FilterContext>& filter_ctxs,
       MemPool* expr_results_pool);
-
   /// Destructor verifies that all stream objects have been released.
   ~ScannerContext();
 
@@ -338,6 +337,8 @@ class ScannerContext {
     return streams_[idx].get();
   }
 
+  int NumStreams() const { return streams_.size(); }
+
   /// Release completed resources for all streams, e.g. the last buffer in each stream if
   /// the current read position is at the end of the buffer. If 'done' is true all
   /// resources are freed, even if the caller has not read that data yet. After calling
@@ -354,8 +355,8 @@ class ScannerContext {
   /// size to 0.
   void ClearStreams();
 
-  /// Add a stream to this ScannerContext for 'range'. Returns the added stream.
-  /// The stream is created in the runtime state's object pool
+  /// Add a stream to this ScannerContext for 'range'. The stream is owned by this
+  /// context.
   Stream* AddStream(io::ScanRange* range);
 
   /// Returns false if scan_node_ is multi-threaded and has been cancelled.
@@ -370,7 +371,6 @@ class ScannerContext {
 
   RuntimeState* state_;
   HdfsScanNodeBase* scan_node_;
-
   HdfsPartitionDescriptor* partition_desc_;
 
   /// Vector of streams. Non-columnar formats will always have one stream per context.


[3/6] impala git commit: IMPALA-4924: Enable Decimal V2 by default

Posted by ta...@apache.org.
IMPALA-4924: Enable Decimal V2 by default

In this commit we enable Decimal_V2 by default. We also update the
expected results in many of our tests.

Testing:
Ran an exhaustive test which almost passed. Updated the few failed
tests in it.

Cherry-pick: not for 2.x

Change-Id: Ibbdd05bf986b7947f106b396017faa3a0bd87fd7
Reviewed-on: http://gerrit.cloudera.org:8080/9062
Reviewed-by: Taras Bobrovytsky <tb...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: 0a1d586d2a2822ad9b4052fef91b1d147b9b4e2b
Parents: 1fc40f4
Author: Taras Bobrovytsky <tb...@cloudera.com>
Authored: Fri Jan 12 14:05:08 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Jan 25 04:33:11 2018 +0000

----------------------------------------------------------------------
 be/src/exprs/expr-test.cc                       | 79 +++++++++++++++++++-
 common/thrift/ImpalaInternalService.thrift      |  2 +-
 .../impala/analysis/AnalyzeExprsTest.java       | 64 ++++++++++------
 .../impala/analysis/AnalyzeStmtsTest.java       |  2 +-
 .../queries/QueryTest/decimal_avro.test         | 18 ++---
 .../queries/QueryTest/exprs.test                | 13 ++--
 .../queries/QueryTest/nested-types-subplan.test | 72 +++++++++---------
 .../queries/QueryTest/spilling-aggs.test        | 16 ++--
 .../functional-query/queries/QueryTest/uda.test | 30 ++++----
 .../queries/QueryTest/values.test               |  8 +-
 testdata/workloads/tpch/queries/tpch-q1.test    |  8 +-
 testdata/workloads/tpch/queries/tpch-q14.test   |  2 +-
 testdata/workloads/tpch/queries/tpch-q17.test   |  2 +-
 testdata/workloads/tpch/queries/tpch-q8.test    |  4 +-
 .../tpch_nested/queries/tpch_nested-q1.test     | 10 +--
 .../tpch_nested/queries/tpch_nested-q14.test    |  4 +-
 .../tpch_nested/queries/tpch_nested-q17.test    |  4 +-
 .../tpch_nested/queries/tpch_nested-q8.test     |  6 +-
 tests/hs2/test_hs2.py                           |  5 +-
 tests/query_test/test_aggregation.py            |  8 +-
 tests/query_test/test_decimal_casting.py        | 12 +--
 tests/query_test/test_mem_usage_scaling.py      |  3 +-
 tests/query_test/test_tpcds_queries.py          |  2 +-
 23 files changed, 235 insertions(+), 139 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index c6e81f1..877127d 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -5093,7 +5093,8 @@ TEST_F(ExprTest, MathFunctions) {
   TestValue("cast(-12345.345 as float) % cast(-7 as float)",
       TYPE_FLOAT, fmodf(-12345.345f, -7.0f));
   TestValue("cast(12345.345 as double) % 7", TYPE_DOUBLE, fmod(12345.345, 7.0));
-  TestValue("-12345.345 % cast(7 as double)", TYPE_DOUBLE, fmod(-12345.345, 7.0));
+  TestValue("cast(-12345.345 as double) % cast(7 as double)",
+      TYPE_DOUBLE, fmod(-12345.345, 7.0));
   TestValue("cast(-12345.345 as double) % -7", TYPE_DOUBLE, fmod(-12345.345, -7.0));
   // Test floating-point modulo by zero.
   TestIsNull("fmod(cast(-12345.345 as float), cast(0 as float))", TYPE_FLOAT);
@@ -7693,7 +7694,9 @@ TEST_F(ExprTest, DecimalFunctions) {
 
 // Sanity check some overflow casting. We have a random test framework that covers
 // this more thoroughly.
-TEST_F(ExprTest, DecimalOverflowCasts) {
+TEST_F(ExprTest, DecimalOverflowCastsDecimalV1) {
+  executor_->PushExecOption("DECIMAL_V2=0");
+
   TestDecimalValue("cast(123.456 as decimal(6,3))",
       Decimal4Value(123456), ColumnType::CreateDecimalType(6, 3));
   TestDecimalValue("cast(-123.456 as decimal(6,1))",
@@ -7762,6 +7765,78 @@ TEST_F(ExprTest, DecimalOverflowCasts) {
   // Tests converting a non-trivial empty string to a decimal (IMPALA-1566).
   TestIsNull("cast(regexp_replace('','a','b') as decimal(15,2))",
       ColumnType::CreateDecimalType(15,2));
+
+  executor_->PopExecOption();
+}
+
+TEST_F(ExprTest, DecimalOverflowCastsDecimalV2) {
+  executor_->PushExecOption("DECIMAL_V2=1");
+
+  TestDecimalValue("cast(123.456 as decimal(6,3))",
+      Decimal4Value(123456), ColumnType::CreateDecimalType(6, 3));
+  TestDecimalValue("cast(-123.456 as decimal(6,1))",
+      Decimal4Value(-1235), ColumnType::CreateDecimalType(6, 1));
+  TestDecimalValue("cast(123.456 as decimal(6,0))",
+      Decimal4Value(123), ColumnType::CreateDecimalType(6, 0));
+  TestDecimalValue("cast(-123.456 as decimal(3,0))",
+      Decimal4Value(-123), ColumnType::CreateDecimalType(3, 0));
+
+  TestDecimalValue("cast(123.4567890 as decimal(10,7))",
+      Decimal8Value(1234567890L), ColumnType::CreateDecimalType(10, 7));
+
+  TestDecimalValue("cast(cast(\"123.01234567890123456789\" as decimal(23,20))\
+      as decimal(12,9))", Decimal8Value(123012345679L),
+      ColumnType::CreateDecimalType(12, 9));
+  TestDecimalValue("cast(cast(\"123.01234567890123456789\" as decimal(23,20))\
+      as decimal(4,1))", Decimal4Value(1230), ColumnType::CreateDecimalType(4, 1));
+
+  TestDecimalValue("cast(cast(\"123.0123456789\" as decimal(13,10))\
+      as decimal(5,2))", Decimal4Value(12301), ColumnType::CreateDecimalType(5, 2));
+
+  // Overflow
+  TestError("cast(123.456 as decimal(2,0))");
+  TestError("cast(123.456 as decimal(2,1))");
+  TestError("cast(123.456 as decimal(2,2))");
+  TestError("cast(99.99 as decimal(2,2))");
+  TestError("cast(99.99 as decimal(2,0))");
+  TestError("cast(-99.99 as decimal(2,2))");
+  TestError("cast(-99.99 as decimal(3,1))");
+
+  TestDecimalValue("cast(999.99 as decimal(6,3))",
+      Decimal4Value(999990), ColumnType::CreateDecimalType(6, 3));
+  TestDecimalValue("cast(-999.99 as decimal(7,4))",
+      Decimal4Value(-9999900), ColumnType::CreateDecimalType(7, 4));
+  TestError("cast(9990.99 as decimal(6,3))");
+  TestError("cast(-9990.99 as decimal(7,4))");
+
+  TestDecimalValue("cast(123.4567890 as decimal(4, 1))",
+      Decimal4Value(1235), ColumnType::CreateDecimalType(4, 1));
+  TestDecimalValue("cast(-123.4567890 as decimal(5, 2))",
+      Decimal4Value(-12346), ColumnType::CreateDecimalType(5, 2));
+  TestError("cast(123.4567890 as decimal(2, 1))");
+  TestError("cast(123.4567890 as decimal(6, 5))");
+
+  TestDecimalValue("cast(pi() as decimal(1, 0))",
+      Decimal4Value(3), ColumnType::CreateDecimalType(1,0));
+  TestDecimalValue("cast(pi() as decimal(4, 1))",
+      Decimal4Value(31), ColumnType::CreateDecimalType(4,1));
+  TestDecimalValue("cast(pi() as decimal(30, 1))",
+      Decimal8Value(31), ColumnType::CreateDecimalType(30,1));
+  TestError("cast(pi() as decimal(4, 4))");
+  TestError("cast(pi() as decimal(11, 11))");
+  TestError("cast(pi() as decimal(31, 31))");
+
+  TestError("cast(140573315541874605.4665184383287 as decimal(17, 13))");
+  TestError("cast(140573315541874605.4665184383287 as decimal(9, 3))");
+
+  // value has 30 digits before the decimal, casting to 29 is an overflow.
+  TestError("cast(99999999999999999999999999999.9 as decimal(29, 1))");
+
+  // Tests converting a non-trivial empty string to a decimal (IMPALA-1566).
+  TestIsNull("cast(regexp_replace('','a','b') as decimal(15,2))",
+      ColumnType::CreateDecimalType(15,2));
+
+  executor_->PopExecOption();
 }
 
 TEST_F(ExprTest, NullValueFunction) {

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/common/thrift/ImpalaInternalService.thrift
----------------------------------------------------------------------
diff --git a/common/thrift/ImpalaInternalService.thrift b/common/thrift/ImpalaInternalService.thrift
index 121c551..9494f6b 100644
--- a/common/thrift/ImpalaInternalService.thrift
+++ b/common/thrift/ImpalaInternalService.thrift
@@ -247,7 +247,7 @@ struct TQueryOptions {
   51: optional bool enable_expr_rewrites = true
 
   // Indicates whether to use the new decimal semantics.
-  52: optional bool decimal_v2 = false
+  52: optional bool decimal_v2 = true
 
   // Indicates whether to use dictionary filtering for Parquet files
   53: optional bool parquet_dictionary_filtering = true

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
index 91d1c00..80ba630 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
@@ -2210,14 +2210,34 @@ public class AnalyzeExprsTest extends AnalyzerTest {
     testFuncExprDepthLimit("cast(", "1", " as int)");
   }
 
-  // Verifies the resulting expr decimal type is exptectedType
-  private void testDecimalExpr(String expr, Type expectedType) {
-    SelectStmt selectStmt = (SelectStmt) AnalyzesOk("select " + expr);
+  // Verifies the resulting expr decimal type is expectedType under decimal v1 and
+  // decimal v2.
+  private void testDecimalExpr(String expr,
+      Type decimalV1ExpectedType, Type decimalV2ExpectedType) {
+    Analyzer analyzer = createAnalyzer(Catalog.DEFAULT_DB);
+
+    analyzer.getQueryOptions().setDecimal_v2(false);
+    SelectStmt selectStmt = (SelectStmt) AnalyzesOk("select " + expr, analyzer);
     Expr root = selectStmt.resultExprs_.get(0);
     Type actualType = root.getType();
     Assert.assertTrue(
-        "Expr: " + expr + " Expected: " + expectedType + " Actual: " + actualType,
-        expectedType.equals(actualType));
+        "Expr: " + expr + " Decimal Version: 1" +
+        " Expected: " + decimalV1ExpectedType + " Actual: " + actualType,
+        decimalV1ExpectedType.equals(actualType));
+
+    analyzer.getQueryOptions().setDecimal_v2(true);
+    selectStmt = (SelectStmt) AnalyzesOk("select " + expr, analyzer);
+    root = selectStmt.resultExprs_.get(0);
+    actualType = root.getType();
+    Assert.assertTrue(
+        "Expr: " + expr + " Decimal Version: 2" +
+            " Expected: " + decimalV2ExpectedType + " Actual: " + actualType,
+        decimalV2ExpectedType.equals(actualType));
+  }
+
+  // Verifies the resulting expr decimal type is exptectedType
+  private void testDecimalExpr(String expr, Type expectedType) {
+    testDecimalExpr(expr, expectedType, expectedType);
   }
 
   @Test
@@ -2236,7 +2256,7 @@ public class AnalyzeExprsTest extends AnalyzerTest {
     testDecimalExpr(decimal_10_0 + " - " + decimal_10_0,
         ScalarType.createDecimalType(11, 0));
     testDecimalExpr(decimal_10_0 + " * " + decimal_10_0,
-        ScalarType.createDecimalType(20, 0));
+        ScalarType.createDecimalType(20, 0), ScalarType.createDecimalType(21, 0));
     testDecimalExpr(decimal_10_0 + " / " + decimal_10_0,
         ScalarType.createDecimalType(21, 11));
     testDecimalExpr(decimal_10_0 + " % " + decimal_10_0,
@@ -2247,7 +2267,7 @@ public class AnalyzeExprsTest extends AnalyzerTest {
     testDecimalExpr(decimal_10_0 + " - " + decimal_5_5,
         ScalarType.createDecimalType(16, 5));
     testDecimalExpr(decimal_10_0 + " * " + decimal_5_5,
-        ScalarType.createDecimalType(15, 5));
+        ScalarType.createDecimalType(15, 5), ScalarType.createDecimalType(16, 5));
     testDecimalExpr(decimal_10_0 + " / " + decimal_5_5,
         ScalarType.createDecimalType(21, 6));
     testDecimalExpr(decimal_10_0 + " % " + decimal_5_5,
@@ -2258,7 +2278,7 @@ public class AnalyzeExprsTest extends AnalyzerTest {
     testDecimalExpr(decimal_5_5 + " - " + decimal_10_0,
         ScalarType.createDecimalType(16, 5));
     testDecimalExpr(decimal_5_5 + " * " + decimal_10_0,
-        ScalarType.createDecimalType(15, 5));
+        ScalarType.createDecimalType(15, 5), ScalarType.createDecimalType(16, 5));
     testDecimalExpr(decimal_5_5 + " / " + decimal_10_0,
         ScalarType.createDecimalType(16, 16));
     testDecimalExpr(decimal_5_5 + " % " + decimal_10_0,
@@ -2266,31 +2286,31 @@ public class AnalyzeExprsTest extends AnalyzerTest {
 
     // Test some overflow cases.
     testDecimalExpr(decimal_10_0 + " + " + decimal_38_34,
-        ScalarType.createDecimalType(38, 34));
+        ScalarType.createDecimalType(38, 34), ScalarType.createDecimalType(38, 27));
     testDecimalExpr(decimal_10_0 + " - " + decimal_38_34,
-        ScalarType.createDecimalType(38, 34));
+        ScalarType.createDecimalType(38, 34), ScalarType.createDecimalType(38, 27));
     testDecimalExpr(decimal_10_0 + " * " + decimal_38_34,
-        ScalarType.createDecimalType(38, 34));
+        ScalarType.createDecimalType(38, 34), ScalarType.createDecimalType(38, 23));
     testDecimalExpr(decimal_10_0 + " / " + decimal_38_34,
-        ScalarType.createDecimalType(38, 34));
+        ScalarType.createDecimalType(38, 34), ScalarType.createDecimalType(38, 6));
     testDecimalExpr(decimal_10_0 + " % " + decimal_38_34,
         ScalarType.createDecimalType(38, 34));
 
     testDecimalExpr(decimal_38_34 + " + " + decimal_5_5,
-        ScalarType.createDecimalType(38, 34));
+        ScalarType.createDecimalType(38, 34), ScalarType.createDecimalType(38, 33));
     testDecimalExpr(decimal_38_34 + " - " + decimal_5_5,
-        ScalarType.createDecimalType(38, 34));
+        ScalarType.createDecimalType(38, 34), ScalarType.createDecimalType(38, 33));
     testDecimalExpr(decimal_38_34 + " * " + decimal_5_5,
-        ScalarType.createDecimalType(38, 38));
+        ScalarType.createDecimalType(38, 38), ScalarType.createDecimalType(38, 33));
     testDecimalExpr(decimal_38_34 + " / " + decimal_5_5,
-        ScalarType.createDecimalType(38, 34));
+        ScalarType.createDecimalType(38, 34), ScalarType.createDecimalType(38, 29));
     testDecimalExpr(decimal_38_34 + " % " + decimal_5_5,
         ScalarType.createDecimalType(34, 34));
 
     testDecimalExpr(decimal_10_0 + " + " + decimal_10_0 + " + " + decimal_10_0,
         ScalarType.createDecimalType(12, 0));
     testDecimalExpr(decimal_10_0 + " - " + decimal_10_0 + " * " + decimal_10_0,
-        ScalarType.createDecimalType(21, 0));
+        ScalarType.createDecimalType(21, 0), ScalarType.createDecimalType(22, 0));
     testDecimalExpr(decimal_10_0 + " / " + decimal_10_0 + " / " + decimal_10_0,
         ScalarType.createDecimalType(32, 22));
     testDecimalExpr(decimal_10_0 + " % " + decimal_10_0 + " + " + decimal_10_0,
@@ -2307,22 +2327,22 @@ public class AnalyzeExprsTest extends AnalyzerTest {
     testDecimalExpr(decimal_10_0 + " + cast(1 as bigint)",
         ScalarType.createDecimalType(20, 0));
     testDecimalExpr(decimal_10_0 + " + cast(1 as float)",
-        ScalarType.createDecimalType(38, 9));
+        ScalarType.createDecimalType(38, 9), ScalarType.createDecimalType(38, 8));
     testDecimalExpr(decimal_10_0 + " + cast(1 as double)",
-        ScalarType.createDecimalType(38, 17));
+        ScalarType.createDecimalType(38, 17), ScalarType.createDecimalType(38, 16));
 
     testDecimalExpr(decimal_5_5 + " + cast(1 as tinyint)",
         ScalarType.createDecimalType(9, 5));
     testDecimalExpr(decimal_5_5 + " - cast(1 as smallint)",
         ScalarType.createDecimalType(11, 5));
     testDecimalExpr(decimal_5_5 + " * cast(1 as int)",
-        ScalarType.createDecimalType(15, 5));
+        ScalarType.createDecimalType(15, 5), ScalarType.createDecimalType(16, 5));
     testDecimalExpr(decimal_5_5 + " % cast(1 as bigint)",
         ScalarType.createDecimalType(5, 5));
     testDecimalExpr(decimal_5_5 + " / cast(1 as float)",
-        ScalarType.createDecimalType(38, 9));
+        ScalarType.createDecimalType(38, 9), ScalarType.createDecimalType(38, 29));
     testDecimalExpr(decimal_5_5 + " + cast(1 as double)",
-        ScalarType.createDecimalType(38, 17));
+        ScalarType.createDecimalType(38, 17), ScalarType.createDecimalType(38, 16));
 
     AnalyzesOk("select " + decimal_5_5 + " = cast(1 as tinyint)");
     AnalyzesOk("select " + decimal_5_5 + " != cast(1 as smallint)");

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
index 1d8d1be..d3311ba 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
@@ -2333,7 +2333,7 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
     AnalysisError("select count(*) from functional.alltypes " +
         "group by bool_col having 5 + 10 * 5.6",
         "HAVING clause '5 + 10 * 5.6' requires return type 'BOOLEAN'. " +
-        "Actual type is 'DOUBLE'.");
+        "Actual type is 'DECIMAL(7,1)'.");
     AnalysisError("select count(*) from functional.alltypes " +
         "group by bool_col having int_col",
         "HAVING clause 'int_col' requires return type 'BOOLEAN'. Actual type is 'INT'.");

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test b/testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
index 73fd27d..19fddcb 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/decimal_avro.test
@@ -43,13 +43,13 @@ select l_shipmode, count(distinct l_quantity), avg(l_extendedprice), max(l_disco
 from tpch_avro_snap.lineitem
 group by l_shipmode
 ---- RESULTS
-'SHIP',50,38267.37,0.10,9
-'REG AIR',50,38268.41,0.10,9
-'TRUCK',50,38209.82,0.10,9
-'RAIL',50,38269.81,0.10,9
-'FOB',50,38246.23,0.10,9
-'MAIL',50,38224.29,0.10,9
-'AIR',50,38299.98,0.10,9
-----TYPES
+'SHIP',50,38267.370378,0.10,9
+'REG AIR',50,38268.410670,0.10,9
+'RAIL',50,38269.811059,0.10,9
+'MAIL',50,38224.291934,0.10,9
+'AIR',50,38299.981696,0.10,9
+'TRUCK',50,38209.826048,0.10,9
+'FOB',50,38246.233625,0.10,9
+---- TYPES
 STRING,DECIMAL,DECIMAL,DECIMAL,DECIMAL
-====
+====
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/functional-query/queries/QueryTest/exprs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/exprs.test b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
index 50ee288..1cb803b 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
@@ -72,7 +72,6 @@ bigint
 ====
 ---- QUERY
 # boolean test: IS [NOT] (TRUE | FALSE | UNKNOWN)
----- QUERY
 select count(*) from alltypesagg where (int_col < 100) is unknown;
 ---- RESULTS
 20
@@ -1879,9 +1878,9 @@ string
 ---- QUERY
 select 1.1 * 1.1 + cast(1.1 as float)
 ---- RESULTS
-2.310000023841858
+2.31000002
 ---- TYPES
-double
+DECIMAL
 ====
 ---- QUERY
 select 1.1 * 1.1 + cast(1.1 as decimal(2,1))
@@ -1900,10 +1899,10 @@ decimal
 ---- QUERY
 select 1.1 * 1.1 + float_col from functional.alltypestiny limit 2;
 ---- RESULTS
-1.21
-2.310000023841858
+1.21000000
+2.31000002
 ---- TYPES
-double
+DECIMAL
 ====
 ---- QUERY
 select 1.1 * 1.1 + c3 from functional.decimal_tiny limit 2;
@@ -2236,7 +2235,7 @@ select tmp.str from (values
 ('multi\nfield\ntwo')) as tmp
 where regexp_like(tmp.str, '^multi.*$')
 ---- RESULTS
-----TYPES
+---- TYPES
 string
 ====
 ---- QUERY

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test b/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
index 82930ad..2f10072 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
@@ -145,9 +145,9 @@ select c_custkey, v.* from customer c,
    from c.c_orders) v
 where c_custkey < 4
 ---- RESULTS
-1,6,587762.91,97960.48,'O','1992-04-19'
-2,7,1028273.43,146896.20,'P','1992-04-05'
+1,6,587762.91,97960.485000,'O','1992-04-19'
 3,0,NULL,NULL,'NULL','NULL'
+2,7,1028273.43,146896.204286,'P','1992-04-05'
 ---- TYPES
 bigint,bigint,decimal,decimal,string,string
 ====
@@ -189,13 +189,13 @@ select c_custkey, v.* from customer c,
    group by o_orderpriority) v
 where c_custkey < 4
 ---- RESULTS
-1,'1-URGENT',2,249248.75,124624.37,'O','1992-04-19'
-1,'2-HIGH',1,65478.05,65478.05,'O','1996-06-29'
-1,'3-MEDIUM',1,95911.01,95911.01,'O','1997-03-23'
-1,'5-LOW',2,177125.10,88562.55,'O','1992-08-22'
-2,'1-URGENT',4,670495.57,167623.89,'O','1992-04-05'
-2,'2-HIGH',1,221397.35,221397.35,'P','1995-03-10'
-2,'4-NOT SPECIFIED',2,136380.51,68190.25,'F','1994-05-21'
+1,'2-HIGH',1,65478.05,65478.050000,'O','1996-06-29'
+1,'5-LOW',2,177125.10,88562.550000,'O','1992-08-22'
+1,'1-URGENT',2,249248.75,124624.375000,'O','1992-04-19'
+1,'3-MEDIUM',1,95911.01,95911.010000,'O','1997-03-23'
+2,'2-HIGH',1,221397.35,221397.350000,'P','1995-03-10'
+2,'4-NOT SPECIFIED',2,136380.51,68190.255000,'F','1994-05-21'
+2,'1-URGENT',4,670495.57,167623.892500,'O','1992-04-05'
 ---- TYPES
 bigint,string,bigint,decimal,decimal,string,string
 ====
@@ -208,19 +208,19 @@ select c_custkey, v.* from customer c,
    from c.c_orders) v
 where c_custkey < 4
 ---- RESULTS
-1,6,587762.91,97960.48,'O','1992-04-19'
-1,6,587762.91,97960.48,'O','1992-04-19'
-1,6,587762.91,97960.48,'O','1992-04-19'
-1,6,587762.91,97960.48,'O','1992-04-19'
-1,6,587762.91,97960.48,'O','1992-04-19'
-1,6,587762.91,97960.48,'O','1992-04-19'
-2,7,1028273.43,146896.20,'P','1992-04-05'
-2,7,1028273.43,146896.20,'P','1992-04-05'
-2,7,1028273.43,146896.20,'P','1992-04-05'
-2,7,1028273.43,146896.20,'P','1992-04-05'
-2,7,1028273.43,146896.20,'P','1992-04-05'
-2,7,1028273.43,146896.20,'P','1992-04-05'
-2,7,1028273.43,146896.20,'P','1992-04-05'
+1,6,587762.91,97960.485000,'O','1992-04-19'
+1,6,587762.91,97960.485000,'O','1992-04-19'
+1,6,587762.91,97960.485000,'O','1992-04-19'
+1,6,587762.91,97960.485000,'O','1992-04-19'
+1,6,587762.91,97960.485000,'O','1992-04-19'
+1,6,587762.91,97960.485000,'O','1992-04-19'
+2,7,1028273.43,146896.204286,'P','1992-04-05'
+2,7,1028273.43,146896.204286,'P','1992-04-05'
+2,7,1028273.43,146896.204286,'P','1992-04-05'
+2,7,1028273.43,146896.204286,'P','1992-04-05'
+2,7,1028273.43,146896.204286,'P','1992-04-05'
+2,7,1028273.43,146896.204286,'P','1992-04-05'
+2,7,1028273.43,146896.204286,'P','1992-04-05'
 ---- TYPES
 bigint,bigint,decimal,decimal,string,string
 ====
@@ -236,19 +236,19 @@ select c_custkey, v.* from customer c,
 from c.c_orders) v
 where c_custkey < 4
 ---- RESULTS
-1,'F',2,197679.65,98839.82,'F','1992-04-19'
-1,'F',2,197679.65,98839.82,'F','1992-04-19'
-1,'O',4,390083.26,97520.81,'O','1996-06-29'
-1,'O',4,390083.26,97520.81,'O','1996-06-29'
-1,'O',4,390083.26,97520.81,'O','1996-06-29'
-1,'O',4,390083.26,97520.81,'O','1996-06-29'
-2,'F',4,319892.45,79973.11,'F','1992-04-05'
-2,'F',4,319892.45,79973.11,'F','1992-04-05'
-2,'F',4,319892.45,79973.11,'F','1992-04-05'
-2,'F',4,319892.45,79973.11,'F','1992-04-05'
-2,'O',2,486983.63,243491.81,'O','1996-08-05'
-2,'O',2,486983.63,243491.81,'O','1996-08-05'
-2,'P',1,221397.35,221397.35,'P','1995-03-10'
+1,'F',2,197679.65,98839.825000,'F','1992-04-19'
+1,'F',2,197679.65,98839.825000,'F','1992-04-19'
+1,'O',4,390083.26,97520.815000,'O','1996-06-29'
+1,'O',4,390083.26,97520.815000,'O','1996-06-29'
+1,'O',4,390083.26,97520.815000,'O','1996-06-29'
+1,'O',4,390083.26,97520.815000,'O','1996-06-29'
+2,'F',4,319892.45,79973.112500,'F','1992-04-05'
+2,'F',4,319892.45,79973.112500,'F','1992-04-05'
+2,'F',4,319892.45,79973.112500,'F','1992-04-05'
+2,'F',4,319892.45,79973.112500,'F','1992-04-05'
+2,'O',2,486983.63,243491.815000,'O','1996-08-05'
+2,'O',2,486983.63,243491.815000,'O','1996-08-05'
+2,'P',1,221397.35,221397.350000,'P','1995-03-10'
 ---- TYPES
 bigint,string,bigint,decimal,decimal,string,string
 ====
@@ -328,7 +328,7 @@ from tpch_nested_parquet.customer c,
 group by opriority
 ---- TYPES
 bigint, string
----- RESULTS:
+---- RESULTS
 ====
 ---- QUERY
 # Test left semi join of a relative table ref.

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test b/testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
index 05552d2..5a58f62 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/spilling-aggs.test
@@ -32,9 +32,9 @@ from lineitem
 group by 1,2
 order by 1,2 limit 3
 ---- RESULTS
-'A',3,0.05,'RAIL'
-'A',5,0.03,'AIR'
-'A',6,0.03,'TRUCK'
+'A',3,0.050000,'RAIL'
+'A',5,0.030000,'AIR'
+'A',6,0.030000,'TRUCK'
 ---- TYPES
 STRING, BIGINT, DECIMAL, STRING
 ---- RUNTIME_PROFILE
@@ -194,11 +194,11 @@ limit 5
 ---- TYPES
 BIGINT,DECIMAL,DECIMAL,DECIMAL,DECIMAL
 ---- RESULTS
-3811460,0.05,50.00,0.05,104899.50
-1744195,0.04,50.00,0.09,104649.50
-5151266,0.07,50.00,0.00,104449.50
-4571042,0.03,50.00,0.09,104399.50
-1198304,0.01,50.00,0.02,104299.50
+3811460,0.050000,50.000000,0.050000,104899.500000
+1744195,0.040000,50.000000,0.090000,104649.500000
+5151266,0.070000,50.000000,0.000000,104449.500000
+4571042,0.030000,50.000000,0.090000,104399.500000
+1198304,0.010000,50.000000,0.020000,104299.500000
 ---- RUNTIME_PROFILE
 row_regex: .*SpilledPartitions: .* \([1-9][0-9]*\)
 ====

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/functional-query/queries/QueryTest/uda.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/uda.test b/testdata/workloads/functional-query/queries/QueryTest/uda.test
index ef3f1a6..62812e6 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/uda.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/uda.test
@@ -82,10 +82,10 @@ bigint,bigint
 ---- QUERY
 # Test that all types are exposed via the FunctionContext correctly.
 # This relies on asserts in the UDA funciton
-select agg_decimal_intermediate(cast(d1 as decimal(2,1)), 2), count(*)
-from functional.decimal_tbl
+select agg_decimal_intermediate(cast(c3 as decimal(2,1)), 2), count(*)
+from functional.decimal_tiny
 ---- RESULTS
-NULL,5
+NULL,100
 ---- TYPES
 decimal,bigint
 ====
@@ -106,7 +106,7 @@ from
    functional.alltypesagg,
    functional.decimal_tiny
 ---- RESULTS
-100,NULL,NULL,160.49989,-10.0989,11.8989,999,499500
+100,NULL,NULL,160.499890,-10.0989,11.8989,999,499500
 ---- TYPES
 decimal,decimal,bigint,decimal,decimal,decimal,bigint,bigint
 ====
@@ -126,17 +126,17 @@ from
 group by
    year,month,day
 ---- RESULTS
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
-100,NULL,NULL,99,5.4994
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
+100,NULL,NULL,99,5.499450
 ---- TYPES
 decimal,decimal,bigint,bigint,decimal
 ====

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/functional-query/queries/QueryTest/values.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/values.test b/testdata/workloads/functional-query/queries/QueryTest/values.test
index 8cdf23c..ac33e3d 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/values.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/values.test
@@ -84,10 +84,10 @@ insert overwrite table i_2749 values
 ---- QUERY
 select dbl1 * dec * dbl2, dbl1 + dec, dbl1 - dec, dbl1 / dec from i_2749;
 ---- RESULTS
-0.1547289,90.00170000000000000,-89.99830000000000000,0.00001888888888888
-3.112781400000001,90.03420000000000000,-89.96580000000000000,0.00038000000000000
-1.1650176,90.01280000000000000,-89.98720000000000000,0.00014222222222222
-1.4835771,90.01629999999999999,-89.98370000000000001,0.00018111111111111
+0.1547289,90.0017000000000000,-89.9983000000000000,0.0000188888889
+3.112781400000001,90.0342000000000000,-89.9658000000000000,0.0003800000000
+1.1650176,90.0128000000000000,-89.9872000000000000,0.0001422222222
+1.4835771,90.0163000000000000,-89.9837000000000000,0.0001811111111
 ---- TYPES
 DOUBLE,DECIMAL,DECIMAL,DECIMAL
 ====

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/tpch/queries/tpch-q1.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/tpch/queries/tpch-q1.test b/testdata/workloads/tpch/queries/tpch-q1.test
index 4147efe..08cb7cc 100644
--- a/testdata/workloads/tpch/queries/tpch-q1.test
+++ b/testdata/workloads/tpch/queries/tpch-q1.test
@@ -23,10 +23,10 @@ order by
   l_returnflag,
   l_linestatus
 ---- RESULTS
-'A','F',37734107.00,56586554400.73,53758257134.8700,55909065222.827692,25.52,38273.12,0.04,1478493
-'N','F',991417.00,1487504710.38,1413082168.0541,1469649223.194375,25.51,38284.46,0.05,38854
-'N','O',74476040.00,111701729697.74,106118230307.6056,110367043872.497010,25.50,38249.11,0.04,2920374
-'R','F',37719753.00,56568041380.90,53741292684.6040,55889619119.831932,25.50,38250.85,0.05,1478870
+'A','F',37734107.00,56586554400.73,53758257134.8700,55909065222.827692,25.522006,38273.129735,0.049985,1478493
+'N','F',991417.00,1487504710.38,1413082168.0541,1469649223.194375,25.516472,38284.467761,0.050093,38854
+'N','O',74476040.00,111701729697.74,106118230307.6056,110367043872.497010,25.502227,38249.117989,0.049997,2920374
+'R','F',37719753.00,56568041380.90,53741292684.6040,55889619119.831932,25.505794,38250.854626,0.050009,1478870
 ---- TYPES
 string, string, decimal, decimal, decimal, decimal, decimal, decimal, decimal, bigint
 ====
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/tpch/queries/tpch-q14.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/tpch/queries/tpch-q14.test b/testdata/workloads/tpch/queries/tpch-q14.test
index d84b9bb..8fb41b4 100644
--- a/testdata/workloads/tpch/queries/tpch-q14.test
+++ b/testdata/workloads/tpch/queries/tpch-q14.test
@@ -15,7 +15,7 @@ where
   and l_shipdate >= '1995-09-01'
   and l_shipdate < '1995-10-01'
 ---- RESULTS
-16.380778
+16.380779
 ---- TYPES
 decimal
 ====
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/tpch/queries/tpch-q17.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/tpch/queries/tpch-q17.test b/testdata/workloads/tpch/queries/tpch-q17.test
index e7ccfb4..ab4a62e 100644
--- a/testdata/workloads/tpch/queries/tpch-q17.test
+++ b/testdata/workloads/tpch/queries/tpch-q17.test
@@ -19,7 +19,7 @@ where
       l_partkey = p_partkey
   )
 ---- RESULTS
-348406.05
+348406.054286
 ---- TYPES
 decimal
 ====
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/tpch/queries/tpch-q8.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/tpch/queries/tpch-q8.test b/testdata/workloads/tpch/queries/tpch-q8.test
index 1b7fe8c..01ed843 100644
--- a/testdata/workloads/tpch/queries/tpch-q8.test
+++ b/testdata/workloads/tpch/queries/tpch-q8.test
@@ -39,8 +39,8 @@ group by
 order by
   o_year
 ---- RESULTS
-1995,0.0344
-1996,0.0414
+1995,0.034436
+1996,0.041486
 ---- TYPES
 int, decimal
 ====
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/tpch_nested/queries/tpch_nested-q1.test b/testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
index e0ff4e9..8077b55 100644
--- a/testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
+++ b/testdata/workloads/tpch_nested/queries/tpch_nested-q1.test
@@ -23,10 +23,10 @@ order by
   l_returnflag,
   l_linestatus
 ---- RESULTS
-'A','F',37734107.00,56586554400.73,53758257134.8700,55909065222.827692,25.52,38273.12,0.04,1478493
-'N','F',991417.00,1487504710.38,1413082168.0541,1469649223.194375,25.51,38284.46,0.05,38854
-'N','O',74476040.00,111701729697.74,106118230307.6056,110367043872.497010,25.50,38249.11,0.04,2920374
-'R','F',37719753.00,56568041380.90,53741292684.6040,55889619119.831932,25.50,38250.85,0.05,1478870
+'A','F',37734107.00,56586554400.73,53758257134.8700,55909065222.827692,25.522006,38273.129735,0.049985,1478493
+'N','F',991417.00,1487504710.38,1413082168.0541,1469649223.194375,25.516472,38284.467761,0.050093,38854
+'N','O',74476040.00,111701729697.74,106118230307.6056,110367043872.497010,25.502227,38249.117989,0.049997,2920374
+'R','F',37719753.00,56568041380.90,53741292684.6040,55889619119.831932,25.505794,38250.854626,0.050009,1478870
 ---- TYPES
 string, string, decimal, decimal, decimal, decimal, decimal, decimal, decimal, bigint
-====
+====
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/tpch_nested/queries/tpch_nested-q14.test b/testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
index b4c66ff..87abd7e 100644
--- a/testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
+++ b/testdata/workloads/tpch_nested/queries/tpch_nested-q14.test
@@ -15,7 +15,7 @@ where
   and l_shipdate >= '1995-09-01'
   and l_shipdate < '1995-10-01'
 ---- RESULTS
-16.380778
+16.380779
 ---- TYPES
 decimal
-====
+====
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/tpch_nested/queries/tpch_nested-q17.test b/testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
index d4b59f3..d07e724 100644
--- a/testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
+++ b/testdata/workloads/tpch_nested/queries/tpch_nested-q17.test
@@ -19,7 +19,7 @@ where
       l_partkey = p_partkey
   )
 ---- RESULTS
-348406.05
+348406.054286
 ---- TYPES
 decimal
-====
+====
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/tpch_nested/queries/tpch_nested-q8.test b/testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
index 94d91a6..19b083b 100644
--- a/testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
+++ b/testdata/workloads/tpch_nested/queries/tpch_nested-q8.test
@@ -36,8 +36,8 @@ group by
 order by
   o_year
 ---- RESULTS
-1995,0.0344
-1996,0.0414
+1995,0.034436
+1996,0.041486
 ---- TYPES
 int, decimal
-====
+====
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/tests/hs2/test_hs2.py
----------------------------------------------------------------------
diff --git a/tests/hs2/test_hs2.py b/tests/hs2/test_hs2.py
index 040d997..c90113c 100644
--- a/tests/hs2/test_hs2.py
+++ b/tests/hs2/test_hs2.py
@@ -439,9 +439,8 @@ class TestHS2(HS2TestSuite):
     log = self.get_log("select * from functional.alltypeserror")
     assert "Error converting column" in log
 
-    # Test overflow warning
-    log = self.get_log("select cast(1000 as decimal(2, 1))")
-    assert "Decimal expression overflowed, returning NULL" in log
+    log = self.get_log("select base64decode('foo')")
+    assert "Invalid base64 string; input length is 3, which is not a multiple of 4" in log
 
   @needs_session()
   def test_get_exec_summary(self):

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/tests/query_test/test_aggregation.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_aggregation.py b/tests/query_test/test_aggregation.py
index aee2236..97c0e41 100644
--- a/tests/query_test/test_aggregation.py
+++ b/tests/query_test/test_aggregation.py
@@ -297,10 +297,10 @@ class TestAggregationQueries(ImpalaTestSuite):
                ndv(smallint_col), ndv(int_col),
                ndv(bigint_col), ndv(float_col),
                ndv(double_col), ndv(string_col),
-               ndv(cast(double_col as decimal(3, 0))),
+               ndv(cast(double_col as decimal(5, 0))),
                ndv(cast(double_col as decimal(10, 5))),
                ndv(cast(double_col as decimal(20, 10))),
-               ndv(cast(double_col as decimal(38, 35))),
+               ndv(cast(double_col as decimal(38, 33))),
                ndv(cast(string_col as varchar(20))),
                ndv(cast(string_col as char(10))),
                ndv(timestamp_col), ndv(id)
@@ -314,10 +314,10 @@ class TestAggregationQueries(ImpalaTestSuite):
                  sampled_ndv(smallint_col, {0}), sampled_ndv(int_col, {0}),
                  sampled_ndv(bigint_col, {0}), sampled_ndv(float_col, {0}),
                  sampled_ndv(double_col, {0}), sampled_ndv(string_col, {0}),
-                 sampled_ndv(cast(double_col as decimal(3, 0)), {0}),
+                 sampled_ndv(cast(double_col as decimal(5, 0)), {0}),
                  sampled_ndv(cast(double_col as decimal(10, 5)), {0}),
                  sampled_ndv(cast(double_col as decimal(20, 10)), {0}),
-                 sampled_ndv(cast(double_col as decimal(38, 35)), {0}),
+                 sampled_ndv(cast(double_col as decimal(38, 33)), {0}),
                  sampled_ndv(cast(string_col as varchar(20)), {0}),
                  sampled_ndv(cast(string_col as char(10)), {0}),
                  sampled_ndv(timestamp_col, {0}), sampled_ndv(id, {0})

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/tests/query_test/test_decimal_casting.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_decimal_casting.py b/tests/query_test/test_decimal_casting.py
index 445fe6b..3becc3f 100644
--- a/tests/query_test/test_decimal_casting.py
+++ b/tests/query_test/test_decimal_casting.py
@@ -81,9 +81,7 @@ class TestDecimalCasting(ImpalaTestSuite):
         expected, actual)
 
   def _normalize_cast_expr(self, decimal_val, precision, cast_from):
-    # Due to IMPALA-4936, casts from double which overflows decimal type don't work
-    # reliably. So casting from string for now until IMPALA-4936 is fixed.
-    if precision > 38 or cast_from == 'string':
+    if cast_from == 'string':
       return "select cast('{0}' as Decimal({1},{2}))"
     else:
       return "select cast({0} as Decimal({1},{2}))"
@@ -139,8 +137,12 @@ class TestDecimalCasting(ImpalaTestSuite):
       val = self._gen_decimal_val(from_precision, scale)
       cast = self._normalize_cast_expr(val, from_precision,\
           vector.get_value('cast_from')).format(val, precision, scale)
-      res = self.execute_scalar(cast)
-      self._assert_decimal_result(cast, res, 'NULL')
+      if vector.get_value('cast_from') == "string":
+        # TODO: This should be an error in both cases (IMPALA-6405).
+        res = self.execute_scalar(cast)
+        self._assert_decimal_result(cast, res, 'NULL')
+      else:
+        res = self.execute_query_expect_failure(self.client, cast)
 
   def test_underflow(self, vector):
     """Test to verify that we truncate when the scale of the number being cast is higher

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/tests/query_test/test_mem_usage_scaling.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_mem_usage_scaling.py b/tests/query_test/test_mem_usage_scaling.py
index efc500b..3a9f5ee 100644
--- a/tests/query_test/test_mem_usage_scaling.py
+++ b/tests/query_test/test_mem_usage_scaling.py
@@ -289,4 +289,5 @@ class TestTpcdsMemLimitError(TestLowMemoryLimits):
         v.get_value('table_format').file_format in ['parquet'])
 
   def test_low_mem_limit_q53(self, vector):
-    self.low_memory_limit_test(vector, 'tpcds-q53', self.MIN_MEM_FOR_TPCDS['q53'])
+    self.low_memory_limit_test(
+        vector, 'tpcds-decimal_v2-q53', self.MIN_MEM_FOR_TPCDS['q53'])

http://git-wip-us.apache.org/repos/asf/impala/blob/0a1d586d/tests/query_test/test_tpcds_queries.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_tpcds_queries.py b/tests/query_test/test_tpcds_queries.py
index 9cec9f7..4b937b1 100644
--- a/tests/query_test/test_tpcds_queries.py
+++ b/tests/query_test/test_tpcds_queries.py
@@ -36,6 +36,7 @@ class TestTpcdsQuery(ImpalaTestSuite):
         v.get_value('table_format').file_format not in ['rc', 'hbase', 'kudu'] and\
         v.get_value('table_format').compression_codec in ['none', 'snap'] and\
         v.get_value('table_format').compression_type != 'record')
+    cls.ImpalaTestMatrix.add_mandatory_exec_option('decimal_v2', 0)
 
     if cls.exploration_strategy() != 'exhaustive':
       # Cut down on the execution time for these tests in core by running only
@@ -277,7 +278,6 @@ class TestTpcdsDecimalV2Query(ImpalaTestSuite):
         v.get_value('table_format').file_format not in ['rc', 'hbase', 'kudu'] and\
         v.get_value('table_format').compression_codec in ['none', 'snap'] and\
         v.get_value('table_format').compression_type != 'record')
-    cls.ImpalaTestMatrix.add_mandatory_exec_option('decimal_v2', 1)
 
     if cls.exploration_strategy() != 'exhaustive':
       # Cut down on the execution time for these tests in core by running only