neilconway commented on code in PR #26062:
URL: https://github.com/apache/datafusion/pull/26062#discussion_r4208426839
##########
datafusion/functions/src/math/common.rs:
##########
@@ -150,10 +151,48 @@ pub(crate) fn lcm_signed_int(x: i64, y: i64) ->
Result<i64, ArrowError> {
})
}
+/// Applies `op` to every value in `array`, like `unary`, but returns an error
+/// if `input_error` returns a message for any non-null value.
+///
+/// Use this for functions that return an error for some argument values, such
+/// as `sqrt`, which returns an error for negative numbers. `try_unary` can
also
+/// return errors, but it can return early on any value, which keeps the
+/// compiler from vectorizing its loop. That makes cheap functions like `sqrt`
+/// several times slower.
+///
+/// Instead, `input_error` is called on every value, including those in null
+/// slots, in the same loop as `op`; only if some value fails is the array
+/// searched again for an error to report. `input_error` should therefore be a
+/// cheap check, such as a comparison.
+pub(crate) fn unary_with_input_check<T: ArrowPrimitiveType>(
+ array: &PrimitiveArray<T>,
+ op: impl Fn(T::Native) -> T::Native,
+ input_error: impl Fn(T::Native) -> Option<&'static str>,
+) -> Result<PrimitiveArray<T>> {
+ let mut any_invalid = false;
+ let values: Vec<T::Native> = array
+ .values()
+ .iter()
+ .map(|&x| {
+ any_invalid |= input_error(x).is_some();
+ op(x)
+ })
+ .collect();
+
+ // The check above also ran on null slots, which can hold any value, so the
+ // failure may be spurious. Re-check just the non-null values.
Review Comment:
A related behavior is that null slots can hold arbitrary values; if those
values happen to fail the error check, we'll need to take the slow path, even
if all the valid slots are not erroneous. But we can at least do better here
than the initial version of the PR -- in the slow path, we were skipping NULLs
on the recheck with a `filter()`, but it's faster to use a bitmap to mask out
null slots. I added that optimization to the PR.
--
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]
