From eaee1ab9f2d30bac1475bdb2830dfde94b002dcc Mon Sep 17 00:00:00 2001 From: Pindikura Ravindra Date: Wed, 17 Jul 2019 13:08:33 +0530 Subject: [PATCH 1/2] ARROW-5964: remove overflow check after rounding - std::round can round down also. - also, there is a check further down for overflow (2^127-1). --- cpp/src/gandiva/precompiled/decimal_ops.cc | 4 ++-- cpp/src/gandiva/precompiled/decimal_ops_test.cc | 14 +++++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/cpp/src/gandiva/precompiled/decimal_ops.cc b/cpp/src/gandiva/precompiled/decimal_ops.cc index 3dd74d7b447..902c8d799ae 100644 --- a/cpp/src/gandiva/precompiled/decimal_ops.cc +++ b/cpp/src/gandiva/precompiled/decimal_ops.cc @@ -491,9 +491,9 @@ static std::array kDoubleScaleMultip BasicDecimal128 FromDouble(double in, int32_t precision, int32_t scale, bool* overflow) { // Multiply decimal with the scale auto unscaled = in * kDoubleScaleMultipliers[scale]; + DECIMAL_OVERFLOW_IF(std::isnan(unscaled), overflow); + unscaled = std::round(unscaled); - DECIMAL_OVERFLOW_IF(std::isnan(unscaled) || std::fabs(unscaled) < std::fabs(in), - overflow); // convert scaled double to int128 int32_t sign = unscaled < 0 ? -1 : 1; diff --git a/cpp/src/gandiva/precompiled/decimal_ops_test.cc b/cpp/src/gandiva/precompiled/decimal_ops_test.cc index ef40e6162a6..131d789b336 100644 --- a/cpp/src/gandiva/precompiled/decimal_ops_test.cc +++ b/cpp/src/gandiva/precompiled/decimal_ops_test.cc @@ -872,6 +872,12 @@ TEST_F(TestDecimalSql, FromDouble) { std::make_tuple(DecimalScalar128{162850, 38, 4}, 16.285, false), std::make_tuple(DecimalScalar128{1629, 38, 2}, 16.285, false), + // round up + std::make_tuple(DecimalScalar128{1, 18, 0}, 1.15470053838, false), + std::make_tuple(DecimalScalar128{-1, 18, 0}, -1.15470053838, false), + std::make_tuple(DecimalScalar128{2, 18, 0}, 1.55470053838, false), + std::make_tuple(DecimalScalar128{-2, 18, 0}, -1.55470053838, false), + // border cases std::make_tuple(DecimalScalar128{-kMaxDoubleInt, 38, 0}, static_cast(-kMaxDoubleInt), false), @@ -887,9 +893,11 @@ TEST_F(TestDecimalSql, FromDouble) { std::make_tuple(DecimalScalar128{1230, 38, 33}, 1.23E-30, false), std::make_tuple(DecimalScalar128{123, 38, 38}, 1.23E-36, false), - // overflow due to very low double - std::make_tuple(DecimalScalar128{0, 0, 38, 0}, std::numeric_limits::min(), - true), + // TODO: std::round() converts this to 0. + // std::make_tuple(DecimalScalar128{0, 0, 38, 0}, + // std::numeric_limits::min(), + // true), + // overflow due to very high double std::make_tuple(DecimalScalar128{0, 0, 38, 0}, std::numeric_limits::max(), true), From 8d6f37af8ef659c02e7ea8d029cd387e00da04fd Mon Sep 17 00:00:00 2001 From: Pindikura Ravindra Date: Wed, 17 Jul 2019 18:31:13 +0530 Subject: [PATCH 2/2] fix tests --- cpp/src/gandiva/precompiled/decimal_ops_test.cc | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/cpp/src/gandiva/precompiled/decimal_ops_test.cc b/cpp/src/gandiva/precompiled/decimal_ops_test.cc index 131d789b336..290cc46946f 100644 --- a/cpp/src/gandiva/precompiled/decimal_ops_test.cc +++ b/cpp/src/gandiva/precompiled/decimal_ops_test.cc @@ -893,12 +893,16 @@ TEST_F(TestDecimalSql, FromDouble) { std::make_tuple(DecimalScalar128{1230, 38, 33}, 1.23E-30, false), std::make_tuple(DecimalScalar128{123, 38, 38}, 1.23E-36, false), - // TODO: std::round() converts this to 0. - // std::make_tuple(DecimalScalar128{0, 0, 38, 0}, - // std::numeric_limits::min(), - // true), + // very small doubles + std::make_tuple(DecimalScalar128{0, 0, 38, 0}, std::numeric_limits::min(), + false), + std::make_tuple(DecimalScalar128{0, 0, 38, 0}, -std::numeric_limits::min(), + false), - // overflow due to very high double + // overflow due to large -ve double + std::make_tuple(DecimalScalar128{0, 0, 38, 0}, -std::numeric_limits::max(), + true), + // overflow due to large +ve double std::make_tuple(DecimalScalar128{0, 0, 38, 0}, std::numeric_limits::max(), true), // overflow due to scaling up.