vvellanki commented on a change in pull request #11415: URL: https://github.com/apache/arrow/pull/11415#discussion_r741289776
########## File path: cpp/src/gandiva/function_registry_arithmetic.cc ########## @@ -107,6 +107,12 @@ std::vector<NativeFunction> GetArithmeticFunctionRegistry() { BINARY_GENERIC_SAFE_NULL_IF_NULL(round, {}, int32, int32, int32), BINARY_GENERIC_SAFE_NULL_IF_NULL(round, {}, int64, int32, int64), + // bround functions + UNARY_SAFE_NULL_IF_NULL(bround, {}, float32, float32), Review comment: I dont see a bround function on floats in the Hive UDFs document - https://cwiki.apache.org/confluence/display/hive/languagemanual+udf ########## File path: cpp/src/gandiva/precompiled/extended_math_ops.cc ########## @@ -236,6 +237,25 @@ gdv_int64 round_int64(gdv_int64 num) { return num; } ROUND_DECIMAL(float32) ROUND_DECIMAL(float64) +// rounds the number to the nearest integer +#define BROUND_DECIMAL(TYPE) \ + FORCE_INLINE \ + gdv_##TYPE bround_##TYPE(gdv_##TYPE num) { \ + const gdv_float64 round_num = round(num); \ + const gdv_float64 dif_num = round_num - num; \ Review comment: typo - please use diff_num ########## File path: cpp/src/gandiva/precompiled/extended_math_ops.cc ########## @@ -236,6 +237,25 @@ gdv_int64 round_int64(gdv_int64 num) { return num; } ROUND_DECIMAL(float32) ROUND_DECIMAL(float64) +// rounds the number to the nearest integer +#define BROUND_DECIMAL(TYPE) \ + FORCE_INLINE \ + gdv_##TYPE bround_##TYPE(gdv_##TYPE num) { \ + const gdv_float64 round_num = round(num); \ + const gdv_float64 dif_num = round_num - num; \ + if ((dif_num != 0.5f) && (dif_num != -0.5f)) { \ Review comment: Should be 0.5f or 0.5d since dif_num is a double? ########## File path: cpp/src/gandiva/precompiled/extended_math_ops_test.cc ########## @@ -120,6 +120,50 @@ TEST(TestExtendedMathOps, TestRoundDecimal) { VerifyFuzzyEquals(round_float64_int32((double)INT_MIN - 1, 0), (double)INT_MIN - 1); } +TEST(TestExtendedMathOps, TestBRoundDecimal) { + EXPECT_FLOAT_EQ(bround_float32(0.0f), 0); + EXPECT_FLOAT_EQ(bround_float32(2.5f), 2); + EXPECT_FLOAT_EQ(bround_float32(3.5f), 4); + EXPECT_FLOAT_EQ(bround_float32(-2.5f), -2); + EXPECT_FLOAT_EQ(bround_float32(-3.5f), -4); + EXPECT_FLOAT_EQ(bround_float32(1.4999999f), 1); + EXPECT_EQ(std::signbit(bround_float32(0)), 0); + EXPECT_EQ(std::signbit(bround_float64(0)), 0); + EXPECT_FLOAT_EQ(bround_float32_int32(8.25f, 1), 8.2f); + EXPECT_FLOAT_EQ(bround_float32_int32(8.35f, 1), 8.4f); + EXPECT_FLOAT_EQ(bround_float32_int32(6.225f, 2), 6.22f); + EXPECT_FLOAT_EQ(bround_float32_int32(8.335f, 2), 8.34f); + EXPECT_FLOAT_EQ(bround_float32_int32(-8.25f, 1), -8.2f); + EXPECT_FLOAT_EQ(bround_float32_int32(-8.35f, 1), -8.4f); + EXPECT_FLOAT_EQ(bround_float32_int32(-6.225f, 2), -6.22f); + EXPECT_FLOAT_EQ(bround_float32_int32(-8.335f, 2), -8.34f); + EXPECT_FLOAT_EQ(bround_float32_int32(-6.225f,-2), -6.22f); + EXPECT_FLOAT_EQ(bround_float32_int32(8.335f, -2), 8.34f); + + VerifyFuzzyEquals(bround_float32(2.5f), 2); + VerifyFuzzyEquals(bround_float32(3.5f), 4); + VerifyFuzzyEquals(bround_float32(-2.5f), -2); + VerifyFuzzyEquals(bround_float32(-3.5f), -4); + VerifyFuzzyEquals(bround_float32(1.4999999f), 1); + VerifyFuzzyEquals(bround_float64(2.5), 2); + VerifyFuzzyEquals(bround_float64(3.5), 4); + VerifyFuzzyEquals(bround_float64(-2.5), -2); + VerifyFuzzyEquals(bround_float64(-3.5), -4); + VerifyFuzzyEquals(bround_float64(1.4999999), 1); Review comment: Please add a test for bround_float64(1.50001) = 2 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org