vvellanki commented on a change in pull request #11415:
URL: https://github.com/apache/arrow/pull/11415#discussion_r743398093
##########
File path: cpp/src/gandiva/function_registry_arithmetic.cc
##########
@@ -107,6 +107,13 @@ 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
+ NativeFunction("bround", {}, DataTypeVector{float64()}, float64(),
+ kResultNullIfNull, "bround_float64"),
+
+ NativeFunction("bround", {}, DataTypeVector{float64(), int32()},
float64(),
Review comment:
Lets not add this in this PR. Please open a separate ticket for this and
create a separate PR.
##########
File path: cpp/src/gandiva/precompiled/extended_math_ops_test.cc
##########
@@ -120,6 +120,46 @@ TEST(TestExtendedMathOps, TestRoundDecimal) {
VerifyFuzzyEquals(round_float64_int32((double)INT_MIN - 1, 0),
(double)INT_MIN - 1);
}
+TEST(TestExtendedMathOps, TestBRoundDecimal) {
+ EXPECT_DOUBLE_EQ(bround_float64(0.0), 0);
+ EXPECT_DOUBLE_EQ(bround_float64(2.5), 2);
+ EXPECT_DOUBLE_EQ(bround_float64(3.5), 4);
+ EXPECT_DOUBLE_EQ(bround_float64(-2.5), -2);
+ EXPECT_DOUBLE_EQ(bround_float64(-3.5), -4);
+ EXPECT_DOUBLE_EQ(bround_float64(1.4999999), 1);
+ EXPECT_EQ(std::signbit(bround_float64(0)), 0);
+ EXPECT_EQ(std::signbit(bround_float64(0)), 0);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(8.25, 1), 8.2);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(8.35, 1), 8.4);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(6.225, 2), 6.22);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(8.335, 2), 8.34);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(-8.25, 1), -8.2);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(-8.35, 1), -8.4);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(-6.225, 2), -6.22);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(-8.335, 2), -8.34);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(-6.225, -2), -6.22);
+ EXPECT_DOUBLE_EQ(bround_float64_int32(8.335, -2), 8.34);
+
+ VerifyFuzzyEquals(bround_float64(2.5), 2);
Review comment:
What is the difference between VerifyFuzzyEquals and EXPECT_DOUBLE_EQ?
##########
File path: cpp/src/gandiva/precompiled/extended_math_ops_test.cc
##########
@@ -120,6 +120,46 @@ TEST(TestExtendedMathOps, TestRoundDecimal) {
VerifyFuzzyEquals(round_float64_int32((double)INT_MIN - 1, 0),
(double)INT_MIN - 1);
}
+TEST(TestExtendedMathOps, TestBRoundDecimal) {
+ EXPECT_DOUBLE_EQ(bround_float64(0.0), 0);
+ EXPECT_DOUBLE_EQ(bround_float64(2.5), 2);
+ EXPECT_DOUBLE_EQ(bround_float64(3.5), 4);
+ EXPECT_DOUBLE_EQ(bround_float64(-2.5), -2);
+ EXPECT_DOUBLE_EQ(bround_float64(-3.5), -4);
+ EXPECT_DOUBLE_EQ(bround_float64(1.4999999), 1);
+ EXPECT_EQ(std::signbit(bround_float64(0)), 0);
+ EXPECT_EQ(std::signbit(bround_float64(0)), 0);
Review comment:
This is same as the test in line 130
##########
File path: cpp/src/gandiva/precompiled/extended_math_ops.cc
##########
@@ -249,6 +265,26 @@ ROUND_DECIMAL(float64)
ROUND_DECIMAL_TO_SCALE(float32)
ROUND_DECIMAL_TO_SCALE(float64)
+// rounds the number to the given scale
+FORCE_INLINE
+gdv_float64 bround_float64_int32(gdv_float64 num, gdv_int32 out_scale) {
+ if (out_scale < 0) {
+ out_scale = out_scale * (-1);
+ }
+ gdv_float64 scale_multiplier = get_scale_multiplier(out_scale);
+ gdv_float64 to_round = num * scale_multiplier;
Review comment:
This can cause an overflow. This implementation has to be thought
through - please do it in a separate PR
--
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
