-
Notifications
You must be signed in to change notification settings - Fork 784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split out arrow-cast (#2594) #2998
Conversation
b06de40
to
dcd5603
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM -- this crate is going to be compiling full on parallelized before we know it
arrow-cast/src/cast.rs
Outdated
cast_with_options(array, to_type, &DEFAULT_CAST_OPTIONS) | ||
} | ||
|
||
fn cast_integer_to_decimal128<T: ArrowNumericType>( | ||
fn cast_integer_to_decimal128<T: ArrowPrimitiveType>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason to also change this from ArrowNumericType
to ArrowPrimitiveType
?
Seems fine as all ArrowNumericTypesare also
ArrowPrimtiveTypes` https://docs.rs/arrow/latest/arrow/datatypes/trait.ArrowNumericType.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ArrowNumericType is part of the arithmetic kernels which haven't been split out
array_value_to_string(dict_array.values(), dict_index) | ||
} | ||
|
||
/// Converts numeric type to a `String` | ||
pub fn lexical_to_string<N: lexical_core::ToLexical>(n: N) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was just moved
Conflicts require #3018 |
arrow-flight failure related to #3019 |
Benchmark runs are scheduled for baseline = 29b3fef and contender = cdc8d0e. cdc8d0e 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?
Part of #2594
Rationale for this change
The cast kernel is a fairly substantial piece of code, that takes a non-trivial time to compile.
This is the last remaining kernel that the IO functionality, i.e. IPC, parquet, etc... depend on, and so this is a precursor to splitting them out.
What changes are included in this PR?
Splits out the casting logic into a separate crate
Are there any user-facing changes?
No