You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by he...@apache.org on 2017/02/14 21:53:09 UTC
[1/4] incubator-impala git commit: Qualify min() in header
Repository: incubator-impala
Updated Branches:
refs/heads/master 1f80396b2 -> 894bb7785
Qualify min() in header
Change-Id: I840869bc2b8ffebc34f4bf4bbefe89976d4e54f2
Reviewed-on: http://gerrit.cloudera.org:8080/5991
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/065bbda9
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/065bbda9
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/065bbda9
Branch: refs/heads/master
Commit: 065bbda991adad716c7ff77c80245ca48d0c8ba4
Parents: 1f80396
Author: Henry Robinson <he...@cloudera.com>
Authored: Thu Feb 9 17:45:16 2017 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Feb 14 09:07:51 2017 +0000
----------------------------------------------------------------------
be/src/exprs/anyval-util.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/065bbda9/be/src/exprs/anyval-util.h
----------------------------------------------------------------------
diff --git a/be/src/exprs/anyval-util.h b/be/src/exprs/anyval-util.h
index 429322a..dfcf1ec 100644
--- a/be/src/exprs/anyval-util.h
+++ b/be/src/exprs/anyval-util.h
@@ -18,6 +18,8 @@
#ifndef IMPALA_EXPRS_ANYVAL_UTIL_H
#define IMPALA_EXPRS_ANYVAL_UTIL_H
+#include <algorithm>
+
#include "runtime/runtime-state.h"
#include "runtime/string-value.inline.h"
#include "runtime/timestamp-value.h"
@@ -218,7 +220,7 @@ class AnyValUtil {
static void TruncateIfNecessary(const FunctionContext::TypeDesc& type, StringVal *val) {
if (type.type == FunctionContext::TYPE_VARCHAR) {
DCHECK(type.len >= 0);
- val->len = min(val->len, type.len);
+ val->len = std::min(val->len, type.len);
}
}
[2/4] incubator-impala git commit: IMPALA-4370: Divide and modulo
result types for DECIMAL version V2
Posted by he...@apache.org.
IMPALA-4370: Divide and modulo result types for DECIMAL version V2
Implement the new DECIMAL return type rules for divide and modulo
expressions, active when query option DECIMAL_V2=1. See the comment
in the code for more details. A couple of examples that show why new
return type rules for divide are desirable.
For modulo, the return types are actually equivalent, though the
rules are expressed differently to have consistency with how
precision fixups are handled for each version.
DECIMAL Version 1:
+-------------------------------------------------------+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-----------------------------------------------------+
| 0 |
+-------------------------------------------------------+
DECIMAL Version 2:
+-------------------------------------------------------+
| cast(1 as decimal(20,0)) / cast(3 as decimal(20,0)) |
+-----------------------------------------------------+
| 0.333333333333333333 |
+-------------------------------------------------------+
DECIMAL Version 1:
+-------------------------------------------------------+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+-------------------------------------------------------+
| NULL |
+-------------------------------------------------------+
WARNINGS: UDF WARNING: Expression overflowed, returning NULL
DECIMAL Version 2:
+-------------------------------------------------------+
| cast(1 as decimal(6,0)) / cast(0.1 as decimal(38,38)) |
+-------------------------------------------------------+
| 10.000000 |
+-------------------------------------------------------+
Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b
Reviewed-on: http://gerrit.cloudera.org:8080/5952
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/a53eeb20
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/a53eeb20
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/a53eeb20
Branch: refs/heads/master
Commit: a53eeb2068970ff7575f4f3fdf3c383861a75f2d
Parents: 065bbda
Author: Dan Hecht <dh...@cloudera.com>
Authored: Tue Feb 7 12:08:54 2017 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Feb 14 18:40:54 2017 +0000
----------------------------------------------------------------------
be/src/exprs/expr-test.cc | 203 +++++++++++++------
be/src/runtime/decimal-value.inline.h | 8 +-
be/src/testutil/impalad-query-executor.h | 6 +-
.../apache/impala/analysis/ArithmeticExpr.java | 3 +-
.../impala/analysis/FunctionCallExpr.java | 2 +-
.../org/apache/impala/analysis/TypesUtil.java | 93 +++++++--
.../org/apache/impala/catalog/ScalarType.java | 54 ++++-
.../java/org/apache/impala/catalog/Type.java | 7 +-
.../queries/QueryTest/decimal-exprs.test | 73 +++++++
.../queries/QueryTest/decimal.test | 44 ----
tests/query_test/test_decimal_queries.py | 29 ++-
11 files changed, 374 insertions(+), 148 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index 1b2dc51..52e74c5 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -1287,89 +1287,164 @@ struct DecimalExpectedResult {
};
struct DecimalTestCase {
+
+ // Return the expected result for the given DECIMAL version. When the expected V1 and
+ // V2 results are the same, allows the V2 expected result to be unspecified in the
+ // test case table. When V2 expected results aren't specified, V2 entries have both
+ // 'null' set to false and 'precision' set to 0, which is an illegal combination and
+ // signals that V1 results should be used instead.
+ const DecimalExpectedResult& Expected(bool v2) const {
+ if (v2 && (expected[1].null || expected[1].precision != 0)) return expected[1];
+ // Either testing version 1, or the results for version 2 were not specified.
+ return expected[0];
+ }
+
+ // Expression to execute.
string expr;
- DecimalExpectedResult expected[2]; // Version 1 and Version 2
+
+ // Expected results for version 1 and Version 2. Version 2 results can be left unset
+ // when the expected result is the same between versions.
+ DecimalExpectedResult expected[2];
};
// Format is:
// { Test Expression (as a string),
-// { Decimal V1 expected results, Decimal V2 expected results } }
+// { expected null, scaled_val, precision, scale for V1
+// expected null, scaled_val, precision, scale for V2 }}
DecimalTestCase decimal_cases[] = {
- { "1.23 + cast(1 as decimal(4,3))",
- {{ false, 2230, 5, 3 }, { false, 2230, 5, 3 }} },
- { "1.23 - cast(0.23 as decimal(10,3))",
- {{ false, 1000, 11, 3 }, { false, 1000, 11, 3 }} },
- { "1.23 * cast(1 as decimal(20,3))",
- {{ false, 123000, 23, 5 }, { false, 123000, 23, 5 }} },
- { "cast(1.23 as decimal(8,2)) / cast(1 as decimal(4,3))",
- {{ false, 12300000, 16, 7}, { false, 12300000, 16, 7}} },
- { "cast(1.23 as decimal(8,2)) % cast(1 as decimal(10,3))",
- {{ false, 230, 9, 3 }, { false, 230, 9, 3 }} },
- { "cast(1.23 as decimal(8,2)) + cast(1 as decimal(20,3))",
- {{ false, 2230, 21, 3 }, { false, 2230, 21, 3 }} },
- { "cast(1.23 as decimal(30,2)) - cast(1 as decimal(4,3))",
- {{ false, 230, 32, 3 }, { false, 230, 32, 3 }} },
- { "cast(1.23 as decimal(30,2)) * cast(1 as decimal(10,3))",
- {{ false, 123000, 38, 5 }, { false, 123000, 38, 5 }} },
+ // Test add/subtract operators
+ { "1.23 + cast(1 as decimal(4,3))", {{ false, 2230, 5, 3 }}},
+ { "1.23 - cast(0.23 as decimal(10,3))", {{ false, 1000, 11, 3 }}},
+ { "cast(1.23 as decimal(8,2)) + cast(1 as decimal(20,3))", {{ false, 2230, 21, 3 }}},
+ { "cast(1.23 as decimal(30,2)) - cast(1 as decimal(4,3))", {{ false, 230, 32, 3 }}},
+ { "cast(1 as decimal(38,0)) + cast(.2 as decimal(38,1))", {{ false, 12, 38, 1 }}},
+ // Test multiply operator
+ { "cast(1.23 as decimal(30,2)) * cast(1 as decimal(10,3))", {{ false, 123000, 38, 5 }}},
+ { "1.23 * cast(1 as decimal(20,3))", {{ false, 123000, 23, 5 }}},
+ // Test divide operator
+ { "cast(1.23 as decimal(8,2)) / cast(1 as decimal(4,3))", {{ false, 12300000, 16, 7}}},
{ "cast(1.23 as decimal(30,2)) / cast(1 as decimal(20,3))",
- {{ false, 1230, 38, 3 }, { false, 1230, 38, 3 }} },
- { "cast(1 as decimal(38,0)) + cast(.2 as decimal(38,1))",
- {{ false, 12, 38, 1 }, { false, 12, 38, 1 }} },
+ {{ false, 1230, 38, 3 },
+ { false, 12300000, 38, 7 }}},
{ "cast(1 as decimal(38,0)) / cast(.2 as decimal(38,1))",
- {{ false, 50, 38, 1 }, { false, 50, 38, 1 }} },
- { "mod(cast('1' as decimal(2,0)), cast('10' as decimal(2,0)))",
- {{ false, 1, 2, 0 }, { false, 1, 2, 0 }} },
- { "mod(cast('1.1' as decimal(2,1)), cast('1.0' as decimal(2,1)))",
- {{ false, 1, 2,1 }, { false, 1, 2,1 }} },
+ {{ false, 50, 38, 1 },
+ { false, 5000000, 38, 6 }}},
+ { "cast(1 as decimal(38,0)) / cast(3 as decimal(38,0))",
+ {{ false, 0, 38, 0 },
+ { false, 333333, 38, 6 }}},
+ { "cast(99999999999999999999999999999999999999 as decimal(38,0)) / "
+ "cast(99999999999999999999999999999999999999 as decimal(38,0))",
+ {{ false, 1, 38, 0 },
+ { false, 1000000, 38, 6 }}},
+ { "cast(99999999999999999999999999999999999999 as decimal(38,0)) / "
+ "cast(0.00000000000000000000000000000000000001 as decimal(38,38))",
+ {{ true, 0, 38, 38 },
+ { true, 0, 38, 6 }}},
+ { "cast(0.00000000000000000000000000000000000001 as decimal(38,38)) / "
+ "cast(99999999999999999999999999999999999999 as decimal(38,0))",
+ {{ false, 0, 38, 38 }}},
+ { "cast(0.00000000000000000000000000000000000001 as decimal(38,38)) / "
+ "cast(0.00000000000000000000000000000000000001 as decimal(38,38))",
+ {{ true, 0, 38, 38 },
+ { false, 1000000, 38, 6 }}},
+ { "cast(9999999999999999999.9999999999999999999 as decimal(38,19)) / "
+ "cast(99999999999999999999999999999.999999999 as decimal(38,9))",
+ {{ false, 1000000000, 38, 19 },
+ { false, 1, 38, 10 }}},
+ { "cast(999999999999999999999999999999999999.99 as decimal(38,2)) / "
+ "cast(99999999999.999999999999999999999999999 as decimal(38,27))",
+ {{ true, 0, 38, 27 },
+ { false, static_cast<int128_t>(10) * 10000000000ll *
+ 10000000000ll * 10000000000ll, 38, 6 }}},
+ { "cast(-2.12 as decimal(17,2)) / cast(12515.95 as decimal(17,2))",
+ {{ false, -16938386618674571, 37, 20 }}},
+ { "cast(-2.12 as decimal(18,2)) / cast(12515.95 as decimal(18,2))",
+ {{ false, 0, 38, 2 },
+ { false, -16938386618674571, 38, 20 }}},
+ { "cast(737373 as decimal(6,0)) / cast(.52525252 as decimal(38,38))",
+ {{ true, 0, 38, 38 },
+ { false, 1403844764038, 38, 6 }}},
+ { "cast(0.000001 as decimal(6,6)) / "
+ "cast(0.0000000000000000000000000000000000001 as decimal(38,38))",
+ {{ true, 0, 38, 38 },
+ { false, static_cast<int128_t>(10000000ll) *
+ 10000000000ll * 10000000000ll * 10000000000ll, 38, 6 }}},
+ { "cast(98765432109876543210 as decimal(20,0)) / "
+ "cast(98765432109876543211 as decimal(20,0))",
+ {{ false, 0, 38, 0 },
+ { false, 999999999999999999, 38, 18 }}},
+ { "cast(111111.1111 as decimal(20, 10)) / cast(.7777 as decimal(38, 38))",
+ {{ true, 0, 38, 38 },
+ { false, 142871429985, 38, 6 }}},
+ // Test modulo operator
+ { "cast(1.23 as decimal(8,2)) % cast(1 as decimal(10,3))", {{ false, 230, 9, 3 }}},
+ { "cast(1 as decimal(38,0)) % cast(.2 as decimal(38,1))", {{ false, 0, 38, 1 }}},
+ { "cast(1 as decimal(38,0)) % cast(3 as decimal(38,0))", {{ false, 1, 38, 0 }}},
+ { "cast(-2.12 as decimal(17,2)) % cast(12515.95 as decimal(17,2))",
+ {{ false, -212, 17, 2 }}},
+ { "cast(-2.12 as decimal(18,2)) % cast(12515.95 as decimal(18,2))",
+ {{ false, -212, 18, 2 }}},
+ { "cast(99999999999999999999999999999999999999 as decimal(38,0)) % "
+ "cast(99999999999999999999999999999999999999 as decimal(38,0))",
+ {{ false, 0, 38, 0 }}},
+ { "cast(998 as decimal(38,0)) % cast(0.999 as decimal(38,38))", {{ false, 0, 38, 38 }}},
+ { "cast(0.998 as decimal(38,38)) % cast(999 as decimal(38,0))", {{ false, 0, 38, 38 }}},
+ { "cast(0.00000000000000000000000000000000000001 as decimal(38,38)) % "
+ "cast(0.0000000000000000000000000000000000001 as decimal(38,38))",
+ {{ false, 1, 38, 38 }}},
+ // Test MOD builtin
+ { "mod(cast('1' as decimal(2,0)), cast('10' as decimal(2,0)))", {{ false, 1, 2, 0 }}},
+ { "mod(cast('1.1' as decimal(2,1)), cast('1.0' as decimal(2,1)))", {{ false, 1, 2, 1 }}},
{ "mod(cast('-1.23' as decimal(5,2)), cast('1.0' as decimal(5,2)))",
- {{ false, -23, 5,2 }, { false, -23, 5,2 }} },
- { "mod(cast('1' as decimal(12,0)), cast('10' as decimal(12,0)))",
- {{ false, 1, 12, 0 }, { false, 1, 12, 0 }} },
+ {{ false, -23, 5, 2 }}},
+ { "mod(cast('1' as decimal(12,0)), cast('10' as decimal(12,0)))", {{ false, 1, 12, 0 }}},
{ "mod(cast('1.1' as decimal(12,1)), cast('1.0' as decimal(12,1)))",
- {{ false, 1, 12, 1 }, { false, 1, 12, 1 }} },
+ {{ false, 1, 12, 1 }}},
{ "mod(cast('-1.23' as decimal(12,2)), cast('1.0' as decimal(12,2)))",
- {{ false, -23, 12, 2 }, { false, -23, 12, 2 }} },
+ {{ false, -23, 12, 2 }}},
{ "mod(cast('1' as decimal(32,0)), cast('10' as decimal(32,0)))",
- {{ false, 1, 32, 0 }, { false, 1, 32, 0 }} },
+ {{ false, 1, 32, 0 }}},
{ "mod(cast('1.1' as decimal(32,1)), cast('1.0' as decimal(32,1)))",
- {{ false, 1, 32, 1 }, { false, 1, 32, 1 }} },
+ {{ false, 1, 32, 1 }}},
{ "mod(cast('-1.23' as decimal(32,2)), cast('1.0' as decimal(32,2)))",
- {{ false, -23, 32, 2 }, { false, -23, 32, 2 }} },
+ {{ false, -23, 32, 2 }}},
+ { "mod(cast(NULL as decimal(2,0)), cast('10' as decimal(2,0)))", {{ true, 0, 2, 0 }}},
+ { "mod(cast('10' as decimal(2,0)), cast(NULL as decimal(2,0)))", {{ true, 0, 2, 0 }}},
+ { "mod(cast('10' as decimal(2,0)), cast('0' as decimal(2,0)))", {{ true, 0, 2, 0 }}},
+ { "mod(cast('10' as decimal(2,0)), cast('0' as decimal(2,0)))", {{ true, 0, 2, 0 }}},
+ { "mod(cast(NULL as decimal(2,0)), NULL)", {{ true, 0, 2, 0 }}},
+ // Test CAST DECIMAL -> DECIMAL
{ "cast(cast(0.12344 as decimal(6,5)) as decimal(6,4))",
- {{ false, 1234, 6, 4 }, { false, 1234, 6, 4 }} },
+ {{ false, 1234, 6, 4 }}},
{ "cast(cast(0.12345 as decimal(6,5)) as decimal(6,4))",
- {{ false, 1234, 6, 4 }, { false, 1235, 6, 4 }} },
+ {{ false, 1234, 6, 4 },
+ { false, 1235, 6, 4 }}},
{ "cast(cast('0.999' as decimal(4,3)) as decimal(1,0))",
- {{ false, 0, 1, 0 }, { false, 1, 1, 0 }} },
+ {{ false, 0, 1, 0 },
+ { false, 1, 1, 0 }}},
{ "cast(cast(999999999.99 as DECIMAL(11,2)) as DECIMAL(9,0))",
- {{ false, 999999999, 9, 0 }, { true, 0, 9, 0 }} },
+ {{ false, 999999999, 9, 0 },
+ { true, 0, 9, 0 }}},
{ "cast(cast(-999999999.99 as DECIMAL(11,2)) as DECIMAL(9,0))",
- {{ false, -999999999, 9, 0 }, { true, 0, 9, 0 }} },
- { "mod(cast(NULL as decimal(2,0)), cast('10' as decimal(2,0)))",
- {{ true, 0, 2, 0 }, { true, 0, 2, 0 }} },
- { "mod(cast('10' as decimal(2,0)), cast(NULL as decimal(2,0)))",
- {{ true, 0, 2, 0 }, { true, 0, 2, 0 }} },
- { "mod(cast('10' as decimal(2,0)), cast('0' as decimal(2,0)))",
- {{ true, 0, 2, 0 }, { true, 0, 2, 0 }} },
- { "mod(cast('10' as decimal(2,0)), cast('0' as decimal(2,0)))",
- {{ true, 0, 2, 0 }, { true, 0, 2, 0 }} },
- { "mod(cast(NULL as decimal(2,0)), NULL)",
- {{ true, 0, 2, 0 }, { true, 0, 2, 0 }} },
+ {{ false, -999999999, 9, 0 },
+ { true, 0, 9, 0 }}},
// IMPALA-2233: Test that implicit casts do not lose precision.
// The overload greatest(decimal(*,*)) is available and should be used.
{ "greatest(0, cast('99999.1111' as decimal(30,10)))",
- {{ false, 999991111000000, 30, 10 }, { false, 999991111000000, 30, 10 }} }
+ {{ false, 999991111000000, 30, 10 },
+ { false, 999991111000000, 30, 10 }}}
};
TEST_F(ExprTest, DecimalArithmeticExprs) {
// Test with both decimal_v2={false, true}
- for (int v2 = 0; v2 <= 1; ++v2) {
+ for (int v2: { 0, 1 }) {
string opt = "DECIMAL_V2=" + lexical_cast<string>(v2);
- executor_->pushExecOption(opt);
+ executor_->PushExecOption(opt);
for (const DecimalTestCase& c : decimal_cases) {
- const DecimalExpectedResult& r = c.expected[v2];
+ const DecimalExpectedResult& r = c.Expected(v2);
const ColumnType& type = ColumnType::CreateDecimalType(r.precision, r.scale);
if (r.null) {
+ TestDecimalResultType(c.expr, type);
TestIsNull(c.expr, type);
} else {
switch (type.GetByteSize()) {
@@ -1387,7 +1462,7 @@ TEST_F(ExprTest, DecimalArithmeticExprs) {
}
}
}
- executor_->popExecOption();
+ executor_->PopExecOption();
}
}
@@ -6252,19 +6327,19 @@ int main(int argc, char **argv) {
int ret;
disable_codegen_ = true;
enable_expr_rewrites_ = false;
- executor_->clearExecOptions();
- executor_->pushExecOption("ENABLE_EXPR_REWRITES=0");
- executor_->pushExecOption("DISABLE_CODEGEN=1");
+ executor_->ClearExecOptions();
+ executor_->PushExecOption("ENABLE_EXPR_REWRITES=0");
+ executor_->PushExecOption("DISABLE_CODEGEN=1");
cout << "Running without codegen" << endl;
ret = RUN_ALL_TESTS();
if (ret != 0) return ret;
disable_codegen_ = false;
enable_expr_rewrites_ = false;
- executor_->clearExecOptions();
- executor_->pushExecOption("ENABLE_EXPR_REWRITES=0");
- executor_->pushExecOption("DISABLE_CODEGEN=0");
- executor_->pushExecOption("EXEC_SINGLE_NODE_ROWS_THRESHOLD=0");
+ executor_->ClearExecOptions();
+ executor_->PushExecOption("ENABLE_EXPR_REWRITES=0");
+ executor_->PushExecOption("DISABLE_CODEGEN=0");
+ executor_->PushExecOption("EXEC_SINGLE_NODE_ROWS_THRESHOLD=0");
cout << endl << "Running with codegen" << endl;
ret = RUN_ALL_TESTS();
if (ret != 0) return ret;
@@ -6272,9 +6347,9 @@ int main(int argc, char **argv) {
// Enable FE expr rewrites to get test for constant folding over all exprs.
disable_codegen_ = true;
enable_expr_rewrites_ = true;
- executor_->clearExecOptions();
- executor_->pushExecOption("ENABLE_EXPR_REWRITES=1");
- executor_->pushExecOption("DISABLE_CODEGEN=1");
+ executor_->ClearExecOptions();
+ executor_->PushExecOption("ENABLE_EXPR_REWRITES=1");
+ executor_->PushExecOption("DISABLE_CODEGEN=1");
cout << endl << "Running without codegen and expr rewrites" << endl;
return RUN_ALL_TESTS();
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/be/src/runtime/decimal-value.inline.h
----------------------------------------------------------------------
diff --git a/be/src/runtime/decimal-value.inline.h b/be/src/runtime/decimal-value.inline.h
index 3a65a16..e7e89aa 100644
--- a/be/src/runtime/decimal-value.inline.h
+++ b/be/src/runtime/decimal-value.inline.h
@@ -228,15 +228,15 @@ template<typename RESULT_T>
inline DecimalValue<RESULT_T> DecimalValue<T>::Divide(int this_scale,
const DecimalValue& other, int other_scale, int result_precision, int result_scale,
bool* is_nan, bool* overflow) const {
- DCHECK_GE(result_scale, this_scale);
+ DCHECK_GE(result_scale + other_scale, this_scale);
if (other.value() == 0) {
// Divide by 0.
*is_nan = true;
return DecimalValue<RESULT_T>();
}
- // We need to scale x up by the result precision and then do an integer divide.
- // This truncates the result to the output precision.
- // TODO: confirm with standard that truncate is okay.
+ // We need to scale x up by the result scale and then do an integer divide.
+ // This truncates the result to the output scale.
+ // TODO: implement rounding when decimal_v2=true.
int scale_by = result_scale + other_scale - this_scale;
// Use higher precision ints for intermediates to avoid overflows. Divides lead to
// large numbers very quickly (and get eliminated by the int divide).
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/be/src/testutil/impalad-query-executor.h
----------------------------------------------------------------------
diff --git a/be/src/testutil/impalad-query-executor.h b/be/src/testutil/impalad-query-executor.h
index 89bed7e..c50ac33 100644
--- a/be/src/testutil/impalad-query-executor.h
+++ b/be/src/testutil/impalad-query-executor.h
@@ -84,15 +84,15 @@ class ImpaladQueryExecutor {
bool eos() { return eos_; }
/// Add a query option, preserving the existing set of query options.
- void pushExecOption(const std::string& exec_option) {
+ void PushExecOption(const std::string& exec_option) {
exec_options_.push_back(exec_option);
}
/// Remove the last query option that was added by pushExecOption().
- void popExecOption() { exec_options_.pop_back(); }
+ void PopExecOption() { exec_options_.pop_back(); }
/// Remove all query options previously added by pushExecOption().
- void clearExecOptions() { exec_options_.clear(); }
+ void ClearExecOptions() { exec_options_.clear(); }
private:
/// fe service-related
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java b/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
index 3e574b2..763a38d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
@@ -199,7 +199,8 @@ public class ArithmeticExpr extends Expr {
case DIVIDE:
case MULTIPLY:
case MOD:
- type_ = TypesUtil.getArithmeticResultType(t0, t1, op_);
+ type_ = TypesUtil.getArithmeticResultType(t0, t1, op_,
+ analyzer.getQueryOptions().isDecimal_v2());
// If both of the children are null, we'll default to the DOUBLE version of the
// operator. This prevents the BE from seeing NULL_TYPE.
if (type_.isNull()) type_ = Type.DOUBLE;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
index 4d7dca8..482a31e 100644
--- a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
@@ -386,7 +386,7 @@ public class FunctionCallExpr extends Expr {
}
}
Preconditions.checkState(returnType.isDecimal() && !returnType.isWildcardDecimal());
- return ScalarType.createDecimalTypeInternal(digitsBefore + digitsAfter, digitsAfter);
+ return ScalarType.createClippedDecimalType(digitsBefore + digitsAfter, digitsAfter);
}
@Override
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java b/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
index 4a61f38..b29fc05 100644
--- a/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
+++ b/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
@@ -87,8 +87,7 @@ public class TypesUtil {
int p2 = t2.decimalPrecision();
int digitsBefore = Math.max(p1 - s1, p2 - s2);
int digitsAfter = Math.max(s1, s2);
- return ScalarType.createDecimalTypeInternal(
- digitsBefore + digitsAfter, digitsAfter);
+ return ScalarType.createClippedDecimalType(digitsBefore + digitsAfter, digitsAfter);
}
/**
@@ -96,7 +95,7 @@ public class TypesUtil {
* if the operation does not make sense for the types.
*/
public static Type getArithmeticResultType(Type t1, Type t2,
- ArithmeticExpr.Operator op) throws AnalysisException {
+ ArithmeticExpr.Operator op, boolean decimal_v2) throws AnalysisException {
Preconditions.checkState(t1.isNumericType() || t1.isNull());
Preconditions.checkState(t2.isNumericType() || t2.isNull());
@@ -117,7 +116,7 @@ public class TypesUtil {
t2 = ((ScalarType) t2).getMinResolutionDecimal();
Preconditions.checkState(t1.isDecimal());
Preconditions.checkState(t2.isDecimal());
- return getDecimalArithmeticResultType(t1, t2, op);
+ return getDecimalArithmeticResultType(t1, t2, op, decimal_v2);
}
Type type = null;
@@ -147,15 +146,36 @@ public class TypesUtil {
}
/**
- * Returns the resulting typical type from (t1 op t2)
- * These rules are mostly taken from the hive/sql server rules with some changes.
+ * Returns the result type for (t1 op t2) where t1 and t2 are both DECIMAL, depending
+ * on whether DECIMAL version 1 or DECIMAL version 2 is enabled.
+ *
+ * TODO: IMPALA-4924: remove DECIMAL V1 code.
+ */
+ private static ScalarType getDecimalArithmeticResultType(Type t1, Type t2,
+ ArithmeticExpr.Operator op, boolean decimal_v2) throws AnalysisException {
+ if (decimal_v2) return getDecimalArithmeticResultTypeV2(t1, t2, op);
+ return getDecimalArithmeticResultTypeV1(t1, t2, op);
+ }
+
+ /**
+ * Returns the result type for (t1 op t2) where t1 and t2 are both DECIMAL, used when
+ * DECIMAL version 1 is enabled.
+ *
+ * These rules were mostly taken from the (pre Dec 2016) Hive / sql server rules with
+ * some changes.
* http://blogs.msdn.com/b/sqlprogrammability/archive/2006/03/29/564110.aspx
+ * https://msdn.microsoft.com/en-us/library/ms190476.aspx
*
* Changes:
* - Multiply does not need +1 for the result precision.
- * - Divide scale truncation is different.
+ * - Divide scale truncation is different. When the maximum precision is exceeded,
+ * unlike SQL server, we do not try to preserve at least a scale of 6.
+ * - For other operators, when maximum precision is exceeded, we clip the precision
+ * and scale at maximum precision rather tha reducing scale. Therefore, we
+ * sacrifice digits to the left of the decimal point before sacrificing digits to
+ * the right.
*/
- public static ScalarType getDecimalArithmeticResultType(Type t1, Type t2,
+ private static ScalarType getDecimalArithmeticResultTypeV1(Type t1, Type t2,
ArithmeticExpr.Operator op) throws AnalysisException {
Preconditions.checkState(t1.isFullySpecifiedDecimal());
Preconditions.checkState(t2.isFullySpecifiedDecimal());
@@ -170,10 +190,10 @@ public class TypesUtil {
switch (op) {
case ADD:
case SUBTRACT:
- return ScalarType.createDecimalTypeInternal(
+ return ScalarType.createClippedDecimalType(
sMax + Math.max(p1 - s1, p2 - s2) + 1, sMax);
case MULTIPLY:
- return ScalarType.createDecimalTypeInternal(p1 + p2, s1 + s2);
+ return ScalarType.createClippedDecimalType(p1 + p2, s1 + s2);
case DIVIDE:
int resultScale = Math.max(DECIMAL_DIVISION_SCALE_INCREMENT, s1 + p2 + 1);
int resultPrecision = p1 - s1 + s2 + resultScale;
@@ -187,9 +207,9 @@ public class TypesUtil {
resultScale = Math.max(s1, s2);
resultPrecision = ScalarType.MAX_PRECISION;
}
- return ScalarType.createDecimalTypeInternal(resultPrecision, resultScale);
+ return ScalarType.createClippedDecimalType(resultPrecision, resultScale);
case MOD:
- return ScalarType.createDecimalTypeInternal(
+ return ScalarType.createClippedDecimalType(
Math.min(p1 - s1, p2 - s2) + sMax, sMax);
default:
throw new AnalysisException(
@@ -198,6 +218,55 @@ public class TypesUtil {
}
/**
+ * Returns the result type for (t1 op t2) where t1 and t2 are both DECIMAL, used when
+ * DECIMAL version 2 is enabled.
+ *
+ * These rules are similar to (post Dec 2016) Hive / sql server rules.
+ * http://blogs.msdn.com/b/sqlprogrammability/archive/2006/03/29/564110.aspx
+ * https://msdn.microsoft.com/en-us/library/ms190476.aspx
+ *
+ * TODO: implement V2 rules for ADD/SUB/MULTIPLY.
+ *
+ * Changes:
+ * - There are slight difference with how precision/scale reduction occurs compared
+ * to SQL server when the desired precision is more than the maximum supported
+ * precision. But an algorithm of reducing scale to a minimum of 6 is used.
+ */
+ private static ScalarType getDecimalArithmeticResultTypeV2(Type t1, Type t2,
+ ArithmeticExpr.Operator op) throws AnalysisException {
+ Preconditions.checkState(t1.isFullySpecifiedDecimal());
+ Preconditions.checkState(t2.isFullySpecifiedDecimal());
+ ScalarType st1 = (ScalarType) t1;
+ ScalarType st2 = (ScalarType) t2;
+ int s1 = st1.decimalScale();
+ int s2 = st2.decimalScale();
+ int p1 = st1.decimalPrecision();
+ int p2 = st2.decimalPrecision();
+ int resultScale;
+ int resultPrecision;
+
+ switch (op) {
+ case DIVIDE:
+ // Divide result always gets at least MIN_ADJUSTED_SCALE decimal places.
+ resultScale = Math.max(ScalarType.MIN_ADJUSTED_SCALE, s1 + p2 + 1);
+ resultPrecision = p1 - s1 + s2 + resultScale;
+ break;
+ case MOD:
+ resultScale = Math.max(s1, s2);
+ resultPrecision = Math.min(p1 - s1, p2 - s2) + resultScale;
+ break;
+ case ADD:
+ case SUBTRACT:
+ case MULTIPLY:
+ default:
+ // Not yet implemented - fall back to V1 rules.
+ return getDecimalArithmeticResultTypeV1(t1, t2, op);
+ }
+ // Use the scale reduction technique when resultPrecision is too large.
+ return ScalarType.createAdjustedDecimalType(resultPrecision, resultScale);
+ }
+
+ /**
* Computes the ColumnType that can represent 'v' with no loss of resolution.
* The scale/precision in BigDecimal is not compatible with SQL decimal semantics
* (much more like significant figures and exponent).
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/fe/src/main/java/org/apache/impala/catalog/ScalarType.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/ScalarType.java b/fe/src/main/java/org/apache/impala/catalog/ScalarType.java
index 052fd24..297f90e 100644
--- a/fe/src/main/java/org/apache/impala/catalog/ScalarType.java
+++ b/fe/src/main/java/org/apache/impala/catalog/ScalarType.java
@@ -66,6 +66,7 @@ public class ScalarType extends Type {
// Hive, mysql, sql server standard.
public static final int MAX_PRECISION = 38;
public static final int MAX_SCALE = MAX_PRECISION;
+ public static final int MIN_ADJUSTED_SCALE = 6;
protected ScalarType(PrimitiveType type) {
type_ = type;
@@ -107,6 +108,9 @@ public class ScalarType extends Type {
return createDecimalType(precision, DEFAULT_SCALE);
}
+ /**
+ * Returns a DECIMAL type with the specified precision and scale.
+ */
public static ScalarType createDecimalType(int precision, int scale) {
Preconditions.checkState(precision >= 0); // Enforced by parser
Preconditions.checkState(scale >= 0); // Enforced by parser.
@@ -116,16 +120,50 @@ public class ScalarType extends Type {
return type;
}
- // Identical to createDecimalType except that higher precisions are truncated
- // to the max storable precision. The BE will report overflow in these cases
- // (think of this as adding ints to BIGINT but BIGINT can still overflow).
- public static ScalarType createDecimalTypeInternal(int precision, int scale) {
+ /**
+ * Returns a DECIMAL wildcard type (i.e. precision and scale hasn't yet been resolved).
+ */
+ public static ScalarType createWildCardDecimalType() {
+ ScalarType type = new ScalarType(PrimitiveType.DECIMAL);
+ type.precision_ = -1;
+ type.scale_ = -1;
+ return type;
+ }
+
+ /**
+ * Returns a DECIMAL type with the specified precision and scale, but truncating the
+ * precision to the max storable precision (i.e. removes digits from before the
+ * decimal point).
+ */
+ public static ScalarType createClippedDecimalType(int precision, int scale) {
+ Preconditions.checkState(precision >= 0);
+ Preconditions.checkState(scale >= 0);
ScalarType type = new ScalarType(PrimitiveType.DECIMAL);
type.precision_ = Math.min(precision, MAX_PRECISION);
type.scale_ = Math.min(type.precision_, scale);
return type;
}
+ /**
+ * Returns a DECIMAL type with the specified precision and scale. When the given
+ * precision exceeds the max storable precision, reduce both precision and scale but
+ * preserve at least MIN_ADJUSTED_SCALE for scale (unless the desired scale was less).
+ */
+ public static ScalarType createAdjustedDecimalType(int precision, int scale) {
+ Preconditions.checkState(precision >= 0);
+ Preconditions.checkState(scale >= 0);
+ if (precision > MAX_PRECISION) {
+ int minScale = Math.min(scale, MIN_ADJUSTED_SCALE);
+ int delta = precision - MAX_PRECISION;
+ precision = MAX_PRECISION;
+ scale = Math.max(scale - delta, minScale);
+ }
+ ScalarType type = new ScalarType(PrimitiveType.DECIMAL);
+ type.precision_ = precision;
+ type.scale_ = scale;
+ return type;
+ }
+
public static ScalarType createVarcharType(int len) {
// length checked in analysis
ScalarType type = new ScalarType(PrimitiveType.VARCHAR);
@@ -336,7 +374,7 @@ public class ScalarType extends Type {
} else if (isNull()) {
return ScalarType.NULL;
} else if (isDecimal()) {
- return createDecimalTypeInternal(MAX_PRECISION, scale_);
+ return createClippedDecimalType(MAX_PRECISION, scale_);
} else {
return ScalarType.INVALID;
}
@@ -347,7 +385,7 @@ public class ScalarType extends Type {
if (type_ == PrimitiveType.DOUBLE || type_ == PrimitiveType.BIGINT || isNull()) {
return this;
} else if (type_ == PrimitiveType.DECIMAL) {
- return createDecimalTypeInternal(MAX_PRECISION, scale_);
+ return createClippedDecimalType(MAX_PRECISION, scale_);
}
return createType(PrimitiveType.values()[type_.ordinal() + 1]);
}
@@ -364,8 +402,8 @@ public class ScalarType extends Type {
case SMALLINT: return createDecimalType(5);
case INT: return createDecimalType(10);
case BIGINT: return createDecimalType(19);
- case FLOAT: return createDecimalTypeInternal(MAX_PRECISION, 9);
- case DOUBLE: return createDecimalTypeInternal(MAX_PRECISION, 17);
+ case FLOAT: return createDecimalType(MAX_PRECISION, 9);
+ case DOUBLE: return createDecimalType(MAX_PRECISION, 17);
default: return ScalarType.INVALID;
}
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/fe/src/main/java/org/apache/impala/catalog/Type.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/Type.java b/fe/src/main/java/org/apache/impala/catalog/Type.java
index 05c71c7..45848c5 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Type.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Type.java
@@ -62,14 +62,13 @@ public abstract class Type {
public static final ScalarType TIMESTAMP = new ScalarType(PrimitiveType.TIMESTAMP);
public static final ScalarType DATE = new ScalarType(PrimitiveType.DATE);
public static final ScalarType DATETIME = new ScalarType(PrimitiveType.DATETIME);
- public static final ScalarType DEFAULT_DECIMAL = (ScalarType)
+ public static final ScalarType DEFAULT_DECIMAL =
ScalarType.createDecimalType(ScalarType.DEFAULT_PRECISION,
ScalarType.DEFAULT_SCALE);
- public static final ScalarType DECIMAL =
- (ScalarType) ScalarType.createDecimalTypeInternal(-1, -1);
+ public static final ScalarType DECIMAL = ScalarType.createWildCardDecimalType();
public static final ScalarType DEFAULT_VARCHAR = ScalarType.createVarcharType(-1);
public static final ScalarType VARCHAR = ScalarType.createVarcharType(-1);
- public static final ScalarType CHAR = (ScalarType) ScalarType.createCharType(-1);
+ public static final ScalarType CHAR = ScalarType.createCharType(-1);
private static ArrayList<ScalarType> integerTypes;
private static ArrayList<ScalarType> numericTypes;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/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
new file mode 100644
index 0000000..89385bd
--- /dev/null
+++ b/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
@@ -0,0 +1,73 @@
+====
+---- QUERY
+# Test DECIMAL V1 divide result type
+set decimal_v2=false;
+select d1 / d2, d2 / d1, d3 / d4, d5 / d3, d3 / d5 from decimal_tbl;
+---- RESULTS
+0.55535553555,1.8006482982,NULL,10000.0891810008,0.000099999108197945064
+21.12612612612,0.0473347547,NULL,0.2544210023,3.930493123209169054441
+37.07207207207,0.0269744835,NULL,0.0000810000,12345.678900000000000000000
+37.07207207207,0.0269744835,NULL,0.0908820008,11.003278877005347593582
+398.92492492492,0.0025067373,NULL,0.0000630900,15850.349728459731155875669
+---- TYPES
+DECIMAL, DECIMAL, DECIMAL, DECIMAL, DECIMAL
+====
+---- QUERY
+# Verify DECIMAL V2. Differences with V1:
+# * d3/d4 does not overflow
+# * d5/d3 has more scale
+set decimal_v2=true;
+select d1 / d2, d2 / d1, d3 / d4, d5 / d3, d3 / d5 from decimal_tbl;
+---- RESULTS
+0.55535553555,1.8006482982,10.000000,10000.08918100081154710738507,0.000099999108197945064
+21.12612612612,0.0473347547,100.000000,0.25442100231523112106860,3.930493123209169054441
+37.07207207207,0.0269744835,1000.000000,0.09088200082702620752593,11.003278877005347593582
+37.07207207207,0.0269744835,10000.000000,0.00008100000073710000670,12345.678900000000000000000
+398.92492492492,0.0025067373,100000.000000,0.00006309009057411982422,15850.349728459731155875669
+---- TYPES
+DECIMAL, DECIMAL, DECIMAL, DECIMAL, DECIMAL
+====
+---- QUERY
+# Test casting behavior without decimal_v2 query option set.
+set decimal_v2=false;
+select cast(d3 as decimal(20, 3)) from functional.decimal_tbl;
+---- RESULTS
+1.234
+12.345
+123.456
+1234.567
+12345.678
+---- TYPES
+DECIMAL
+====
+---- QUERY
+# Test casting behavior with decimal_v2 query option set.
+set decimal_v2=true;
+select cast(d3 as decimal(20, 3)) from functional.decimal_tbl;
+---- RESULTS
+1.235
+12.346
+123.457
+1234.568
+12345.679
+---- TYPES
+DECIMAL
+====
+---- QUERY
+# Test casting behavior without decimal_v2 query option set.
+set decimal_v2=false;
+select sum(cast(d3 as DECIMAL(20,2)) + cast(d5 as DECIMAL(20,4))) from functional.decimal_tbl;
+---- RESULTS
+26078.2788
+---- TYPES
+DECIMAL
+====
+---- QUERY
+# Test casting behavior with decimal_v2 query option set.
+set decimal_v2=true;
+select sum(cast(d3 as DECIMAL(20,2)) + cast(d5 as DECIMAL(20,4))) from functional.decimal_tbl;
+---- RESULTS
+26078.3189
+---- TYPES
+DECIMAL
+====
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/testdata/workloads/functional-query/queries/QueryTest/decimal.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/decimal.test b/testdata/workloads/functional-query/queries/QueryTest/decimal.test
index 4a1316b..6e7cbae 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/decimal.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/decimal.test
@@ -449,47 +449,3 @@ NULL,0.0000
---- TYPES
DECIMAL, DECIMAL
====
----- QUERY
-# Test casting behavior without decimal_v2 query option set.
-set decimal_v2=false;
-select cast(d3 as decimal(20, 3)) from functional.decimal_tbl;
----- RESULTS
-1.234
-12.345
-123.456
-1234.567
-12345.678
----- TYPES
-DECIMAL
-====
----- QUERY
-# Test casting behavior with decimal_v2 query option set.
-set decimal_v2=true;
-select cast(d3 as decimal(20, 3)) from functional.decimal_tbl;
----- RESULTS
-1.235
-12.346
-123.457
-1234.568
-12345.679
----- TYPES
-DECIMAL
-====
----- QUERY
-# Test casting behavior without decimal_v2 query option set.
-set decimal_v2=false;
-select sum(cast(d3 as DECIMAL(20,2)) + cast(d5 as DECIMAL(20,4))) from functional.decimal_tbl;
----- RESULTS
-26078.2788
----- TYPES
-DECIMAL
-====
----- QUERY
-# Test casting behavior with decimal_v2 query option set.
-set decimal_v2=true;
-select sum(cast(d3 as DECIMAL(20,2)) + cast(d5 as DECIMAL(20,4))) from functional.decimal_tbl;
----- RESULTS
-26078.3189
----- TYPES
-DECIMAL
-====
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/a53eeb20/tests/query_test/test_decimal_queries.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_decimal_queries.py b/tests/query_test/test_decimal_queries.py
index 80caf56..036d879 100644
--- a/tests/query_test/test_decimal_queries.py
+++ b/tests/query_test/test_decimal_queries.py
@@ -20,11 +20,10 @@
from copy import copy
from tests.common.impala_test_suite import ImpalaTestSuite
+from tests.common.test_dimensions import create_exec_option_dimension_from_dict
from tests.common.test_vector import ImpalaTestDimension
class TestDecimalQueries(ImpalaTestSuite):
- BATCH_SIZES = [0, 1]
-
@classmethod
def get_workload(cls):
return 'functional-query'
@@ -33,8 +32,9 @@ class TestDecimalQueries(ImpalaTestSuite):
def add_test_dimensions(cls):
super(TestDecimalQueries, cls).add_test_dimensions()
cls.ImpalaTestMatrix.add_dimension(
- ImpalaTestDimension('batch_size', *TestDecimalQueries.BATCH_SIZES))
-
+ create_exec_option_dimension_from_dict({
+ 'decimal_v2' : ['false', 'true'],
+ 'batch_size' : [0, 1]}))
# Hive < 0.11 does not support decimal so we can't run these tests against the other
# file formats.
# TODO: Enable them on Hive >= 0.11.
@@ -44,9 +44,24 @@ class TestDecimalQueries(ImpalaTestSuite):
v.get_value('table_format').file_format == 'parquet')
def test_queries(self, vector):
- new_vector = copy(vector)
- new_vector.get_value('exec_option')['batch_size'] = vector.get_value('batch_size')
- self.run_test_case('QueryTest/decimal', new_vector)
+ self.run_test_case('QueryTest/decimal', vector)
+
+# Tests involving DECIMAL typed expressions. The results depend on whether DECIMAL
+# version 1 or version 2 are enabled, so the .test file itself toggles the DECIMAL_V2
+# query option.
+class TestDecimalExprs(ImpalaTestSuite):
+ @classmethod
+ def get_workload(cls):
+ return 'functional-query'
+
+ @classmethod
+ def add_test_dimensions(cls):
+ super(TestDecimalExprs, cls).add_test_dimensions()
+ cls.ImpalaTestMatrix.add_constraint(lambda v:
+ (v.get_value('table_format').file_format == 'parquet'))
+
+ def test_exprs(self, vector):
+ self.run_test_case('QueryTest/decimal-exprs', vector)
# TODO: when we have a good way to produce Avro decimal data (e.g. upgrade Hive), we can
# run Avro through the same tests as above instead of using avro_decimal_tbl.
[3/4] incubator-impala git commit: IMPALA-4920: custom cluster tests:
fix generation of py.test options
Posted by he...@apache.org.
IMPALA-4920: custom cluster tests: fix generation of py.test options
This patch fixes a problem in which unnecessary quote characters were
part of some impala-py.test arguments for running the custom cluster
tests. The bug was causing py.test metadata files as described by the
py.test options --junit-xml and --result-log to end up outside the
intended path for such artifacts. By removing the unnecessary escaped
quotes, custom cluster py.test metadata artifacts will now be in the
proper location. I doubly made sure it was safe to remove these quotes
by adding a space in the RESULTS_DIR. The space isn't expanded but is
instead properly treated as part of RESULTS_DIR.
Change-Id: If6a56bc6bca826a8b50340e5caea7687441899e6
Reviewed-on: http://gerrit.cloudera.org:8080/5978
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/9e36088c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/9e36088c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/9e36088c
Branch: refs/heads/master
Commit: 9e36088c8d2859bfb862aa19fc1c6b68172a195e
Parents: a53eeb2
Author: Michael Brown <mi...@cloudera.com>
Authored: Mon Feb 13 09:45:36 2017 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Feb 14 21:09:45 2017 +0000
----------------------------------------------------------------------
tests/run-custom-cluster-tests.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/9e36088c/tests/run-custom-cluster-tests.sh
----------------------------------------------------------------------
diff --git a/tests/run-custom-cluster-tests.sh b/tests/run-custom-cluster-tests.sh
index 72108f6..ce161b1 100755
--- a/tests/run-custom-cluster-tests.sh
+++ b/tests/run-custom-cluster-tests.sh
@@ -41,8 +41,8 @@ if [[ -d "${AUX_CUSTOM_DIR}" ]]
then
ARGS+=("${AUX_CUSTOM_DIR}")
fi
-ARGS+=("--junitxml=\"${RESULTS_DIR}/TEST-impala-custom-cluster.xml\"")
-ARGS+=("--resultlog=\"${RESULTS_DIR}/TEST-impala-custom-cluster.log\"")
+ARGS+=("--junitxml=${RESULTS_DIR}/TEST-impala-custom-cluster.xml")
+ARGS+=("--resultlog=${RESULTS_DIR}/TEST-impala-custom-cluster.log")
ARGS+=("$@")
impala-py.test "${ARGS[@]}"
[4/4] incubator-impala git commit: IMPALA-4839: Remove implicit
'localhost' for KUDU_MASTER_HOSTS
Posted by he...@apache.org.
IMPALA-4839: Remove implicit 'localhost' for KUDU_MASTER_HOSTS
The Kudu query tests were failing on a remote cluster because the Kudu
master was always set to '127.0.0.1', with no way to override it.
This patch corrects the issue with a number of changes:
- Add a pytest command line option to specify an arbitrary Kudu master
- Consolidate the place where the default Kudu master is derived. It
had been stored both in the env and in tests/common/__init__.py,
with different files looking to different places. For now, just look
to the env, and remove the value from __init__.py.
- The kudu_client test fixture in conftest.py was using the connect()
method from impala.dbapi (part of the Impyla library), without
specifying the host param. In the absence of that, the default value
is 'localhost', so add the host param to the connect() call.
- Define the various defaults for pytest config as constants at the top
of conftest.py.
Change-Id: I9df71480a165f4ce21ae3edab6ce7227fbf76f77
Reviewed-on: http://gerrit.cloudera.org:8080/5877
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
Tested-by: Impala Public Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/894bb778
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/894bb778
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/894bb778
Branch: refs/heads/master
Commit: 894bb7785519f4f00d1af52e9c6602c43396503c
Parents: 9e36088
Author: David Knupp <dk...@cloudera.com>
Authored: Wed Feb 1 16:03:28 2017 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Feb 14 21:51:39 2017 +0000
----------------------------------------------------------------------
bin/impala-config.sh | 2 +-
bin/start-impala-cluster.py | 9 ++--
testdata/bin/generate-schema-statements.py | 2 +-
tests/common/__init__.py | 2 -
tests/conftest.py | 69 +++++++++++++++----------
tests/custom_cluster/test_kudu.py | 2 +-
tests/query_test/test_kudu.py | 5 +-
7 files changed, 52 insertions(+), 39 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/bin/impala-config.sh
----------------------------------------------------------------------
diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index 2cc0429..4a89432 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -326,7 +326,7 @@ else
fi
export NUM_CONCURRENT_TESTS="${NUM_CONCURRENT_TESTS-${CORES}}"
-export KUDU_MASTER="${KUDU_MASTER:-127.0.0.1}"
+export KUDU_MASTER_HOSTS="${KUDU_MASTER_HOSTS:-127.0.0.1}"
export KUDU_MASTER_PORT="${KUDU_MASTER_PORT:-7051}"
export IMPALA_FE_DIR="$IMPALA_HOME/fe"
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/bin/start-impala-cluster.py
----------------------------------------------------------------------
diff --git a/bin/start-impala-cluster.py b/bin/start-impala-cluster.py
index df7dea6..b276b08 100755
--- a/bin/start-impala-cluster.py
+++ b/bin/start-impala-cluster.py
@@ -27,9 +27,8 @@ from getpass import getuser
from time import sleep, time
from optparse import OptionParser
from testdata.common import cgroups
-from tests.common import KUDU_MASTER_HOSTS
-
+KUDU_MASTER_HOSTS = os.getenv('KUDU_MASTER_HOSTS', '127.0.0.1')
DEFAULT_IMPALA_MAX_LOG_FILES = os.environ.get('IMPALA_MAX_LOG_FILES', 10)
@@ -72,7 +71,7 @@ parser.add_option("--log_level", type="int", dest="log_level", default=1,
help="Set the impalad backend logging level")
parser.add_option("--jvm_args", dest="jvm_args", default="",
help="Additional arguments to pass to the JVM(s) during startup.")
-parser.add_option("--kudu_masters", default=KUDU_MASTER_HOSTS,
+parser.add_option("--kudu_master_hosts", default=KUDU_MASTER_HOSTS,
help="The host name or address of the Kudu master. Multiple masters "
"can be specified using a comma separated list.")
@@ -237,9 +236,9 @@ def start_impalad_instances(cluster_size):
(mem_limit, # Goes first so --impalad_args will override it.
build_impalad_logging_args(i, service_name), build_jvm_args(i),
build_impalad_port_args(i), param_args)
- if options.kudu_masters:
+ if options.kudu_master_hosts:
# Must be prepended, otherwise the java options interfere.
- args = "-kudu_master_hosts %s %s" % (options.kudu_masters, args)
+ args = "-kudu_master_hosts %s %s" % (options.kudu_master_hosts, args)
stderr_log_file_path = os.path.join(options.log_dir, '%s-error.log' % service_name)
exec_impala_process(IMPALAD_PATH, args, stderr_log_file_path)
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/testdata/bin/generate-schema-statements.py
----------------------------------------------------------------------
diff --git a/testdata/bin/generate-schema-statements.py b/testdata/bin/generate-schema-statements.py
index 07e722a..a214822 100755
--- a/testdata/bin/generate-schema-statements.py
+++ b/testdata/bin/generate-schema-statements.py
@@ -226,7 +226,7 @@ def build_table_template(file_format, columns, partition_columns, row_format,
partitioned_by = "partition by hash partitions 3"
# Fetch KUDU host and port from environment
- kudu_master = os.getenv("KUDU_MASTER_ADDRESS", "127.0.0.1")
+ kudu_master = os.getenv("KUDU_MASTER_HOSTS", "127.0.0.1")
kudu_master_port = os.getenv("KUDU_MASTER_PORT", "7051")
row_format_stmt = str()
tblproperties["kudu.master_addresses"] = \
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/tests/common/__init__.py
----------------------------------------------------------------------
diff --git a/tests/common/__init__.py b/tests/common/__init__.py
index 665bfe2..e69de29 100644
--- a/tests/common/__init__.py
+++ b/tests/common/__init__.py
@@ -1,2 +0,0 @@
-# These values should match the impalad startup flag -kudu_master_hosts
-KUDU_MASTER_HOSTS = "127.0.0.1"
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/tests/conftest.py
----------------------------------------------------------------------
diff --git a/tests/conftest.py b/tests/conftest.py
index 44b00ff..351117d 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -27,7 +27,6 @@ import logging
import os
import pytest
-from common import KUDU_MASTER_HOSTS
from common.test_result_verifier import QueryTestResult
from tests.common.patterns import is_valid_impala_identifier
from tests.comparison.db_connection import ImpalaConnection
@@ -37,17 +36,23 @@ logging.basicConfig(level=logging.INFO, format='%(threadName)s: %(message)s')
LOG = logging.getLogger('test_configuration')
DEFAULT_CONN_TIMEOUT = 45
-
-def _get_default_nn_http_addr():
- """Return the namenode ip and webhdfs port if the default shouldn't be used"""
- if FILESYSTEM == 'isilon':
- return "%s:%s" % (os.getenv("ISILON_NAMENODE"), ISILON_WEBHDFS_PORT)
- return None
+DEFAULT_EXPLORATION_STRATEGY = 'core'
+DEFAULT_HDFS_XML_CONF = os.path.join(os.environ['HADOOP_CONF_DIR'], "hdfs-site.xml")
+DEFAULT_HIVE_SERVER2 = 'localhost:11050'
+DEFAULT_IMPALAD_HS2_PORT = '21050'
+DEFAULT_IMPALADS = "localhost:21000,localhost:21001,localhost:21002"
+DEFAULT_KUDU_MASTER_HOSTS = os.getenv('KUDU_MASTER_HOSTS', '127.0.0.1')
+DEFAULT_KUDU_MASTER_PORT = os.getenv('KUDU_MASTER_PORT', '7051')
+DEFAULT_METASTORE_SERVER = 'localhost:9083'
+DEFAULT_NAMENODE_ADDR = None
+if FILESYSTEM == 'isilon':
+ DEFAULT_NAMENODE_ADDR = "{node}:{port}".format(node=os.getenv("ISILON_NAMENODE"),
+ port=ISILON_WEBHDFS_PORT)
def pytest_addoption(parser):
"""Adds a new command line options to py.test"""
- parser.addoption("--exploration_strategy", default="core",
+ parser.addoption("--exploration_strategy", default=DEFAULT_EXPLORATION_STRATEGY,
help="Default exploration strategy for all tests. Valid values: core, "
"pairwise, exhaustive.")
@@ -55,25 +60,27 @@ def pytest_addoption(parser):
help="Override exploration strategy for specific workloads using the "
"format: workload:exploration_strategy. Ex: tpch:core,tpcds:pairwise.")
- parser.addoption("--impalad", default="localhost:21000,localhost:21001,localhost:21002",
+ parser.addoption("--impalad", default=DEFAULT_IMPALADS,
help="A comma-separated list of impalad host:ports to target. Note: "
"Not all tests make use of all impalad, some tests target just the "
"first item in the list (it is considered the 'default'")
- parser.addoption("--impalad_hs2_port", default="21050",
+ parser.addoption("--impalad_hs2_port", default=DEFAULT_IMPALAD_HS2_PORT,
help="The impalad HiveServer2 port.")
- parser.addoption("--metastore_server", default="localhost:9083",
+ parser.addoption("--metastore_server", default=DEFAULT_METASTORE_SERVER,
help="The Hive Metastore server host:port to connect to.")
- parser.addoption("--hive_server2", default="localhost:11050",
+ parser.addoption("--hive_server2", default=DEFAULT_HIVE_SERVER2,
help="Hive's HiveServer2 host:port to connect to.")
- default_xml_path = os.path.join(os.environ['HADOOP_CONF_DIR'], "hdfs-site.xml")
- parser.addoption("--minicluster_xml_conf", default=default_xml_path,
+ parser.addoption("--kudu_master_hosts", default=DEFAULT_KUDU_MASTER_HOSTS,
+ help="Kudu master. Can be supplied as hostname, or hostname:port.")
+
+ parser.addoption("--minicluster_xml_conf", default=DEFAULT_HDFS_XML_CONF,
help="The full path to the HDFS xml configuration file")
- parser.addoption("--namenode_http_address", default=_get_default_nn_http_addr(),
+ parser.addoption("--namenode_http_address", default=DEFAULT_NAMENODE_ADDR,
help="The host:port for the HDFS Namenode's WebHDFS interface. Takes"
" precedence over any configuration read from --minicluster_xml_conf")
@@ -298,20 +305,21 @@ def kudu_client():
"""Provides a new Kudu client as a pytest fixture. The client only exists for the
duration of the method it is used in.
"""
- if "," in KUDU_MASTER_HOSTS:
+ kudu_master = pytest.config.option.kudu_master_hosts
+
+ if "," in kudu_master:
raise Exception("Multi-master not supported yet")
- if ":" in KUDU_MASTER_HOSTS:
- host, port = KUDU_MASTER_HOSTS.split(":")
+ if ":" in kudu_master:
+ host, port = kudu_master.split(":")
else:
- host, port = KUDU_MASTER_HOSTS, 7051
+ host, port = kudu_master, DEFAULT_KUDU_MASTER_PORT
kudu_client = kudu_connect(host, port)
+ yield kudu_client
+
try:
- yield kudu_client
- finally:
- try:
- kudu_client.close()
- except Exception as e:
- LOG.warn("Error closing Kudu client: %s", e)
+ kudu_client.close()
+ except Exception as e:
+ LOG.warn("Error closing Kudu client: %s", e)
@pytest.yield_fixture(scope="class")
@@ -387,12 +395,17 @@ def __unique_conn(db_name=None, timeout=DEFAULT_CONN_TIMEOUT):
@contextlib.contextmanager
def __auto_closed_conn(db_name=None, timeout=DEFAULT_CONN_TIMEOUT):
- """Returns a connection to Impala. This is intended to be used in a "with" block. The
- connection will be closed upon exiting the block.
+ """Returns a connection to Impala. This is intended to be used in a "with" block.
+ The connection will be closed upon exiting the block.
The returned connection will have a 'db_name' property.
"""
- conn = impala_connect(database=db_name, timeout=timeout)
+ default_impalad = pytest.config.option.impalad.split(',')[0]
+ impalad_host = default_impalad.split(':')[0]
+ hs2_port = pytest.config.option.impalad_hs2_port
+
+ conn = impala_connect(host=impalad_host, port=hs2_port, database=db_name,
+ timeout=timeout)
try:
conn.db_name = db_name
yield conn
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/tests/custom_cluster/test_kudu.py
----------------------------------------------------------------------
diff --git a/tests/custom_cluster/test_kudu.py b/tests/custom_cluster/test_kudu.py
index bd1584b..9377863 100644
--- a/tests/custom_cluster/test_kudu.py
+++ b/tests/custom_cluster/test_kudu.py
@@ -19,10 +19,10 @@ import logging
import pytest
from kudu.schema import INT32
-from tests.common import KUDU_MASTER_HOSTS
from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
from tests.common.kudu_test_suite import KuduTestSuite
+KUDU_MASTER_HOSTS = pytest.config.option.kudu_master_hosts
LOG = logging.getLogger(__name__)
class TestKuduOperations(CustomClusterTestSuite, KuduTestSuite):
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/894bb778/tests/query_test/test_kudu.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_kudu.py b/tests/query_test/test_kudu.py
index 5b28120..b7235e8 100644
--- a/tests/query_test/test_kudu.py
+++ b/tests/query_test/test_kudu.py
@@ -32,11 +32,12 @@ import logging
import pytest
import textwrap
-from tests.common import KUDU_MASTER_HOSTS
from tests.common.kudu_test_suite import KuduTestSuite
from tests.common.impala_cluster import ImpalaCluster
from tests.verifiers.metric_verifier import MetricVerifier
+KUDU_MASTER_HOSTS = pytest.config.option.kudu_master_hosts
+
LOG = logging.getLogger(__name__)
class TestKuduOperations(KuduTestSuite):
@@ -62,6 +63,8 @@ class TestKuduOperations(KuduTestSuite):
def test_kudu_partition_ddl(self, vector, unique_database):
self.run_test_case('QueryTest/kudu_partition_ddl', vector, use_db=unique_database)
+ @pytest.mark.skipif(pytest.config.option.testing_remote_cluster,
+ reason="Test references hardcoded hostnames: IMPALA-4873")
@pytest.mark.execute_serially
def test_kudu_alter_table(self, vector, unique_database):
self.run_test_case('QueryTest/kudu_alter', vector, use_db=unique_database)