From 4cbf105f2a3945e2c914ba6819302f1cd81c0ccc Mon Sep 17 00:00:00 2001 From: Victor Zoubok Date: Thu, 15 Oct 2026 14:29:22 -0700 Subject: [PATCH 1/2] DX-90291: Gandiva: fix round(double, scale) overflow for large values --- .../gandiva/precompiled/extended_math_ops.cc | 20 ++++++++---- .../precompiled/extended_math_ops_test.cc | 32 +++++++++++++++++++ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/cpp/src/gandiva/precompiled/extended_math_ops.cc b/cpp/src/gandiva/precompiled/extended_math_ops.cc index 348d595faf49..a6acccb9dbe1 100644 --- a/cpp/src/gandiva/precompiled/extended_math_ops.cc +++ b/cpp/src/gandiva/precompiled/extended_math_ops.cc @@ -304,14 +304,20 @@ gdv_float64 bround_float64(gdv_float64 num) { return num - diff_num; } +// doubles at or above 2^52 and have no fractional part, so no point to truncate +static const gdv_float64 kMaxFractionalDouble = static_cast(1LL << 52); // rounds the number to the given scale -#define ROUND_DECIMAL_TO_SCALE(TYPE) \ - FORCE_INLINE \ - gdv_##TYPE round_##TYPE##_int32(gdv_##TYPE number, gdv_int32 out_scale) { \ - gdv_float64 scale_multiplier = get_scale_multiplier(out_scale); \ - return static_cast( \ - trunc(number * scale_multiplier + ((number >= 0) ? 0.5 : -0.5)) / \ - scale_multiplier); \ +// every ordered comparison with NaN is false: <, <=, >, >= and == +#define ROUND_DECIMAL_TO_SCALE(TYPE) \ + FORCE_INLINE \ + gdv_##TYPE round_##TYPE##_int32(gdv_##TYPE number, gdv_int32 out_scale) { \ + gdv_float64 scale_multiplier = get_scale_multiplier(out_scale); \ + if (!(fabs(number) < kMaxFractionalDouble / scale_multiplier)) { \ + return number; \ + } \ + gdv_float64 scaled = number * scale_multiplier; \ + return static_cast(trunc(scaled + ((number >= 0) ? 0.5 : -0.5)) / \ + scale_multiplier); \ } ROUND_DECIMAL_TO_SCALE(float32) diff --git a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc index e7a0f74c1b45..fccd40157e11 100644 --- a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc +++ b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc @@ -153,6 +153,15 @@ TEST(TestExtendedMathOps, TestRoundDecimal) { EXPECT_FLOAT_EQ(round_float32_int32(static_cast(9.134123), 2), 9.13f); EXPECT_FLOAT_EQ(round_float32_int32(static_cast(-1.923), 1), -1.9f); + const float max_float = std::numeric_limits::max(); + const float lowest_float = std::numeric_limits::lowest(); + EXPECT_EQ(round_float32_int32(max_float, 100), max_float); + EXPECT_EQ(round_float32_int32(lowest_float, 100), lowest_float); + EXPECT_EQ(round_float32_int32(1.0e30f, 2), 1.0e30f); + EXPECT_EQ(round_float32_int32(451.65f, 100), 451.65f); + EXPECT_EQ(round_float32_int32(-451.65f, 100), -451.65f); + EXPECT_EQ(round_float32_int32(16777216.0f, 2), 16777216.0f); + VerifyFuzzyEquals(round_float64(1234.245), 1234); VerifyFuzzyEquals(round_float64(-11.7892), -12); VerifyFuzzyEquals(round_float64(1.4999999), 1); @@ -165,6 +174,29 @@ TEST(TestExtendedMathOps, TestRoundDecimal) { EXPECT_EQ(std::signbit(round_float64_int32(0, -2)), 0); VerifyFuzzyEquals(round_float64_int32((double)INT_MAX + 1, 0), (double)INT_MAX + 1); VerifyFuzzyEquals(round_float64_int32((double)INT_MIN - 1, 0), (double)INT_MIN - 1); + + const double max_double = std::numeric_limits::max(); + const double lowest_double = std::numeric_limits::lowest(); + EXPECT_EQ(round_float64_int32(2.29e208, 100), 2.29e208); + EXPECT_EQ(round_float64_int32(-2.29e208, 100), -2.29e208); + EXPECT_EQ(round_float64_int32(max_double, 100), max_double); + EXPECT_EQ(round_float64_int32(lowest_double, 100), lowest_double); + EXPECT_EQ(round_float64_int32(451.65, 100), 451.65); + EXPECT_EQ(round_float64_int32(-451.65, 100), -451.65); + + const double ticket_val = 2.2926114935177252E208; + EXPECT_EQ(round_float64_int32(ticket_val, 100), ticket_val); + EXPECT_EQ(round_float64_int32(-ticket_val, 100), -ticket_val); + for (gdv_int32 scale : {3, 4, 7, 8, 10, -100}) { + EXPECT_EQ(round_float64_int32(ticket_val, scale), ticket_val); + EXPECT_EQ(round_float64_int32(-ticket_val, scale), -ticket_val); + } + EXPECT_EQ(round_float64_int32(1.5, -100), 0); + + const double limit = std::ldexp(1.0, 52) / 100.0; + const double just_above = std::nextafter(limit, max_double); + EXPECT_EQ(round_float64_int32(limit, 2), limit); + EXPECT_EQ(round_float64_int32(just_above, 2), just_above); } TEST(TestExtendedMathOps, TestBRoundDecimal) { From 59b548e5e74945f0e6554ad7ec2d07fa3c064598 Mon Sep 17 00:00:00 2001 From: Victor Zoubok Date: Fri, 16 Oct 2026 12:44:00 -0700 Subject: [PATCH 2/2] DX-90291: Rename round limit constant, document float32 use --- cpp/src/gandiva/precompiled/extended_math_ops.cc | 7 ++++--- .../gandiva/precompiled/extended_math_ops_test.cc | 13 +++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cpp/src/gandiva/precompiled/extended_math_ops.cc b/cpp/src/gandiva/precompiled/extended_math_ops.cc index a6acccb9dbe1..879baeeb76b7 100644 --- a/cpp/src/gandiva/precompiled/extended_math_ops.cc +++ b/cpp/src/gandiva/precompiled/extended_math_ops.cc @@ -304,15 +304,16 @@ gdv_float64 bround_float64(gdv_float64 num) { return num - diff_num; } -// doubles at or above 2^52 and have no fractional part, so no point to truncate -static const gdv_float64 kMaxFractionalDouble = static_cast(1LL << 52); +// limit for abs(number * 10^scale_multiplier), computed in double - works for float32 and float64 versions. +// at or above 2^52 double (scaled) has no fractional part, so there is nothing to round +static const gdv_float64 kMaxRoundableScaledValue = static_cast(1LL << 52); // rounds the number to the given scale // every ordered comparison with NaN is false: <, <=, >, >= and == #define ROUND_DECIMAL_TO_SCALE(TYPE) \ FORCE_INLINE \ gdv_##TYPE round_##TYPE##_int32(gdv_##TYPE number, gdv_int32 out_scale) { \ gdv_float64 scale_multiplier = get_scale_multiplier(out_scale); \ - if (!(fabs(number) < kMaxFractionalDouble / scale_multiplier)) { \ + if (!(fabs(number) < kMaxRoundableScaledValue / scale_multiplier)) { \ return number; \ } \ gdv_float64 scaled = number * scale_multiplier; \ diff --git a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc index fccd40157e11..df5dce490b2a 100644 --- a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc +++ b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc @@ -162,6 +162,11 @@ TEST(TestExtendedMathOps, TestRoundDecimal) { EXPECT_EQ(round_float32_int32(-451.65f, 100), -451.65f); EXPECT_EQ(round_float32_int32(16777216.0f, 2), 16777216.0f); + const float limit_float = static_cast(std::ldexp(1.0, 52) / 100.0); + const float just_above_float = std::nextafter(limit_float, max_float); + EXPECT_EQ(round_float32_int32(limit_float, 2), limit_float); + EXPECT_EQ(round_float32_int32(just_above_float, 2), just_above_float); + VerifyFuzzyEquals(round_float64(1234.245), 1234); VerifyFuzzyEquals(round_float64(-11.7892), -12); VerifyFuzzyEquals(round_float64(1.4999999), 1); @@ -193,10 +198,10 @@ TEST(TestExtendedMathOps, TestRoundDecimal) { } EXPECT_EQ(round_float64_int32(1.5, -100), 0); - const double limit = std::ldexp(1.0, 52) / 100.0; - const double just_above = std::nextafter(limit, max_double); - EXPECT_EQ(round_float64_int32(limit, 2), limit); - EXPECT_EQ(round_float64_int32(just_above, 2), just_above); + const double limit_double = std::ldexp(1.0, 52) / 100.0; + const double just_above_double = std::nextafter(limit_double, max_double); + EXPECT_EQ(round_float64_int32(limit_double, 2), limit_double); + EXPECT_EQ(round_float64_int32(just_above_double, 2), just_above_double); } TEST(TestExtendedMathOps, TestBRoundDecimal) {