minor fix

rui-mo · Feb 26, 2024 · eefb1b9 · eefb1b9
1 parent aa18b28
commit eefb1b9
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 35 deletions.
diff --git 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<double>::max()},
-      "Cannot cast DOUBLE '1.7976931348623157E308' to DECIMAL(38, 2). Result overflows.");
+      "Result overflows.");
   testThrow<double>(
       DOUBLE(),
       DECIMAL(38, 2),
       {std::numeric_limits<double>::lowest()},
-      "Cannot cast DOUBLE '-1.7976931348623157E308' to DECIMAL(38, 2). Result overflows.");
+      "Result overflows.");
   testCast(
       makeConstant<double>(std::numeric_limits<double>::min(), 1),
       makeConstant<int128_t>(0, 1, DECIMAL(38, 2)));
@@ -2169,10 +2169,7 @@ TEST_F(CastExprTest, realToDecimal) {
           DECIMAL(38, 18)));
 
   testThrow<float>(
-      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<float>(
       REAL(),
       DECIMAL(10, 2),
@@ -2189,22 +2186,22 @@ TEST_F(CastExprTest, realToDecimal) {
       REAL(),
       DECIMAL(20, 2),
       {static_cast<float>(DecimalUtil::kLongDecimalMax)},
-      "Cannot cast REAL '9.999999680285692E37' to DECIMAL(20, 2). Result overflows.");
+      "Result overflows.");
   testThrow<float>(
       REAL(),
       DECIMAL(20, 2),
       {static_cast<float>(DecimalUtil::kLongDecimalMin)},
-      "Cannot cast REAL '-9.999999680285692E37' to DECIMAL(20, 2). Result overflows.");
+      "Result overflows.");
   testThrow<float>(
       REAL(),
       DECIMAL(38, 2),
       {std::numeric_limits<float>::max()},
-      "Cannot cast REAL '3.4028234663852886E38' to DECIMAL(38, 2). Result overflows.");
+      "Result overflows.");
   testThrow<float>(
       REAL(),
       DECIMAL(38, 2),
       {std::numeric_limits<float>::lowest()},
-      "Cannot cast REAL '-3.4028234663852886E38' to DECIMAL(38, 2). Result overflows.");
+      "Result overflows.");
   testCast(
       makeConstant<float>(std::numeric_limits<float>::min(), 1),
       makeConstant<int128_t>(0, 1, DECIMAL(38, 2)));

diff --git a/velox/type/DecimalUtil.h b/velox/type/DecimalUtil.h
@@ -226,36 +226,35 @@ 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<int128_t>::min() ||
-          value >= std::numeric_limits<int128_t>::max()) {
-        return Status::UserError("Result overflows.");
-      }
     }
 
-    // Calculate the precise fractional digits.
-    const auto integralValue =
-        static_cast<uint128_t>(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];
+    // long double rounded;
+    // 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 {
+    //   const auto toValue =
+    //       (long double)value * DecimalUtil::kPowersOfTen[scale];
+    //   rounded = std::round(toValue);
+    // }
+    if (scale > digits) {
+      auto toValue = (long double)value * DecimalUtil::kPowersOfTen[digits];
+      double sign = (double(0) < toValue) - (toValue < double(0));
+      toValue += 1e-9 * sign;
+      toValue = std::round(toValue);
+      toValue *= DecimalUtil::kPowersOfTen[scale - digits];
     } else {
-      rounded = std::round(
-          std::round(scaledValue) /
-          DecimalUtil::kPowersOfTen[fractionDigits - scale]);
+      auto toValue = (long double)value * DecimalUtil::kPowersOfTen[scale];
+      // Add the sign (-1, 0, 1) times a tiny value to fix floating point issues.
+      double sign = (double(0) < toValue) - (toValue < double(0));
+      toValue += 1e-9 * sign;
     }
 
-    const auto result = folly::tryTo<TOutput>(rounded);
+    const auto result = folly::tryTo<TOutput>(toValue);
     if (result.hasError()) {
       return Status::UserError("Result overflows.");
     }