pitrou commented on code in PR #37536:
URL: https://github.com/apache/arrow/pull/37536#discussion_r1317009896
##########
cpp/src/arrow/compute/kernels/aggregate_test.cc:
##########
@@ -592,25 +652,51 @@ TEST(TestNullSumKernel, Basics) {
Datum null_result = std::make_shared<Int64Scalar>();
Datum zero_result = std::make_shared<Int64Scalar>(0);
- EXPECT_THAT(Sum(ScalarFromJSON(ty, "null")), ResultWith(null_result));
- EXPECT_THAT(Sum(ArrayFromJSON(ty, "[]")), ResultWith(null_result));
- EXPECT_THAT(Sum(ArrayFromJSON(ty, "[null]")), ResultWith(null_result));
- EXPECT_THAT(Sum(ChunkedArrayFromJSON(ty, {"[null]", "[]", "[null, null]"})),
- ResultWith(null_result));
+ for (auto func : {Sum, SumChecked}) {
+ SCOPED_TRACE(func);
+ auto default_options = ScalarAggregateOptions::Defaults();
+ EXPECT_THAT(func(ScalarFromJSON(ty, "null"), default_options, nullptr),
+ ResultWith(null_result));
+ EXPECT_THAT(func(ArrayFromJSON(ty, "[]"), default_options, nullptr),
+ ResultWith(null_result));
+ EXPECT_THAT(func(ArrayFromJSON(ty, "[null]"), default_options, nullptr),
+ ResultWith(null_result));
+ EXPECT_THAT(func(ChunkedArrayFromJSON(ty, {"[null]", "[]", "[null,
null]"}),
+ default_options, nullptr),
+ ResultWith(null_result));
- ScalarAggregateOptions options(/*skip_nulls=*/true, /*min_count=*/0);
- EXPECT_THAT(Sum(ScalarFromJSON(ty, "null"), options),
ResultWith(zero_result));
- EXPECT_THAT(Sum(ArrayFromJSON(ty, "[]"), options), ResultWith(zero_result));
- EXPECT_THAT(Sum(ArrayFromJSON(ty, "[null]"), options),
ResultWith(zero_result));
- EXPECT_THAT(Sum(ChunkedArrayFromJSON(ty, {"[null]", "[]", "[null, null]"}),
options),
- ResultWith(zero_result));
+ ScalarAggregateOptions options(/*skip_nulls=*/true, /*min_count=*/0);
+ EXPECT_THAT(func(ScalarFromJSON(ty, "null"), options, nullptr),
+ ResultWith(zero_result));
+ EXPECT_THAT(func(ArrayFromJSON(ty, "[]"), options, nullptr),
ResultWith(zero_result));
+ EXPECT_THAT(func(ArrayFromJSON(ty, "[null]"), options, nullptr),
+ ResultWith(zero_result));
+ EXPECT_THAT(func(ChunkedArrayFromJSON(ty, {"[null]", "[]", "[null,
null]"}), options,
+ nullptr),
+ ResultWith(zero_result));
+
+ options = ScalarAggregateOptions(/*skip_nulls=*/false, /*min_count=*/0);
+ EXPECT_THAT(func(ScalarFromJSON(ty, "null"), options, nullptr),
+ ResultWith(null_result));
+ EXPECT_THAT(func(ArrayFromJSON(ty, "[]"), options, nullptr),
ResultWith(zero_result));
+ EXPECT_THAT(func(ArrayFromJSON(ty, "[null]"), options, nullptr),
+ ResultWith(null_result));
+ EXPECT_THAT(func(ChunkedArrayFromJSON(ty, {"[null]", "[]", "[null,
null]"}), options,
+ nullptr),
+ ResultWith(null_result));
+ }
+}
- options = ScalarAggregateOptions(/*skip_nulls=*/false, /*min_count=*/0);
- EXPECT_THAT(Sum(ScalarFromJSON(ty, "null"), options),
ResultWith(null_result));
- EXPECT_THAT(Sum(ArrayFromJSON(ty, "[]"), options), ResultWith(zero_result));
- EXPECT_THAT(Sum(ArrayFromJSON(ty, "[null]"), options),
ResultWith(null_result));
- EXPECT_THAT(Sum(ChunkedArrayFromJSON(ty, {"[null]", "[]", "[null, null]"}),
options),
- ResultWith(null_result));
+TEST(TestSumKernel, Overflow) {
+ int64_t large_scalar = 1000000000000000;
+ int64_t length = 10000;
+ auto scalar = std::make_shared<Int64Scalar>(large_scalar);
Review Comment:
> I added uint64, but I don't think there are checked arithmetic ops for
decimals?
[AddChecked](https://github.com/apache/arrow/blob/main/cpp/src/arrow/compute/kernels/base_arithmetic_internal.h#L85)
and
[MultiplyChecked](https://github.com/apache/arrow/blob/main/cpp/src/arrow/compute/kernels/base_arithmetic_internal.h#L356)
do not seem to check for overflows.
That seems to be because Add and Multiply will adjust the result precision
as required:
```python
>>> a = pa.array([Decimal(9000)])
>>> a.type
Decimal128Type(decimal128(4, 0))
>>> pc.add_checked(a, a)
<pyarrow.lib.Decimal128Array object at 0x7f399c4c78e0>
[
18000
]
>>> pc.add_checked(a, a).type
Decimal128Type(decimal128(5, 0))
```
However, Sum does not:
```python
>>> a = pa.array([Decimal(1000)]*20)
>>> a.type
Decimal128Type(decimal128(4, 0))
>>> pc.sum(a)
<pyarrow.Decimal128Scalar: Decimal('20000')>
>>> pc.sum(a).validate(full=True)
Traceback (most recent call last):
...
ArrowInvalid: Decimal value 20000 does not fit in precision of decimal128(4,
0)
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
