andygrove opened a new issue, #5092:
URL: https://github.com/apache/datafusion-comet/issues/5092
### What is the problem the feature request solves?
In `native/spark-expr/src/math_funcs/checked_arithmetic.rs`, the ANSI-mode
integer branches of `checked_binary` re-implement arrow's checked numeric
kernels: they call `arity::try_binary` with
`add_checked`/`sub_checked`/`mul_checked`/`div_checked`, which is exactly what
`arrow::compute::kernels::numeric::{add, sub, mul, div}` do for primitive
integers. In addition, `checked_arithmetic_internal` materializes scalar
operands with `to_array_of_size`, which the arrow kernels' `Datum` path makes
unnecessary.
### Describe the potential solution
For integer types in ANSI mode, call the arrow kernels directly with `Datum`
operands and keep the existing error remap (`ArrowError::DivideByZero` to Spark
DIVIDE_BY_ZERO, other errors to Spark ARITHMETIC_OVERFLOW).
Not replaceable, must stay as is:
- Float `checked_div`: `div_checked` on floats errors on a zero divisor
(pinned by `test_checked_div_by_zero`), while arrow's `div` kernel on floats
wraps to inf/NaN.
- The Try-mode branch (null on overflow with a manual
`BooleanBufferBuilder`): arrow has no null-on-overflow kernel.
### Additional context
Cleanup opportunity spotted in the same file: inside the Try-mode loop, the
`else if` branch guarded by `is_ansi_mode` is unreachable because ANSI mode
returns earlier in the function.
Found during an audit of native code that replicates existing arrow-rs
kernels.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
