From c7ac67e867c8e63e34aedf625757c0d53d461093 Mon Sep 17 00:00:00 2001 From: rui-mo Date: Fri, 23 Feb 2024 14:22:44 +0800 Subject: [PATCH] minor fix --- velox/expression/tests/CastExprTest.cpp | 17 ++++++------- velox/type/DecimalUtil.h | 33 ++++++++----------------- 2 files changed, 17 insertions(+), 33 deletions(-) diff --git a/velox/expression/tests/CastExprTest.cpp b/velox/expression/tests/CastExprTest.cpp index 6e83395e2bcc4..c8822f016d10e 100644 --- a/velox/expression/tests/CastExprTest.cpp +++ b/velox/expression/tests/CastExprTest.cpp @@ -2086,12 +2086,12 @@ TEST_F(CastExprTest, doubleToDecimal) { DOUBLE(), DECIMAL(38, 2), {std::numeric_limits::max()}, - "Cannot cast DOUBLE '1.7976931348623157E308' to DECIMAL(38, 2). Result overflows."); + "Result overflows."); testThrow( DOUBLE(), DECIMAL(38, 2), {std::numeric_limits::lowest()}, - "Cannot cast DOUBLE '-1.7976931348623157E308' to DECIMAL(38, 2). Result overflows."); + "Result overflows."); testCast( makeConstant(std::numeric_limits::min(), 1), makeConstant(0, 1, DECIMAL(38, 2))); @@ -2169,10 +2169,7 @@ TEST_F(CastExprTest, realToDecimal) { DECIMAL(38, 18))); testThrow( - REAL(), - DECIMAL(10, 2), - {9999999999999999999999.99}, - "Cannot cast REAL '9.999999778196308E21' to DECIMAL(10, 2). Result overflows."); + REAL(), DECIMAL(10, 2), {9999999999999999999999.99}, "Result overflows."); testThrow( REAL(), DECIMAL(10, 2), @@ -2189,22 +2186,22 @@ TEST_F(CastExprTest, realToDecimal) { REAL(), DECIMAL(20, 2), {static_cast(DecimalUtil::kLongDecimalMax)}, - "Cannot cast REAL '9.999999680285692E37' to DECIMAL(20, 2). Result overflows."); + "Result overflows."); testThrow( REAL(), DECIMAL(20, 2), {static_cast(DecimalUtil::kLongDecimalMin)}, - "Cannot cast REAL '-9.999999680285692E37' to DECIMAL(20, 2). Result overflows."); + "Result overflows."); testThrow( REAL(), DECIMAL(38, 2), {std::numeric_limits::max()}, - "Cannot cast REAL '3.4028234663852886E38' to DECIMAL(38, 2). Result overflows."); + "Result overflows."); testThrow( REAL(), DECIMAL(38, 2), {std::numeric_limits::lowest()}, - "Cannot cast REAL '-3.4028234663852886E38' to DECIMAL(38, 2). Result overflows."); + "Result overflows."); testCast( makeConstant(std::numeric_limits::min(), 1), makeConstant(0, 1, DECIMAL(38, 2))); diff --git a/velox/type/DecimalUtil.h b/velox/type/DecimalUtil.h index 6c46032975bbf..c3a8b370a6d85 100644 --- a/velox/type/DecimalUtil.h +++ b/velox/type/DecimalUtil.h @@ -226,33 +226,20 @@ class DecimalUtil { // A double provides from 15 to 17 decimal digits, so at least 15 digits // are precise. digits = 15; - if (value <= std::numeric_limits::min() || - value >= std::numeric_limits::max()) { - return Status::UserError("Result overflows."); - } } - // Calculate the precise fractional digits. - const auto integralValue = - static_cast(value > 0 ? value : -value); - const auto integralDigits = - integralValue == 0 ? 0 : countDigits(integralValue); - const auto fractionDigits = digits - integralDigits; - /// Scales up the input value to keep all the precise fractional digits - /// before rounding. Convert value to long double type, as double * int128_t - /// returns int128_t and fractional digits are lost. No need to consider - /// 'toValue' becoming infinite as DOUBLE_MAX * 10^38 < LONG_DOUBLE_MAX. - const auto scaledValue = - (long double)value * DecimalUtil::kPowersOfTen[fractionDigits]; - long double rounded; - if (scale > fractionDigits) { - rounded = std::round(scaledValue) * - DecimalUtil::kPowersOfTen[scale - fractionDigits]; + if (scale > digits) { + // Convert value to long double type, as double * int128_t returns + // int128_t and fractional digits are lost. No need to consider 'toValue' + // becoming infinite as DOUBLE_MAX * 10^38 < LONG_DOUBLE_MAX. + const auto toValue = + (long double)value * DecimalUtil::kPowersOfTen[digits]; + rounded = std::round(toValue) * DecimalUtil::kPowersOfTen[scale - digits]; } else { - rounded = std::round( - std::round(scaledValue) / - DecimalUtil::kPowersOfTen[fractionDigits - scale]); + const auto toValue = + (long double)value * DecimalUtil::kPowersOfTen[scale]; + rounded = std::round(toValue); } const auto result = folly::tryTo(rounded);