ARROW-10234: [C++][Gandiva] Fix logic of round() for floats/decimals in Gandiva

sgnkc · sgnkc · commit b1ef5438a468 · 2020-10-08T17:48:42.000+05:30
This patch attempts to resolve the bug introduced by the addition of round() in ARROW-9641

round() for floats is returning incorrect results for some edge cases, like round(cast(1.55 as float), 1) gives 1.6, but it should be 1.5, since the result after casting 1.55 to float comes to 1.5499999523162842, due to inaccurate representation of floating point numbers in memory.

Removing an intermediate explicit cast to float statement for a double value, which is used in subsequent computations, minimises the error introduced due to the incorrect representation.
diff --git a/cpp/src/gandiva/precompiled/extended_math_ops.cc b/cpp/src/gandiva/precompiled/extended_math_ops.cc
@@ -116,8 +116,7 @@ POWER(float64, float64, float64)
 #define ROUND_DECIMAL(TYPE)                                                 \
   FORCE_INLINE                                                              \
   gdv_##TYPE round_##TYPE##_int32(gdv_##TYPE number, gdv_int32 out_scale) { \
-    gdv_##TYPE scale_multiplier =                                           \
-        static_cast<gdv_##TYPE>(get_scale_multiplier(out_scale));           \
+    gdv_float64 scale_multiplier = get_scale_multiplier(out_scale);         \
     return static_cast<gdv_##TYPE>(                                         \
         trunc(number * scale_multiplier + ((number > 0) ? 0.5 : -0.5)) /    \
         scale_multiplier);                                                  \
diff --git a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc
@@ -93,6 +93,10 @@ TEST(TestExtendedMathOps, TestRoundDecimal) {
   EXPECT_FLOAT_EQ(round_float32_int32(-1234.4567f, 3), -1234.457f);
   EXPECT_FLOAT_EQ(round_float32_int32(-1234.4567f, -3), -1000);
   EXPECT_FLOAT_EQ(round_float32_int32(1234.4567f, 0), 1234);
+  EXPECT_FLOAT_EQ(round_float32_int32(1.5499999523162842f, 1), 1.5f);
+  EXPECT_FLOAT_EQ(round_float32_int32(static_cast<float>(1.55), 1), 1.5f);
+  EXPECT_FLOAT_EQ(round_float32_int32(static_cast<float>(9.134123), 2), 9.13f);
+  EXPECT_FLOAT_EQ(round_float32_int32(static_cast<float>(-1.923), 1), -1.9f);
 
   VerifyFuzzyEquals(round_float64_int32(1234.789, 2), 1234.79);
   VerifyFuzzyEquals(round_float64_int32(1234.12345, -3), 1000);
