Skip to content

Commit d2c3045

Browse files
committed
Decimal sum uses smallest physical type that supports output dtype
Instead of always casting values to i256 we take the primitive value of the return dtype and perform operations in that space Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent 704f560 commit d2c3045

1 file changed

Lines changed: 82 additions & 32 deletions

File tree

vortex-array/src/aggregate_fn/fns/sum/decimal.rs

Lines changed: 82 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4+
use crate::dtype::DecimalDType;
5+
use crate::dtype::DecimalType;
46
use itertools::Itertools;
7+
use num_traits::AsPrimitive;
8+
use num_traits::CheckedAdd;
9+
use num_traits::NumOps;
10+
use vortex_buffer::BitBuffer;
11+
use vortex_buffer::Buffer;
12+
use vortex_error::VortexExpect;
513
use vortex_error::VortexResult;
614
use vortex_error::vortex_panic;
7-
use vortex_mask::AllOr;
15+
use vortex_mask::Mask;
816

917
use super::SumState;
1018
use crate::arrays::DecimalArray;
@@ -19,40 +27,82 @@ pub(super) fn accumulate_decimal(inner: &mut SumState, d: &DecimalArray) -> Vort
1927
};
2028

2129
let mask = d.validity_mask()?;
22-
match mask.bit_buffer() {
23-
AllOr::None => Ok(false),
24-
AllOr::All => match_each_decimal_value_type!(d.values_type(), |T| {
25-
for &v in d.buffer::<T>().iter() {
26-
match value.checked_add(&DecimalValue::from(v)) {
27-
Some(r) => {
28-
*value = r;
29-
// Check for overflow
30-
if !value.fits_in_precision(*dtype) {
31-
return Ok(true);
32-
}
33-
}
34-
None => return Ok(true),
35-
}
36-
}
37-
Ok(false)
38-
}),
39-
AllOr::Some(validity) => match_each_decimal_value_type!(d.values_type(), |T| {
40-
for (&v, valid) in d.buffer::<T>().iter().zip_eq(validity.iter()) {
41-
if valid {
42-
match value.checked_add(&DecimalValue::from(v)) {
43-
Some(r) => {
44-
*value = r;
45-
if !value.fits_in_precision(*dtype) {
46-
return Ok(true);
47-
}
48-
}
49-
None => return Ok(true),
50-
}
51-
}
30+
let validity = match &mask {
31+
Mask::AllTrue(_) => None,
32+
Mask::Values(mask_values) => Some(mask_values.bit_buffer()),
33+
Mask::AllFalse(_) => {
34+
return Ok(false);
35+
}
36+
};
37+
38+
let values_type = DecimalType::smallest_decimal_value_type(&dtype);
39+
match_each_decimal_value_type!(d.values_type(), |I| {
40+
match_each_decimal_value_type!(values_type, |O| {
41+
let initial_val: O = value
42+
.cast()
43+
.vortex_expect("cannot fail to cast initial value");
44+
45+
let res = sum_to_scalar(d.buffer::<I>(), validity, initial_val, *dtype);
46+
match res {
47+
Some(v) => *value = v,
48+
None => return Ok(true),
5249
}
5350
Ok(false)
54-
}),
51+
})
52+
})
53+
}
54+
55+
/// Compute the checked sum and convert the result to a [`Scalar`].
56+
///
57+
/// Returns a null scalar if the sum overflows the underlying integer type or if the result
58+
/// exceeds the declared decimal precision.
59+
fn sum_to_scalar<T, O>(
60+
values: Buffer<T>,
61+
validity: Option<&BitBuffer>,
62+
initial: O,
63+
return_decimal_dtype: DecimalDType,
64+
) -> Option<DecimalValue>
65+
where
66+
T: AsPrimitive<O>,
67+
O: CheckedAdd + NumOps + Into<DecimalValue> + Copy + 'static,
68+
bool: AsPrimitive<O>,
69+
{
70+
let raw_sum = match validity {
71+
Some(v) => sum_decimal_with_validity(values, v, initial),
72+
None => sum_decimal(values, initial),
73+
};
74+
75+
raw_sum
76+
.map(Into::<DecimalValue>::into)
77+
// We have to make sure that the decimal value fits the precision of the decimal dtype.
78+
.filter(|v| v.fits_in_precision(return_decimal_dtype))
79+
}
80+
81+
fn sum_decimal<T: AsPrimitive<I>, I: Copy + CheckedAdd + 'static>(
82+
values: Buffer<T>,
83+
initial: I,
84+
) -> Option<I> {
85+
let mut sum = initial;
86+
for v in values.iter() {
87+
let v: I = v.as_();
88+
sum = CheckedAdd::checked_add(&sum, &v)?;
89+
}
90+
Some(sum)
91+
}
92+
93+
fn sum_decimal_with_validity<T, I>(values: Buffer<T>, validity: &BitBuffer, initial: I) -> Option<I>
94+
where
95+
T: AsPrimitive<I>,
96+
I: NumOps + CheckedAdd + Copy + 'static,
97+
bool: AsPrimitive<I>,
98+
{
99+
let mut sum = initial;
100+
for (v, valid) in values.iter().zip_eq(validity) {
101+
let v: I = v.as_() * valid.as_();
102+
103+
sum = CheckedAdd::checked_add(&sum, &v)?;
55104
}
105+
Some(sum)
56106
}
57107

58108
#[cfg(test)]

0 commit comments

Comments
 (0)