Uh oh!
There was an error while loading. Please reload this page.
ARROW-11725: [Rust][DataFusion] Make use of the new divide_scalar kernel in arrow - #9543
ARROW-11725: [Rust][DataFusion] Make use of the new divide_scalar kernel in arrow#9543abreis wants to merge 2 commits into
Conversation
| macro_rules! binary_string_array_op_scalar { | ||
| ($LEFT:expr, $RIGHT:expr, $OP:ident) => {{ | ||
| let result = match $LEFT.data_type() { | ||
| let result: Result<Arc<dyn Array>> = match $LEFT.data_type() { |
There was a problem hiding this comment.
I could not find a way to do this type erasure directly in BinaryExpr::evaluate (L419, which only had scalar operations on BooleanArray before), so I'm doing it here and also for binary_array_op_scalar (L244).
alamb
left a comment
There was a problem hiding this comment.
Thanks @abreis -- looks really nice to me. @jorgecarleitao or @Dandandan any thoughts?
Uh oh!
There was an error while loading. Please reload this page.
alamb
commented
Feb 23, 2021
I plan to merge this once the CI goes green |
abreis
commented
Feb 23, 2021
CI failure seems unrelated. Note that the first commit already passed CI, and this second commit only changes a few error strings, so it should be safe to merge. |
alamb
commented
Feb 23, 2021
The integration failure looks like https://issues.apache.org/jira/browse/ARROW-11717 |
alamb
commented
Feb 23, 2021
Merged. 🎉 Thanks @abreis |
This is a small PR to make DataFusion use the just-merged
divide_scalararrow kernel (#9454).Performance-wise:
arrowside, this specialized kernel is ~40-50% faster than the standarddivide, mostly due to not having to check for divide-by-zero on every row;datafusionside, it can now skip thescalar.to_array_of_size(num_rows)allocation, which should be a decent win for operations on large arrays.The eventual goal is to have
op_scalarvariants for every arithmetic operation —dividewill show the biggest performance gains but all variants should save DataFusion a (possibly expensive) allocation.