impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tarmstr...@apache.org
Subject [1/3] incubator-impala git commit: IMPALA-4513: Promote integer types for ABS()
Date Mon, 25 Sep 2017 06:28:53 GMT
Repository: incubator-impala
Updated Branches:
  refs/heads/master 495325440 -> 646920810


IMPALA-4513: Promote integer types for ABS()

The internal representation of the most negative number
in two's complement requires 1 more bit to represent the
positive version.  This means ABS() must promote integer
types to the next highest width.

Change-Id: I86cc880e78258d5f90471bd8af4caeb4305eed77
Reviewed-on: http://gerrit.cloudera.org:8080/8004
Reviewed-by: Tim Armstrong <tarmstrong@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/f53ce3b1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/f53ce3b1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/f53ce3b1

Branch: refs/heads/master
Commit: f53ce3b16d476214167a686ecb513c6d866105b9
Parents: 4953254
Author: Zachary Amsden <zamsden@cloudera.com>
Authored: Thu Sep 7 14:15:19 2017 -0700
Committer: Impala Public Jenkins <impala-public-jenkins@gerrit.cloudera.org>
Committed: Sat Sep 23 02:41:32 2017 +0000

----------------------------------------------------------------------
 be/src/exprs/expr-test.cc                       | 17 +++++++++++++++-
 be/src/exprs/math-functions-ir.cc               | 21 ++++++++++++++++----
 be/src/exprs/math-functions.h                   |  6 +++---
 common/function-registry/impala_functions.py    |  6 +++---
 .../queries/QueryTest/exprs.test                |  4 ++--
 5 files changed, 41 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index 33d77e0..e4d633d 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -4253,6 +4253,21 @@ TEST_F(ExprTest, MathFunctions) {
   TestValue("e()", TYPE_DOUBLE, M_E);
   TestValue("abs(cast(-1.0 as double))", TYPE_DOUBLE, 1.0);
   TestValue("abs(cast(1.0 as double))", TYPE_DOUBLE, 1.0);
+  TestValue("abs(-127)", TYPE_SMALLINT, 127);
+  TestValue("abs(-128)", TYPE_SMALLINT, 128);
+  TestValue("abs(127)", TYPE_SMALLINT, 127);
+  TestValue("abs(128)", TYPE_INT, 128);
+  TestValue("abs(-32767)", TYPE_INT, 32767);
+  TestValue("abs(-32768)", TYPE_INT, 32768);
+  TestValue("abs(32767)", TYPE_INT, 32767);
+  TestValue("abs(32768)", TYPE_BIGINT, 32768);
+  TestValue("abs(-1 * cast(pow(2, 31) as int))", TYPE_BIGINT, 2147483648);
+  TestValue("abs(cast(pow(2, 31) as int))", TYPE_BIGINT, 2147483648);
+  TestValue("abs(2147483647)", TYPE_BIGINT, 2147483647);
+  TestValue("abs(2147483647)", TYPE_BIGINT, 2147483647);
+  TestValue("abs(-9223372036854775807)", TYPE_BIGINT,  9223372036854775807);
+  TestValue("abs(9223372036854775807)", TYPE_BIGINT,  9223372036854775807);
+  TestIsNull("abs(-9223372036854775808)", TYPE_BIGINT);
   TestValue("sign(0.0)", TYPE_FLOAT, 0.0f);
   TestValue("sign(-0.0)", TYPE_FLOAT, 0.0f);
   TestValue("sign(+0.0)", TYPE_FLOAT, 0.0f);
@@ -4521,7 +4536,7 @@ TEST_F(ExprTest, MathFunctions) {
 
   // NULL arguments. In some cases the NULL can match multiple overloads so the result
   // type depends on the order in which function overloads are considered.
-  TestIsNull("abs(NULL)", TYPE_TINYINT);
+  TestIsNull("abs(NULL)", TYPE_SMALLINT);
   TestIsNull("sign(NULL)", TYPE_FLOAT);
   TestIsNull("exp(NULL)", TYPE_DOUBLE);
   TestIsNull("ln(NULL)", TYPE_DOUBLE);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/be/src/exprs/math-functions-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/math-functions-ir.cc b/be/src/exprs/math-functions-ir.cc
index 48b98a9..cc86d5b 100644
--- a/be/src/exprs/math-functions-ir.cc
+++ b/be/src/exprs/math-functions-ir.cc
@@ -59,12 +59,25 @@ DoubleVal MathFunctions::E(FunctionContext* ctx) {
     return RET_TYPE(FN(v1.val, v2.val)); \
   }
 
-ONE_ARG_MATH_FN(Abs, BigIntVal, BigIntVal, llabs);
+// N.B. - for integer math, we have to promote ABS() to the next highest integer type
+// because in two's complement arithmetic, the largest negative value for any bit width
+// is not representable as a positive value within the same width.  For the largest width,
+// we simply overflow.  In the unlikely event a workaround is needed, one can simply
+// cast to a higher precision decimal type.
+BigIntVal MathFunctions::Abs(FunctionContext* ctx, const BigIntVal& v) {
+  if (v.is_null) return BigIntVal::null();
+  if (UNLIKELY(v.val == std::numeric_limits<BigIntVal::underlying_type_t>::min()))
{
+      ctx->AddWarning("abs() overflowed, returning NULL");
+      return BigIntVal::null();
+  }
+  return BigIntVal(llabs(v.val));
+}
+
+ONE_ARG_MATH_FN(Abs, BigIntVal, IntVal, llabs);
+ONE_ARG_MATH_FN(Abs, IntVal, SmallIntVal, abs);
+ONE_ARG_MATH_FN(Abs, SmallIntVal, TinyIntVal, abs);
 ONE_ARG_MATH_FN(Abs, DoubleVal, DoubleVal, fabs);
 ONE_ARG_MATH_FN(Abs, FloatVal, FloatVal, fabs);
-ONE_ARG_MATH_FN(Abs, IntVal, IntVal, abs);
-ONE_ARG_MATH_FN(Abs, SmallIntVal, SmallIntVal, abs);
-ONE_ARG_MATH_FN(Abs, TinyIntVal, TinyIntVal, abs);
 ONE_ARG_MATH_FN(Sin, DoubleVal, DoubleVal, sin);
 ONE_ARG_MATH_FN(Asin, DoubleVal, DoubleVal, asin);
 ONE_ARG_MATH_FN(Cos, DoubleVal, DoubleVal, cos);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/be/src/exprs/math-functions.h
----------------------------------------------------------------------
diff --git a/be/src/exprs/math-functions.h b/be/src/exprs/math-functions.h
index 867ca2b..7063907 100644
--- a/be/src/exprs/math-functions.h
+++ b/be/src/exprs/math-functions.h
@@ -48,9 +48,9 @@ class MathFunctions {
   static BigIntVal Abs(FunctionContext*, const BigIntVal&);
   static DoubleVal Abs(FunctionContext*, const DoubleVal&);
   static FloatVal Abs(FunctionContext*, const FloatVal&);
-  static IntVal Abs(FunctionContext*, const IntVal&);
-  static SmallIntVal Abs(FunctionContext*, const SmallIntVal&);
-  static TinyIntVal Abs(FunctionContext*, const TinyIntVal&);
+  static BigIntVal Abs(FunctionContext*, const IntVal&);
+  static IntVal Abs(FunctionContext*, const SmallIntVal&);
+  static SmallIntVal Abs(FunctionContext*, const TinyIntVal&);
   static DoubleVal Sin(FunctionContext*, const DoubleVal&);
   static DoubleVal Asin(FunctionContext*, const DoubleVal&);
   static DoubleVal Cos(FunctionContext*, const DoubleVal&);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/common/function-registry/impala_functions.py
----------------------------------------------------------------------
diff --git a/common/function-registry/impala_functions.py b/common/function-registry/impala_functions.py
index c004775..0e3a3b8 100644
--- a/common/function-registry/impala_functions.py
+++ b/common/function-registry/impala_functions.py
@@ -260,9 +260,9 @@ visible_functions = [
   [['abs'], 'BIGINT', ['BIGINT'], 'impala::MathFunctions::Abs'],
   [['abs'], 'DOUBLE', ['DOUBLE'], 'impala::MathFunctions::Abs'],
   [['abs'], 'FLOAT', ['FLOAT'], 'impala::MathFunctions::Abs'],
-  [['abs'], 'INT', ['INT'], 'impala::MathFunctions::Abs'],
-  [['abs'], 'SMALLINT', ['SMALLINT'], 'impala::MathFunctions::Abs'],
-  [['abs'], 'TINYINT', ['TINYINT'], 'impala::MathFunctions::Abs'],
+  [['abs'], 'BIGINT', ['INT'], 'impala::MathFunctions::Abs'],
+  [['abs'], 'INT', ['SMALLINT'], 'impala::MathFunctions::Abs'],
+  [['abs'], 'SMALLINT', ['TINYINT'], 'impala::MathFunctions::Abs'],
   [['sign'], 'FLOAT', ['DOUBLE'], 'impala::MathFunctions::Sign'],
   [['sin'], 'DOUBLE', ['DOUBLE'], 'impala::MathFunctions::Sin'],
   [['asin'], 'DOUBLE', ['DOUBLE'], 'impala::MathFunctions::Asin'],

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f53ce3b1/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 9b1a1a7..92854b1 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
@@ -1866,7 +1866,7 @@ Infinity,NaN,-0,-Infinity,Nan
 double,double,double,double,double
 ====
 ---- QUERY
-# Test that abs() retains the type of the paramter IMPALA-1424
+# Test that abs() promotes the type of the paramter IMPALA-4513
 select abs(cast(1 as int)), abs(cast(1 as smallint)),
   abs(cast(1 as tinyint)), abs(cast(8589934592 as bigint)),
   abs(cast(-1.3 as double)), abs(cast(-1.3 as float)),
@@ -1874,7 +1874,7 @@ select abs(cast(1 as int)), abs(cast(1 as smallint)),
 ---- RESULTS
 1,1,1,8589934592,1.3,1.299999952316284,1.322
 ---- TYPES
-int, smallint, tinyint, bigint, double, float, decimal
+bigint, int, smallint, bigint, double, float, decimal
 ====
 ---- QUERY
 # Regression test for IMPALA-1508


Mime
View raw message