Support Arbitrary Number of Arrays in downcast_primitive_array #2809
Conversation
| @@ -50,427 +62,183 @@ macro_rules! downcast_primitive_array { | |||
| ($values:ident => $e:expr, $($p:pat => $fallback:expr $(,)*)*) => { | |||
There was a problem hiding this comment.
I eventually hope to split the core logic of downcast_primitive_array into something which maps DataType => ArrowPrimitiveType but this is the first step towards that
tustvold
force-pushed
the
simplify-downcast-primitive
branch
from
cf967b8 to
7979eea
Compare
tustvold
force-pushed
the
simplify-downcast-primitive
branch
from
7979eea to
f100521
Compare
tustvold
force-pushed
the
simplify-downcast-primitive
branch
from
f100521 to
ff98c00
Compare
| /// Repeats the provided pattern based on the number of comma separated identifiers | ||
| #[doc(hidden)] | ||
| #[macro_export] | ||
| macro_rules! repeat_pat { | ||
| ($e:pat, $v_:ident) => { | ||
| $e | ||
| }; | ||
| ($e:pat, $v_:ident $(, $tail:ident)+) => { | ||
| ($e, $crate::repeat_pat!($e $(, $tail)+)) | ||
| } | ||
| } |
| (($($values:ident),+) => $e:block $(($($p:pat),+) => $fallback:expr $(,)*)*) => { | ||
| $crate::downcast_primitive_array!($($values),+ => $e $($($p),+ => $fallback)*) | ||
| }; | ||
| ($($values:ident),+ => $e:block $($($p:pat),+ => $fallback:expr $(,)*)*) => { |
There was a problem hiding this comment.
The number of $p:pat might be different to $values:ident and that will cause error. But the caller should take care of it.
There was a problem hiding this comment.
Yeah, as this is impersonating a match block, I think that is fine
| (($($values:ident),+) => $e:block $($($p:pat),+ => $fallback:expr $(,)*)*) => { | ||
| $crate::downcast_primitive_array!($($values),+ => $e $($($p),+ => $fallback)*) | ||
| }; | ||
| (($($values:ident),+) => $e:block $(($($p:pat),+) => $fallback:expr $(,)*)*) => { | ||
| $crate::downcast_primitive_array!($($values),+ => $e $($($p),+ => $fallback)*) | ||
| }; |
There was a problem hiding this comment.
What's the difference between this and below? With additional ( )?
There was a problem hiding this comment.
Yeah, because typically single arguments are not enclosed in ( ) but multiple are, this allows both patterns
|
Benchmark runs are scheduled for baseline = c8321f4 and contender = 2ae2309. 2ae2309 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #.
Rationale for this change
Uses some macro trickery to allow arbitrary numbers of arrays in downcast_primitive_array. Not only is this potentially useful, it reduces code duplication and yields better consistency. Unfortunately this consistency is back-firing slightly, as support for Float16 SIMD arithmetic is not implemented, and this was just left out of the macro 😅
What changes are included in this PR?
Are there any user-facing changes?
Float16Array will be downcast in the binary invocation, this probably counts as an API change