This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 9b1fbede84e GH-50641: [C++][Compute] Fix correctness error in decimal
round_binary kernel (#50642)
9b1fbede84e is described below
commit 9b1fbede84e54ef27ceda41869bc24ea03ad8b78
Author: Daniel Anderson <[email protected]>
AuthorDate: Tue Jul 28 01:36:16 2026 -0700
GH-50641: [C++][Compute] Fix correctness error in decimal round_binary
kernel (#50642)
### Rationale for this change
There are major correctness bugs in the `round_binary` kernel for decimal
input. #50641 describes them in more detail. Essentially, the midpoint
`half_pow10` of the rounding range is computed outside of the loop and assumes
that `ndigits` == 0. The consequence is that for round-to-nearest modes,
`round_binary` will frequently give wrong answers when `ndigits` != 0. Also as
a result of assuming `ndigits` is always 0, the function does not attempt to
round negative scale inputs.
There is also a correctness issue with the decimal implementation of the
HALF_TO_ODD rounding mode, which applies both to the `round` and `round_binary`
compute functions when the input is negative. In this case, the function does
round to an odd, but rounds to the closest odd greater than the input rather
than the nearest odd.
### What changes are included in this PR?
The mentioned bugs are fixed. For `round_binary`, we make sure to compute
`pow` and `half_pow10` inside `RoundBinary::Call` where we know the row value
of `ndigits`.
To fix the HALF_TO_ODD mode, we check whether the sign of the remainder is
positive (1) instead of the truthy value of the sign (which turns out to be
always `true` since `remainder.Sign()` is either 1 or -1. This matches the
HALF_TO_EVEN implementation.
Added decimal `round_binary` unit tests to
`scalar_round_arithmetic_test.cc`. They were absent up to this point which is
why the bug wasn't caught previously. I did some refactoring and added a couple
structs to make it easier to share test data between
TestUnaryRoundArithmeticDecimal and the new TestBinaryRoundArithmeticDecimal.
Those test cases use a constant `ndigits` for every row, but serve to verify
that `round_binary` works as well as `round` if used in the same way. I also
cre [...]
### Are these changes tested?
Yes. The new TestBinaryRoundArithmeticDecimal tests provide relatively
extensive coverage of the `round_binary` kernel. I confirmed that they failed
without the changes and passed with the fixes in place. There are a couple
numbers included in the `TestBinaryRoundArithmeticDecimal.RoundNDigitsArray`
test data that require the HALF_TO_ODD fix to be rounded correctly.
Besides that, I ran the main Arrow C++ unit tests and Arrow compute tests
to ensure they pass.
### Are there any user-facing changes?
No user-facing changes.
**This PR contains a "Critical Fix".** This PR fixes bugs in the decimal
`round_binary` kernel and decimal HALF_TO_ODD rounding mode which cause
incorrect data to be produced, even for the most common of inputs.
* GitHub Issue: #50641
Lead-authored-by: Daniel Anderson <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/compute/kernels/scalar_round.cc | 24 +-
.../kernels/scalar_round_arithmetic_test.cc | 520 +++++++++++++--------
2 files changed, 326 insertions(+), 218 deletions(-)
diff --git a/cpp/src/arrow/compute/kernels/scalar_round.cc
b/cpp/src/arrow/compute/kernels/scalar_round.cc
index 208b9875a1c..f762f9e0fdd 100644
--- a/cpp/src/arrow/compute/kernels/scalar_round.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_round.cc
@@ -391,7 +391,7 @@ struct RoundImpl<Type, RoundMode::HALF_TO_ODD> {
const T& pow10, const int32_t
scale) {
auto scaled = val->ReduceScaleBy(scale, /*round=*/false);
if (scaled.low_bits() % 2 == 0) {
- scaled += remainder.Sign() ? 1 : -1;
+ scaled += remainder.Sign() >= 0 ? 1 : -1;
}
*val = scaled.IncreaseScaleBy(scale);
}
@@ -915,29 +915,20 @@ struct RoundBinary<ArrowType, kRoundMode,
enable_if_decimal<ArrowType>> {
using CType = typename TypeTraits<ArrowType>::CType;
using State = RoundOptionsWrapper<RoundBinaryOptions, double>;
const ArrowType& ty;
- int32_t pow;
- // pow10 is "1" for the given decimal scale. Similarly half_pow10 is "0.5".
- CType half_pow10, neg_half_pow10;
explicit RoundBinary(const State& state, const DataType& out_ty)
: RoundBinary(out_ty) {}
explicit RoundBinary(const DataType& out_ty)
- : ty(checked_cast<const ArrowType&>(out_ty)),
- pow(static_cast<int32_t>(ty.scale() - 0)) {
- if (pow >= ty.precision() || pow < 0) {
- half_pow10 = neg_half_pow10 = 0;
- } else {
- half_pow10 = CType::GetHalfScaleMultiplier(pow);
- neg_half_pow10 = -half_pow10;
- }
- }
+ : ty(checked_cast<const ArrowType&>(out_ty)) {}
template <typename T = ArrowType, typename CType0 = typename
TypeTraits<T>::CType0,
typename CType1 = typename TypeTraits<T>::CType1>
enable_if_decimal_value<CType> Call(KernelContext* ctx, CType0 arg0, CType1
arg1,
Status* st) const {
- if (pow - arg1 >= ty.precision()) {
+ int32_t pow = static_cast<int32_t>(ty.scale() - arg1);
+
+ if (pow >= ty.precision()) {
*st = Status::Invalid("Rounding to ", arg1, " digits will not fit in
precision of ",
ty);
return 0;
@@ -946,7 +937,8 @@ struct RoundBinary<ArrowType, kRoundMode,
enable_if_decimal<ArrowType>> {
return arg0;
}
- CType0 pow10 = CType0::GetScaleMultiplier(static_cast<int32_t>(ty.scale()
- arg1));
+ // pow10 is "1" for the given decimal scale. Similarly half_pow10 is "0.5".
+ CType0 pow10 = CType0::GetScaleMultiplier(pow);
std::pair<CType, CType> pair;
*st = arg0.Divide(pow10).Value(&pair);
@@ -955,6 +947,8 @@ struct RoundBinary<ArrowType, kRoundMode,
enable_if_decimal<ArrowType>> {
const auto& remainder = pair.second;
if (remainder == 0) return arg0;
if (kRoundMode >= RoundMode::HALF_DOWN) {
+ CType0 half_pow10 = CType::GetHalfScaleMultiplier(pow);
+ CType0 neg_half_pow10 = -half_pow10;
if (remainder == half_pow10 || remainder == neg_half_pow10) {
// On the halfway point, use tiebreaker
RoundImpl<CType0, kRoundMode>::Round(&arg0, remainder, pow10, pow);
diff --git a/cpp/src/arrow/compute/kernels/scalar_round_arithmetic_test.cc
b/cpp/src/arrow/compute/kernels/scalar_round_arithmetic_test.cc
index a572af4195c..1e9e0c9d50c 100644
--- a/cpp/src/arrow/compute/kernels/scalar_round_arithmetic_test.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_round_arithmetic_test.cc
@@ -507,218 +507,188 @@ TEST(TestUnaryRound, DispatchBestRound) {
}
}
-class TestUnaryRoundArithmeticDecimal : public TestRoundArithmeticDecimal {};
+enum class RoundTestCheckFunc {
+ kCheckScalar,
+ kCheckRaises,
+};
+
+// Const ndigits test case used by both TestUnaryRoundArithmeticDecimal and
+// TestBinaryRoundArithmeticDecimal
+struct RoundDecimalTestCase {
+ RoundTestCheckFunc check_func;
+ int32_t ndigits;
+ RoundMode round_mode;
+ std::string expected; // Expected result array or expected error msg
+};
+
+struct RoundDecimalTestVariant {
+ std::vector<std::shared_ptr<DataType>> decimal_types;
+ std::string input_json;
+ std::vector<RoundDecimalTestCase> cases;
+ bool scientific_data = false; // Whether to use DecimalArrayFromJSON
+};
+
+std::vector<RoundDecimalTestVariant> GetRoundDecimalTestCases() {
+ using enum RoundTestCheckFunc;
+ return {
+ {{decimal128(4, 3), decimal256(4, 3)},
+ R"(["1.010", "1.012", "1.015", "1.019", "-1.010", "-1.012", "-1.015",
"-1.019", null])",
+ {{kCheckScalar, 2, RoundMode::DOWN,
+ R"(["1.010", "1.010", "1.010", "1.010", "-1.010", "-1.020", "-1.020",
"-1.020", null])"},
+ {kCheckScalar, 2, RoundMode::UP,
+ R"(["1.010", "1.020", "1.020", "1.020", "-1.010", "-1.010", "-1.010",
"-1.010", null])"},
+ {kCheckScalar, 2, RoundMode::TOWARDS_ZERO,
+ R"(["1.010", "1.010", "1.010", "1.010", "-1.010", "-1.010", "-1.010",
"-1.010", null])"},
+ {kCheckScalar, 2, RoundMode::TOWARDS_INFINITY,
+ R"(["1.010", "1.020", "1.020", "1.020", "-1.010", "-1.020", "-1.020",
"-1.020", null])"},
+ {kCheckScalar, 2, RoundMode::HALF_DOWN,
+ R"(["1.010", "1.010", "1.010", "1.020", "-1.010", "-1.010", "-1.020",
"-1.020", null])"},
+ {kCheckScalar, 2, RoundMode::HALF_UP,
+ R"(["1.010", "1.010", "1.020", "1.020", "-1.010", "-1.010", "-1.010",
"-1.020", null])"},
+ {kCheckScalar, 2, RoundMode::HALF_TOWARDS_ZERO,
+ R"(["1.010", "1.010", "1.010", "1.020", "-1.010", "-1.010", "-1.010",
"-1.020", null])"},
+ {kCheckScalar, 2, RoundMode::HALF_TOWARDS_INFINITY,
+ R"(["1.010", "1.010", "1.020", "1.020", "-1.010", "-1.010", "-1.020",
"-1.020", null])"},
+ {kCheckScalar, 2, RoundMode::HALF_TO_EVEN,
+ R"(["1.010", "1.010", "1.020", "1.020", "-1.010", "-1.010", "-1.020",
"-1.020", null])"},
+ {kCheckScalar, 2, RoundMode::HALF_TO_ODD,
+ R"(["1.010", "1.010", "1.010", "1.020", "-1.010", "-1.010", "-1.010",
"-1.020", null])"}}}};
+}
+
+std::vector<RoundDecimalTestVariant> GetRoundTowardsInfinityDecimalTestCases()
{
+ using enum RoundTestCheckFunc;
+ return {{{decimal128(4, 2), decimal256(4, 2)},
+ R"(["1.00", "1.99", "1.01", "-42.00", "-42.99", "-42.15", null])",
+ {
+ {kCheckScalar, 0, RoundMode::TOWARDS_INFINITY,
+ R"(["1.00", "2.00", "2.00", "-42.00", "-43.00", "-43.00",
null])"},
+ {kCheckScalar, 1, RoundMode::TOWARDS_INFINITY,
+ R"(["1.00", "2.00", "1.10", "-42.00", "-43.00", "-42.20",
null])"},
+ {kCheckScalar, 2, RoundMode::TOWARDS_INFINITY,
+ R"(["1.00", "1.99", "1.01", "-42.00", "-42.99", "-42.15",
null])"},
+ {kCheckScalar, 4, RoundMode::TOWARDS_INFINITY,
+ R"(["1.00", "1.99", "1.01", "-42.00", "-42.99", "-42.15",
null])"},
+ {kCheckScalar, 100, RoundMode::TOWARDS_INFINITY,
+ R"(["1.00", "1.99", "1.01", "-42.00", "-42.99", "-42.15",
null])"},
+ {kCheckScalar, -1, RoundMode::TOWARDS_INFINITY,
+ R"(["10.00", "10.00", "10.00", "-50.00", "-50.00", "-50.00",
null])"},
+ {kCheckRaises, -2, RoundMode::TOWARDS_INFINITY,
+ "Rounding to -2 digits will not fit in precision"},
+ }},
+ {{decimal128(4, 2), decimal256(4, 2)},
+ R"(["99.99"])",
+ {{kCheckRaises, -1, RoundMode::TOWARDS_INFINITY,
+ "Rounded value 100.00 does not fit in precision"}}},
+ {{decimal128(2, -2), decimal256(2, -2)},
+ R"(["10E2", "12E2", "18E2", "-10E2", "-12E2", "-18E2", null])",
+ {{kCheckScalar, 0, RoundMode::TOWARDS_INFINITY,
+ R"(["10E2", "12E2", "18E2", "-10E2", "-12E2", "-18E2", null])"},
+ {kCheckScalar, 2, RoundMode::TOWARDS_INFINITY,
+ R"(["10E2", "12E2", "18E2", "-10E2", "-12E2", "-18E2", null])"},
+ {kCheckScalar, 100, RoundMode::TOWARDS_INFINITY,
+ R"(["10E2", "12E2", "18E2", "-10E2", "-12E2", "-18E2", null])"},
+ {kCheckScalar, -1, RoundMode::TOWARDS_INFINITY,
+ R"(["10E2", "12E2", "18E2", "-10E2", "-12E2", "-18E2", null])"},
+ {kCheckScalar, -2, RoundMode::TOWARDS_INFINITY,
+ R"(["10E2", "12E2", "18E2", "-10E2", "-12E2", "-18E2", null])"},
+ {kCheckScalar, -3, RoundMode::TOWARDS_INFINITY,
+ R"(["10E2", "20E2", "20E2", "-10E2", "-20E2", "-20E2", null])"},
+ {kCheckRaises, -4, RoundMode::TOWARDS_INFINITY,
+ "Rounding to -4 digits will not fit in precision"}},
+ true}};
+}
+
+std::vector<RoundDecimalTestVariant> GetRoundHalfToEvenDecimalTestCases() {
+ using enum RoundTestCheckFunc;
+ return {
+ {{decimal128(4, 2), decimal256(4, 2)},
+ R"(["1.00", "5.99", "1.01", "-42.00", "-42.99", "-42.15", "1.50",
"2.50", "-5.50", "-2.55", null])",
+ {
+ {kCheckScalar, 0, RoundMode::HALF_TO_EVEN,
+ R"(["1.00", "6.00", "1.00", "-42.00", "-43.00", "-42.00", "2.00",
"2.00", "-6.00", "-3.00", null])"},
+ {kCheckScalar, 1, RoundMode::HALF_TO_EVEN,
+ R"(["1.00", "6.00", "1.00", "-42.00", "-43.00", "-42.20", "1.50",
"2.50", "-5.50", "-2.60", null])"},
+ {kCheckScalar, 2, RoundMode::HALF_TO_EVEN,
+ R"(["1.00", "5.99", "1.01", "-42.00", "-42.99", "-42.15", "1.50",
"2.50", "-5.50", "-2.55", null])"},
+ {kCheckScalar, 4, RoundMode::HALF_TO_EVEN,
+ R"(["1.00", "5.99", "1.01", "-42.00", "-42.99", "-42.15", "1.50",
"2.50", "-5.50", "-2.55", null])"},
+ {kCheckScalar, 100, RoundMode::HALF_TO_EVEN,
+ R"(["1.00", "5.99", "1.01", "-42.00", "-42.99", "-42.15", "1.50",
"2.50", "-5.50", "-2.55", null])"},
+ {kCheckScalar, -1, RoundMode::HALF_TO_EVEN,
+ R"(["0.00", "10.00", "0.00", "-40.00", "-40.00", "-40.00", "0.00",
"0.00", "-10.00", "0.00", null])"},
+ {kCheckRaises, -2, RoundMode::HALF_TO_EVEN,
+ "Rounding to -2 digits will not fit in precision"},
+ }},
+ {{decimal128(4, 2), decimal256(4, 2)},
+ R"(["99.99"])",
+ {{kCheckRaises, -1, RoundMode::HALF_TO_EVEN,
+ "Rounded value 100.00 does not fit in precision"}}},
+ {{decimal128(2, -2), decimal256(2, -2)},
+ R"(["5E2", "10E2", "12E2", "15E2", "18E2", "-10E2", "-12E2", "-15E2",
"-18E2", null])",
+ {{kCheckScalar, 0, RoundMode::HALF_TO_EVEN,
+ R"(["5E2", "10E2", "12E2", "15E2", "18E2", "-10E2", "-12E2", "-15E2",
"-18E2", null])"},
+ {kCheckScalar, 2, RoundMode::HALF_TO_EVEN,
+ R"(["5E2", "10E2", "12E2", "15E2", "18E2", "-10E2", "-12E2", "-15E2",
"-18E2", null])"},
+ {kCheckScalar, 100, RoundMode::HALF_TO_EVEN,
+ R"(["5E2", "10E2", "12E2", "15E2", "18E2", "-10E2", "-12E2", "-15E2",
"-18E2", null])"},
+ {kCheckScalar, -1, RoundMode::HALF_TO_EVEN,
+ R"(["5E2", "10E2", "12E2", "15E2", "18E2", "-10E2", "-12E2", "-15E2",
"-18E2", null])"},
+ {kCheckScalar, -2, RoundMode::HALF_TO_EVEN,
+ R"(["5E2", "10E2", "12E2", "15E2", "18E2", "-10E2", "-12E2", "-15E2",
"-18E2", null])"},
+ {kCheckScalar, -3, RoundMode::HALF_TO_EVEN,
+ R"(["0", "10E2", "10E2", "20E2", "20E2", "-10E2", "-10E2", "-20E2",
"-20E2", null])"},
+ {kCheckRaises, -4, RoundMode::HALF_TO_EVEN,
+ "Rounding to -4 digits will not fit in precision"}},
+ true}};
+}
+
+class TestUnaryRoundArithmeticDecimal : public TestRoundArithmeticDecimal {
+ protected:
+ void RunUnaryRoundDecimalTestVariants(
+ const std::string& round_func,
+ const std::vector<RoundDecimalTestVariant>& decimal_test_variants) {
+ RoundOptions options;
+ for (const auto& variant : decimal_test_variants) {
+ for (std::shared_ptr<DataType> ty : variant.decimal_types) {
+ std::shared_ptr<Array> values;
+ if (variant.scientific_data) {
+ values = DecimalArrayFromJSON(ty, variant.input_json);
+ } else {
+ values = ArrayFromJSON(ty, variant.input_json);
+ }
+
+ for (const auto& test_case : variant.cases) {
+ options.ndigits = test_case.ndigits;
+ options.round_mode = test_case.round_mode;
+
+ if (test_case.check_func == RoundTestCheckFunc::kCheckScalar) {
+ std::shared_ptr<Array> expected_arr;
+ if (variant.scientific_data) {
+ expected_arr = DecimalArrayFromJSON(ty, test_case.expected);
+ } else {
+ expected_arr = ArrayFromJSON(ty, test_case.expected);
+ }
+ CheckScalar(round_func, {values}, expected_arr, &options);
+ } else { // test_case.check_func == RoundTestCheckFunc.kCheckRaises
+ CheckRaises(round_func, {values}, test_case.expected, &options);
+ }
+ }
+ }
+ }
+ }
+};
// Check two modes exhaustively, give all modes a simple test
TEST_F(TestUnaryRoundArithmeticDecimal, Round) {
- const auto func = "round";
- RoundOptions options(2, RoundMode::DOWN);
- for (const auto& ty : {decimal128(4, 3), decimal256(4, 3)}) {
- auto values = ArrayFromJSON(
- ty,
- R"(["1.010", "1.012", "1.015", "1.019", "-1.010", "-1.012", "-1.015",
"-1.019", null])");
- options.round_mode = RoundMode::DOWN;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.010", "1.010", "1.010", "-1.010", "-1.020",
"-1.020", "-1.020", null])"),
- &options);
- options.round_mode = RoundMode::UP;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.020", "1.020", "1.020", "-1.010", "-1.010",
"-1.010", "-1.010", null])"),
- &options);
- options.round_mode = RoundMode::TOWARDS_ZERO;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.010", "1.010", "1.010", "-1.010", "-1.010",
"-1.010", "-1.010", null])"),
- &options);
- options.round_mode = RoundMode::TOWARDS_INFINITY;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.020", "1.020", "1.020", "-1.010", "-1.020",
"-1.020", "-1.020", null])"),
- &options);
- options.round_mode = RoundMode::HALF_DOWN;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.010", "1.010", "1.020", "-1.010", "-1.010",
"-1.020", "-1.020", null])"),
- &options);
- options.round_mode = RoundMode::HALF_UP;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.010", "1.020", "1.020", "-1.010", "-1.010",
"-1.010", "-1.020", null])"),
- &options);
- options.round_mode = RoundMode::HALF_TOWARDS_ZERO;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.010", "1.010", "1.020", "-1.010", "-1.010",
"-1.010", "-1.020", null])"),
- &options);
- options.round_mode = RoundMode::HALF_TOWARDS_INFINITY;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.010", "1.020", "1.020", "-1.010", "-1.010",
"-1.020", "-1.020", null])"),
- &options);
- options.round_mode = RoundMode::HALF_TO_EVEN;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.010", "1.020", "1.020", "-1.010", "-1.010",
"-1.020", "-1.020", null])"),
- &options);
- options.round_mode = RoundMode::HALF_TO_ODD;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.010", "1.010", "1.010", "1.020", "-1.010", "-1.010",
"-1.010", "-1.020", null])"),
- &options);
- }
+ RunUnaryRoundDecimalTestVariants("round", GetRoundDecimalTestCases());
}
TEST_F(TestUnaryRoundArithmeticDecimal, RoundTowardsInfinity) {
- const auto func = "round";
- RoundOptions options(0, RoundMode::TOWARDS_INFINITY);
- for (const auto& ty : {decimal128(4, 2), decimal256(4, 2)}) {
- auto values = ArrayFromJSON(
- ty, R"(["1.00", "1.99", "1.01", "-42.00", "-42.99", "-42.15", null])");
- CheckScalar(func, {ArrayFromJSON(ty, R"([])")}, ArrayFromJSON(ty,
R"([])"), &options);
- options.ndigits = 0;
- CheckScalar(
- func, {values},
- ArrayFromJSON(ty,
- R"(["1.00", "2.00", "2.00", "-42.00", "-43.00",
"-43.00", null])"),
- &options);
- options.ndigits = 1;
- CheckScalar(
- func, {values},
- ArrayFromJSON(ty,
- R"(["1.00", "2.00", "1.10", "-42.00", "-43.00",
"-42.20", null])"),
- &options);
- options.ndigits = 2;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = 4;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = 100;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = -1;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty, R"(["10.00", "10.00", "10.00", "-50.00", "-50.00", "-50.00",
null])"),
- &options);
- options.ndigits = -2;
- CheckRaises(func, {values}, "Rounding to -2 digits will not fit in
precision",
- &options);
- options.ndigits = -1;
- CheckRaises(func, {ArrayFromJSON(ty, R"(["99.99"])")},
- "Rounded value 100.00 does not fit in precision", &options);
- }
- for (const auto& ty : {decimal128(2, -2), decimal256(2, -2)}) {
- auto values = DecimalArrayFromJSON(
- ty, R"(["10E2", "12E2", "18E2", "-10E2", "-12E2", "-18E2", null])");
- options.ndigits = 0;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = 2;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = 100;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = -1;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = -2;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = -3;
- CheckScalar(func, {values},
- DecimalArrayFromJSON(
- ty, R"(["10E2", "20E2", "20E2", "-10E2", "-20E2", "-20E2",
null])"),
- &options);
- options.ndigits = -4;
- CheckRaises(func, {values}, "Rounding to -4 digits will not fit in
precision",
- &options);
- }
+ RunUnaryRoundDecimalTestVariants("round",
GetRoundTowardsInfinityDecimalTestCases());
}
TEST_F(TestUnaryRoundArithmeticDecimal, RoundHalfToEven) {
- const auto func = "round";
- RoundOptions options(0, RoundMode::HALF_TO_EVEN);
- for (const auto& ty : {decimal128(4, 2), decimal256(4, 2)}) {
- auto values = ArrayFromJSON(
- ty,
- R"(["1.00", "5.99", "1.01", "-42.00", "-42.99", "-42.15", "1.50",
"2.50", "-5.50", "-2.55", null])");
- CheckScalar(func, {ArrayFromJSON(ty, R"([])")}, ArrayFromJSON(ty,
R"([])"), &options);
- options.ndigits = 0;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.00", "6.00", "1.00", "-42.00", "-43.00", "-42.00", "2.00",
"2.00", "-6.00", "-3.00", null])"),
- &options);
- options.ndigits = 1;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["1.00", "6.00", "1.00", "-42.00", "-43.00", "-42.20", "1.50",
"2.50", "-5.50", "-2.60", null])"),
- &options);
- options.ndigits = 2;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = 4;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = 100;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = -1;
- CheckScalar(
- func, {values},
- ArrayFromJSON(
- ty,
- R"(["0.00", "10.00", "0.00", "-40.00", "-40.00", "-40.00", "0.00",
"0.00", "-10.00", "0.00", null])"),
- &options);
- options.ndigits = -2;
- CheckRaises(func, {values}, "Rounding to -2 digits will not fit in
precision",
- &options);
- options.ndigits = -1;
- CheckRaises(func, {ArrayFromJSON(ty, R"(["99.99"])")},
- "Rounded value 100.00 does not fit in precision", &options);
- }
- for (const auto& ty : {decimal128(2, -2), decimal256(2, -2)}) {
- auto values = DecimalArrayFromJSON(
- ty,
- R"(["5E2", "10E2", "12E2", "15E2", "18E2", "-10E2", "-12E2", "-15E2",
"-18E2", null])");
- options.ndigits = 0;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = 2;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = 100;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = -1;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = -2;
- CheckScalar(func, {values}, values, &options);
- options.ndigits = -3;
- CheckScalar(
- func, {values},
- DecimalArrayFromJSON(
- ty,
- R"(["0", "10E2", "10E2", "20E2", "20E2", "-10E2", "-10E2",
"-20E2", "-20E2", null])"),
- &options);
- options.ndigits = -4;
- CheckRaises(func, {values}, "Rounding to -4 digits will not fit in
precision",
- &options);
- }
+ RunUnaryRoundDecimalTestVariants("round",
GetRoundHalfToEvenDecimalTestCases());
}
TEST_F(TestUnaryRoundArithmeticDecimal, RoundCeil) {
@@ -1007,6 +977,150 @@ TEST_F(TestUnaryRoundArithmeticDecimal,
RoundToMultipleHalfToOdd) {
}
}
+class TestBinaryRoundArithmeticDecimal : public TestRoundArithmeticDecimal {
+ protected:
+ void RunBinaryRoundDecimalTestVariants(
+ const std::string& round_func,
+ const std::vector<RoundDecimalTestVariant>& decimal_test_variants) {
+ RoundBinaryOptions options;
+ for (const auto& variant : decimal_test_variants) {
+ for (std::shared_ptr<DataType> ty : variant.decimal_types) {
+ std::shared_ptr<Array> values;
+ if (variant.scientific_data) {
+ values = DecimalArrayFromJSON(ty, variant.input_json);
+ } else {
+ values = ArrayFromJSON(ty, variant.input_json);
+ }
+
+ for (const auto& test_case : variant.cases) {
+ options.round_mode = test_case.round_mode;
+
+ ASSERT_OK_AND_ASSIGN(auto ndigits_scalar,
+ arrow::MakeScalar(int32(), test_case.ndigits));
+ ASSERT_OK_AND_ASSIGN(auto ndigits_arr,
+ MakeArrayFromScalar(*ndigits_scalar,
values->length()));
+
+ if (test_case.check_func == RoundTestCheckFunc::kCheckScalar) {
+ std::shared_ptr<Array> expected_arr;
+ if (variant.scientific_data) {
+ expected_arr = DecimalArrayFromJSON(ty, test_case.expected);
+ } else {
+ expected_arr = ArrayFromJSON(ty, test_case.expected);
+ }
+ CheckScalar(round_func, {values, ndigits_arr}, expected_arr,
&options);
+ } else { // test_case.check_func == RoundTestCheckFunc.kCheckRaises
+ CheckRaises(round_func, {values, ndigits_arr}, test_case.expected,
&options);
+ }
+ }
+ }
+ }
+ }
+};
+
+// Check two modes exhaustively, give all modes a simple test
+TEST_F(TestBinaryRoundArithmeticDecimal, Round) {
+ RunBinaryRoundDecimalTestVariants("round_binary",
GetRoundDecimalTestCases());
+}
+
+TEST_F(TestBinaryRoundArithmeticDecimal, RoundTowardsInfinity) {
+ RunBinaryRoundDecimalTestVariants("round_binary",
+ GetRoundTowardsInfinityDecimalTestCases());
+}
+
+TEST_F(TestBinaryRoundArithmeticDecimal, RoundHalfToEven) {
+ RunBinaryRoundDecimalTestVariants("round_binary",
GetRoundHalfToEvenDecimalTestCases());
+}
+
+// Test binary round where ndigits varies per element
+TEST_F(TestBinaryRoundArithmeticDecimal, RoundNDigitsArray) {
+ const auto func = "round_binary";
+ RoundBinaryOptions options;
+
+ for (const auto& ty : {decimal128(4, 2), decimal256(4, 2)}) {
+ auto values =
+ ArrayFromJSON(ty,
+ R"(["61.55", "61.55", "61.52", "60.50", "65.00",
"65.95", "-61.55",
+ "-61.55", "-61.52", "-60.50", "-65.00", "-65.95",
null, "32.78"])");
+ auto ndigits_arr =
+ ArrayFromJSON(int32(), "[2, 1, 1, 0, -1, -1, 2, 1, 1, 0, -1, -1, 5,
null]");
+
+ std::vector<std::pair<RoundMode, std::string>> round_modes_and_expected{{
+ {RoundMode::DOWN,
+ R"(["61.55", "61.50", "61.50", "60.00", "60.00", "60.00", "-61.55",
+ "-61.60", "-61.60", "-61.00", "-70.00", "-70.00", null, null])"},
+ {RoundMode::UP,
+ R"(["61.55", "61.60", "61.60", "61.00", "70.00", "70.00", "-61.55",
+ "-61.50", "-61.50", "-60.00", "-60.00", "-60.00", null, null])"},
+ {RoundMode::TOWARDS_ZERO,
+ R"(["61.55", "61.50", "61.50", "60.00", "60.00", "60.00", "-61.55",
+ "-61.50", "-61.50", "-60.00", "-60.00", "-60.00", null, null])"},
+ {RoundMode::TOWARDS_INFINITY,
+ R"(["61.55", "61.60", "61.60", "61.00", "70.00", "70.00", "-61.55",
+ "-61.60", "-61.60", "-61.00", "-70.00", "-70.00", null, null])"},
+ {RoundMode::HALF_DOWN,
+ R"(["61.55", "61.50", "61.50", "60.00", "60.00", "70.00", "-61.55",
+ "-61.60", "-61.50", "-61.00", "-70.00", "-70.00", null, null])"},
+ {RoundMode::HALF_UP,
+ R"(["61.55", "61.60", "61.50", "61.00", "70.00", "70.00", "-61.55",
+ "-61.50", "-61.50", "-60.00", "-60.00", "-70.00", null, null])"},
+ {RoundMode::HALF_TOWARDS_ZERO,
+ R"(["61.55", "61.50", "61.50", "60.00", "60.00", "70.00", "-61.55",
+ "-61.50", "-61.50", "-60.00", "-60.00", "-70.00", null, null])"},
+ {RoundMode::HALF_TOWARDS_INFINITY,
+ R"(["61.55", "61.60", "61.50", "61.00", "70.00", "70.00", "-61.55",
+ "-61.60", "-61.50", "-61.00", "-70.00", "-70.00", null, null])"},
+ {RoundMode::HALF_TO_EVEN,
+ R"(["61.55", "61.60", "61.50", "60.00", "60.00", "70.00", "-61.55",
+ "-61.60", "-61.50", "-60.00", "-60.00", "-70.00", null, null])"},
+ {RoundMode::HALF_TO_ODD,
+ R"(["61.55", "61.50", "61.50", "61.00", "70.00", "70.00", "-61.55",
+ "-61.50", "-61.50", "-61.00", "-70.00", "-70.00", null, null])"},
+ }};
+
+ for (const auto& pair : round_modes_and_expected) {
+ options.round_mode = pair.first;
+ CheckScalar(func, {values, ndigits_arr}, ArrayFromJSON(ty, pair.second),
&options);
+ }
+ }
+
+ for (const auto& ty : {decimal128(2, -2), decimal256(2, -2)}) {
+ auto values = DecimalArrayFromJSON(
+ ty,
+ R"(["15E2", "15E2", "5E2", "15E2", "18E2", "-15E2", "-15E2", "-5E2",
"-15E2", "-18E2", null, "15E2"])");
+ auto ndigits_arr =
+ ArrayFromJSON(int32(), "[100, -2, -3, -3, -3, 100, -2, -3, -3, -3, 5,
null]");
+
+ std::vector<std::pair<RoundMode, std::string>> round_modes_and_expected{{
+ {RoundMode::DOWN,
+ R"(["15E2", "15E2", "0", "10E2", "10E2", "-15E2", "-15E2", "-10E2",
"-20E2", "-20E2", null, null])"},
+ {RoundMode::UP,
+ R"(["15E2", "15E2", "10E2", "20E2", "20E2", "-15E2", "-15E2", "0",
"-10E2", "-10E2", null, null])"},
+ {RoundMode::TOWARDS_ZERO,
+ R"(["15E2", "15E2", "0", "10E2", "10E2", "-15E2", "-15E2", "0",
"-10E2", "-10E2", null, null])"},
+ {RoundMode::TOWARDS_INFINITY,
+ R"(["15E2", "15E2", "10E2", "20E2", "20E2", "-15E2", "-15E2",
"-10E2", "-20E2", "-20E2", null, null])"},
+ {RoundMode::HALF_DOWN,
+ R"(["15E2", "15E2", "0", "10E2", "20E2", "-15E2", "-15E2", "-10E2",
"-20E2", "-20E2", null, null])"},
+ {RoundMode::HALF_UP,
+ R"(["15E2", "15E2", "10E2", "20E2", "20E2", "-15E2", "-15E2", "0",
"-10E2", "-20E2", null, null])"},
+ {RoundMode::HALF_TOWARDS_ZERO,
+ R"(["15E2", "15E2", "0", "10E2", "20E2", "-15E2", "-15E2", "0",
"-10E2", "-20E2", null, null])"},
+ {RoundMode::HALF_TOWARDS_INFINITY,
+ R"(["15E2", "15E2", "10E2", "20E2", "20E2", "-15E2", "-15E2",
"-10E2", "-20E2", "-20E2", null, null])"},
+ {RoundMode::HALF_TO_EVEN,
+ R"(["15E2", "15E2", "0", "20E2", "20E2", "-15E2", "-15E2", "0",
"-20E2", "-20E2", null, null])"},
+ {RoundMode::HALF_TO_ODD,
+ R"(["15E2", "15E2", "10E2", "10E2", "20E2", "-15E2", "-15E2",
"-10E2", "-10E2", "-20E2", null, null])"},
+ }};
+
+ for (const auto& pair : round_modes_and_expected) {
+ options.round_mode = pair.first;
+ CheckScalar(func, {values, ndigits_arr}, DecimalArrayFromJSON(ty,
pair.second),
+ &options);
+ }
+ }
+}
+
TYPED_TEST_SUITE(TestUnaryRoundIntegral, IntegralTypes);
TYPED_TEST_SUITE(TestUnaryRoundSigned, SignedIntegerTypes);
TYPED_TEST_SUITE(TestUnaryRoundUnsigned, UnsignedIntegerTypes);
