Skip to content

Commit e4cea19

Browse files
committed
Add bounded decimal dispatch macros and migrate kernels
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent ec2b1b5 commit e4cea19

5 files changed

Lines changed: 273 additions & 37 deletions

File tree

‎vortex-array/src/aggregate_fn/fns/all_non_distinct/decimal.rs‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,9 @@ use vortex_error::VortexResult;
77
use crate::arrays::DecimalArray;
88
use crate::arrays::decimal::DecimalArrayExt;
99
use crate::dtype::BigCast;
10+
use crate::match_decimal_binary;
1011
use crate::match_each_decimal_value_type;
1112

12-
#[expect(
13-
clippy::cognitive_complexity,
14-
reason = "decimal widening depends on both source value types and the chosen widest type"
15-
)]
1613
pub(super) fn check_decimal_identical(
1714
lhs: &DecimalArray,
1815
rhs: &DecimalArray,
@@ -24,19 +21,15 @@ pub(super) fn check_decimal_identical(
2421
}
2522

2623
let widest = lhs.values_type().max(rhs.values_type());
27-
match_each_decimal_value_type!(lhs.values_type(), |L| {
28-
match_each_decimal_value_type!(rhs.values_type(), |R| {
29-
match_each_decimal_value_type!(widest, |W| {
30-
Ok(lhs
31-
.buffer::<L>()
32-
.iter()
33-
.zip(rhs.buffer::<R>().iter())
34-
.all(|(lhs, rhs)| {
35-
<W as BigCast>::from(*lhs).vortex_expect("decimal widening should succeed")
36-
== <W as BigCast>::from(*rhs)
37-
.vortex_expect("decimal widening should succeed")
38-
}))
39-
})
40-
})
24+
match_decimal_binary!(lhs.values_type(), rhs.values_type(), widest, |L, R, W| {
25+
Ok(lhs
26+
.buffer::<L>()
27+
.iter()
28+
.zip(rhs.buffer::<R>().iter())
29+
.all(|(lhs, rhs)| {
30+
<W as BigCast>::from(*lhs).vortex_expect("decimal widening should succeed")
31+
== <W as BigCast>::from(*rhs)
32+
.vortex_expect("decimal widening should succeed")
33+
}))
4134
})
4235
}

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::dtype::DType;
1919
use crate::dtype::DecimalDType;
2020
use crate::dtype::DecimalType;
2121
use crate::dtype::NativeDecimalType;
22-
use crate::match_each_decimal_value_type;
22+
use crate::match_decimal_unary;
2323
use crate::scalar::DecimalValue;
2424

2525
/// Accumulate a decimal array into the sum state.
@@ -47,17 +47,15 @@ pub(crate) fn accumulate_decimal(
4747
.vortex_expect("decimal sum result dtype");
4848

4949
let values_type = DecimalType::smallest_decimal_value_type(dtype);
50-
match_each_decimal_value_type!(d.values_type(), |T| {
51-
match_each_decimal_value_type!(values_type, |I| {
52-
let initial: I = value
53-
.cast()
54-
.vortex_expect("cannot fail to cast initial value");
55-
match sum_decimal_value(initial, d.buffer::<T>(), validity, *dtype) {
56-
Some(v) => *value = v,
57-
None => return Ok(true),
58-
}
59-
Ok(false)
60-
})
50+
match_decimal_unary!(d.values_type(), values_type, |T, I| {
51+
let initial: I = value
52+
.cast()
53+
.vortex_expect("cannot fail to cast initial value");
54+
match sum_decimal_value(initial, d.buffer::<T>(), validity, *dtype) {
55+
Some(v) => *value = v,
56+
None => return Ok(true),
57+
}
58+
Ok(false)
6159
})
6260
}
6361

‎vortex-array/src/arrays/decimal/compute/cast.rs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use crate::dtype::Nullability;
3333
use crate::dtype::PType;
3434
use crate::dtype::ToI256;
3535
use crate::dtype::i256;
36+
use crate::match_decimal_unary;
3637
use crate::match_each_decimal_value_type;
3738
use crate::match_each_integer_ptype;
3839
use crate::scalar::DecimalToIntegerCast;
@@ -461,13 +462,9 @@ pub fn upcast_decimal_values(
461462
}
462463
let validity = array.validity()?;
463464

464-
// Use match_each_decimal_value_type to dispatch based on source and target types
465-
match_each_decimal_value_type!(from_values_type, |F| {
466-
let from_buffer = array.buffer::<F>();
467-
match_each_decimal_value_type!(to_values_type, |T| {
468-
let to_buffer = upcast_decimal_buffer::<F, T>(from_buffer);
469-
Ok(DecimalArray::new(to_buffer, decimal_dtype, validity))
470-
})
465+
match_decimal_unary!(from_values_type, to_values_type, |F, T| {
466+
let to_buffer = upcast_decimal_buffer::<F, T>(array.buffer::<F>());
467+
Ok(DecimalArray::new(to_buffer, decimal_dtype, validity))
471468
})
472469
}
473470

‎vortex-array/src/dtype/decimal/macros.rs‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,139 @@ macro_rules! match_each_decimal_value_type {
5858
}
5959
}};
6060
}
61+
62+
/// Dispatches a decimal input and output type, instantiating only `In <= Out`.
63+
/// This produces 21 type combinations instead of 36.
64+
///
65+
/// The caller must choose an output type at least as wide as the input storage type.
66+
/// A decimal array's storage is no wider than
67+
/// [`DecimalType::smallest_decimal_value_type`](crate::dtype::DecimalType::smallest_decimal_value_type)
68+
/// for its precision, so using that type (or a wider working type) satisfies this contract.
69+
/// Narrowing casts must use unrestricted dispatch instead. Invalid combinations panic.
70+
/// Each type expression is evaluated once.
71+
///
72+
/// ```
73+
/// use num_traits::AsPrimitive;
74+
/// use vortex_array::dtype::DecimalType;
75+
/// use vortex_array::match_decimal_unary;
76+
///
77+
/// let value = match_decimal_unary!(DecimalType::I8, DecimalType::I64, |In, Out| {
78+
/// let input: In = 42i8.as_();
79+
/// <In as AsPrimitive<Out>>::as_(input).to_string()
80+
/// });
81+
/// assert_eq!(value, "42");
82+
/// ```
83+
#[macro_export]
84+
macro_rules! match_decimal_unary {
85+
($input:expr, $output:expr, | $in:ident, $out:ident | $body:block) => {{
86+
let (input, output) = ($input, $output);
87+
match output {
88+
$crate::dtype::DecimalType::I8 => {
89+
type $out = i8;
90+
$crate::match_decimal_unary!(@input input, |$in| $body; I8 => i8)
91+
}
92+
$crate::dtype::DecimalType::I16 => {
93+
type $out = i16;
94+
$crate::match_decimal_unary!(@input input, |$in| $body; I8 => i8, I16 => i16)
95+
}
96+
$crate::dtype::DecimalType::I32 => {
97+
type $out = i32;
98+
$crate::match_decimal_unary!(@input input, |$in| $body; I8 => i8, I16 => i16, I32 => i32)
99+
}
100+
$crate::dtype::DecimalType::I64 => {
101+
type $out = i64;
102+
$crate::match_decimal_unary!(@input input, |$in| $body; I8 => i8, I16 => i16, I32 => i32, I64 => i64)
103+
}
104+
$crate::dtype::DecimalType::I128 => {
105+
type $out = i128;
106+
$crate::match_decimal_unary!(@input input, |$in| $body; I8 => i8, I16 => i16, I32 => i32, I64 => i64, I128 => i128)
107+
}
108+
$crate::dtype::DecimalType::I256 => {
109+
type $out = $crate::dtype::i256;
110+
$crate::match_decimal_unary!(@input input, |$in| $body; I8 => i8, I16 => i16, I32 => i32, I64 => i64, I128 => i128, I256 => $crate::dtype::i256)
111+
}
112+
}
113+
}};
114+
(@input $input:expr, | $in:ident | $body:block; $( $variant:ident => $native:ty ),+) => {{
115+
#[allow(unreachable_patterns)]
116+
match $input {
117+
$(
118+
$crate::dtype::DecimalType::$variant => {
119+
type $in = $native;
120+
$body
121+
}
122+
)+
123+
_ => panic!("decimal input storage type must be no wider than the output type"),
124+
}
125+
}};
126+
}
127+
128+
/// Dispatches two decimal input types and an output type, instantiating only combinations
129+
/// where both inputs are no wider than the output (`Left <= Out` and `Right <= Out`).
130+
///
131+
/// This produces 91 type combinations instead of the 216 from unrestricted nested dispatch.
132+
/// See [`match_decimal_unary!`] for the storage contract. Invalid combinations panic.
133+
/// Each type expression is evaluated once, from left to right.
134+
///
135+
/// ```
136+
/// use num_traits::AsPrimitive;
137+
/// use vortex_array::dtype::DecimalType;
138+
/// use vortex_array::match_decimal_binary;
139+
///
140+
/// let value = match_decimal_binary!(
141+
/// DecimalType::I8, DecimalType::I16, DecimalType::I64, |Left, Right, Out| {
142+
/// let lhs: Left = 1i8.as_();
143+
/// let rhs: Right = 2i8.as_();
144+
/// (<Left as AsPrimitive<Out>>::as_(lhs) + <Right as AsPrimitive<Out>>::as_(rhs)).to_string()
145+
/// }
146+
/// );
147+
/// assert_eq!(value, "3");
148+
/// ```
149+
#[macro_export]
150+
macro_rules! match_decimal_binary {
151+
($left:expr, $right:expr, $output:expr, | $lhs:ident, $rhs:ident, $out:ident | $body:block) => {{
152+
let (left, right, output) = ($left, $right, $output);
153+
match output {
154+
$crate::dtype::DecimalType::I8 => {
155+
type $out = i8;
156+
$crate::match_decimal_unary!(@input left, |$lhs| {
157+
$crate::match_decimal_unary!(@input right, |$rhs| $body; I8 => i8)
158+
}; I8 => i8)
159+
}
160+
$crate::dtype::DecimalType::I16 => {
161+
type $out = i16;
162+
$crate::match_decimal_unary!(@input left, |$lhs| {
163+
$crate::match_decimal_unary!(@input right, |$rhs| $body; I8 => i8, I16 => i16)
164+
}; I8 => i8, I16 => i16)
165+
}
166+
$crate::dtype::DecimalType::I32 => {
167+
type $out = i32;
168+
$crate::match_decimal_unary!(@input left, |$lhs| {
169+
$crate::match_decimal_unary!(@input right, |$rhs| $body; I8 => i8, I16 => i16, I32 => i32)
170+
}; I8 => i8, I16 => i16, I32 => i32)
171+
}
172+
$crate::dtype::DecimalType::I64 => {
173+
type $out = i64;
174+
$crate::match_decimal_unary!(@input left, |$lhs| {
175+
$crate::match_decimal_unary!(@input right, |$rhs| $body; I8 => i8, I16 => i16, I32 => i32, I64 => i64)
176+
}; I8 => i8, I16 => i16, I32 => i32, I64 => i64)
177+
}
178+
$crate::dtype::DecimalType::I128 => {
179+
type $out = i128;
180+
$crate::match_decimal_unary!(@input left, |$lhs| {
181+
$crate::match_decimal_unary!(@input right, |$rhs| $body; I8 => i8, I16 => i16, I32 => i32, I64 => i64, I128 => i128)
182+
}; I8 => i8, I16 => i16, I32 => i32, I64 => i64, I128 => i128)
183+
}
184+
$crate::dtype::DecimalType::I256 => {
185+
type $out = $crate::dtype::i256;
186+
$crate::match_decimal_unary!(@input left, |$lhs| {
187+
$crate::match_decimal_unary!(@input right, |$rhs| $body; I8 => i8, I16 => i16, I32 => i32, I64 => i64, I128 => i128, I256 => $crate::dtype::i256)
188+
}; I8 => i8, I16 => i16, I32 => i32, I64 => i64, I128 => i128, I256 => $crate::dtype::i256)
189+
}
190+
}
191+
}};
192+
}
193+
194+
#[cfg(test)]
195+
#[path = "macros_tests.rs"]
196+
mod tests;
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// SPDX-FileCopyrightText: Copyright the Vortex contributors
3+
4+
use std::cell::Cell;
5+
use std::panic::catch_unwind;
6+
7+
use num_traits::AsPrimitive;
8+
9+
use crate::dtype::DecimalType;
10+
use crate::dtype::NativeDecimalType;
11+
use crate::dtype::i256;
12+
13+
const TYPES: [DecimalType; 6] = [
14+
DecimalType::I8,
15+
DecimalType::I16,
16+
DecimalType::I32,
17+
DecimalType::I64,
18+
DecimalType::I128,
19+
DecimalType::I256,
20+
];
21+
22+
// Only widening pairs implement this trait. Using it in the macro bodies makes instantiating
23+
// even one narrowing combination a compile error, independently of the runtime test inputs.
24+
trait Widen<Out> {
25+
fn widen(self) -> Out;
26+
}
27+
28+
macro_rules! impl_widen {
29+
($out:ty; $( $input:ty ),+) => {
30+
$(
31+
impl Widen<$out> for $input {
32+
fn widen(self) -> $out {
33+
self.as_()
34+
}
35+
}
36+
)+
37+
};
38+
}
39+
40+
impl_widen!(i8; i8);
41+
impl_widen!(i16; i8, i16);
42+
impl_widen!(i32; i8, i16, i32);
43+
impl_widen!(i64; i8, i16, i32, i64);
44+
impl_widen!(i128; i8, i16, i32, i64, i128);
45+
impl_widen!(i256; i8, i16, i32, i64, i128, i256);
46+
47+
#[test]
48+
fn unary_dispatches_only_widening_pairs() {
49+
for input in TYPES {
50+
for output in TYPES {
51+
let result = catch_unwind(|| {
52+
match_decimal_unary!(input, output, |In, Out| {
53+
let value: In = (-42i8).as_();
54+
let widened: Out = value.widen();
55+
assert_eq!(In::DECIMAL_TYPE, input);
56+
assert_eq!(Out::DECIMAL_TYPE, output);
57+
widened.to_string()
58+
})
59+
});
60+
if input <= output {
61+
assert_eq!(result.unwrap(), "-42");
62+
} else {
63+
assert!(result.is_err());
64+
}
65+
}
66+
}
67+
}
68+
69+
#[test]
70+
fn binary_dispatches_only_widening_triples() {
71+
for left in TYPES {
72+
for right in TYPES {
73+
for output in TYPES {
74+
let result = catch_unwind(|| {
75+
match_decimal_binary!(left, right, output, |Left, Right, Out| {
76+
let lhs: Left = (-42i8).as_();
77+
let rhs: Right = 43i8.as_();
78+
let lhs: Out = lhs.widen();
79+
let rhs: Out = rhs.widen();
80+
assert_eq!(Left::DECIMAL_TYPE, left);
81+
assert_eq!(Right::DECIMAL_TYPE, right);
82+
assert_eq!(Out::DECIMAL_TYPE, output);
83+
(lhs + rhs).to_string()
84+
})
85+
});
86+
if left <= output && right <= output {
87+
assert_eq!(result.unwrap(), "1");
88+
} else {
89+
assert!(result.is_err());
90+
}
91+
}
92+
}
93+
}
94+
}
95+
96+
#[test]
97+
fn dispatch_evaluates_type_expressions_once_in_order() {
98+
let calls = Cell::new(0);
99+
let next = |expected| {
100+
assert_eq!(calls.get(), expected);
101+
calls.set(expected + 1);
102+
DecimalType::I8
103+
};
104+
match_decimal_unary!(next(0), next(1), |In, Out| {
105+
assert_eq!(In::DECIMAL_TYPE, Out::DECIMAL_TYPE);
106+
});
107+
match_decimal_binary!(next(2), next(3), next(4), |Left, Right, Out| {
108+
assert_eq!(Left::DECIMAL_TYPE, Out::DECIMAL_TYPE);
109+
assert_eq!(Right::DECIMAL_TYPE, Out::DECIMAL_TYPE);
110+
});
111+
assert_eq!(calls.get(), 5);
112+
}

0 commit comments

Comments
 (0)