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/03/23 21:54:15 UTC

[3/6] impala git commit: IMPALA-6622: Backport parts of IMPALA-4924 to 2.x

IMPALA-6622: Backport parts of IMPALA-4924 to 2.x

We enabled Decimal V2 by default on master (but not on the 2.x branch)
in IMPALA-4924. There were some other code changes that are not
specific to enableing Decimal V2 that are causing merge conflicts. In
this patch, we backport those changes to reduce the chance of
conflicts.

Change-Id: I2504fb6ec7fa350296b058156b6fd7fb97bc4f9b
Reviewed-on: http://gerrit.cloudera.org:8080/9768
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/dee0ec0e
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/dee0ec0e
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/dee0ec0e

Branch: refs/heads/2.x
Commit: dee0ec0e74d7a740bea9e9e16403b4d7483f4515
Parents: 308e734
Author: Taras Bobrovytsky <tb...@cloudera.com>
Authored: Fri Jan 12 14:05:08 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Mar 23 04:10:54 2018 +0000

----------------------------------------------------------------------
 be/src/exprs/expr-test.cc                       | 78 +++++++++++++++++++-
 .../impala/analysis/AnalyzeExprsTest.java       | 65 ++++++++++------
 .../queries/QueryTest/exprs.test                |  3 +-
 tests/hs2/test_hs2.py                           |  5 +-
 tests/query_test/test_aggregation.py            |  8 +-
 tests/query_test/test_decimal_casting.py        |  4 +-
 6 files changed, 127 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/dee0ec0e/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index c183273..bd25328 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -5308,7 +5308,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);
@@ -7948,7 +7949,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))",
@@ -8017,6 +8020,77 @@ 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).
+  TestError("cast(regexp_replace('','a','b') as decimal(15,2))");
+
+  executor_->PopExecOption();
 }
 
 TEST_F(ExprTest, NullValueFunction) {

http://git-wip-us.apache.org/repos/asf/impala/blob/dee0ec0e/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 f3e2294..a20d643 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
@@ -2215,14 +2215,35 @@ 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) {
+    TQueryOptions queryOpts = new TQueryOptions();
+
+    queryOpts.setDecimal_v2(false);
+    SelectStmt selectStmt =
+        (SelectStmt) AnalyzesOk("select " + expr, createAnalysisCtx(queryOpts));
     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));
+
+    queryOpts.setDecimal_v2(true);
+    selectStmt = (SelectStmt) AnalyzesOk("select " + expr, createAnalysisCtx(queryOpts));
+    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
@@ -2241,7 +2262,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,
@@ -2252,7 +2273,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,
@@ -2263,7 +2284,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,
@@ -2271,31 +2292,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,
@@ -2312,22 +2333,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/dee0ec0e/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 e9c6d97..2902c97 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
@@ -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/dee0ec0e/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/dee0ec0e/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/dee0ec0e/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..684b86f 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}))"