Jefffrey commented on code in PR #19871:
URL: https://github.com/apache/datafusion/pull/19871#discussion_r2702542865
##########
datafusion/functions/src/math/signum.rs:
##########
@@ -98,6 +98,34 @@ impl ScalarUDFImpl for SignumFunc {
}
fn invoke_with_args(&self, args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
+ let arg = &args.args[0];
+
+ // Scalar fast path for float types - avoid array conversion overhead
+ if let ColumnarValue::Scalar(scalar) = arg {
Review Comment:
We don't need to repeat this comment about fast paths each time (not to
mention specifying it for "float types" is confusing considering the function
signature already limits the inputs to float types). So it can actually be a
bit misleading as it might imply we omit fast path for non-float types. We're
better off removing the comment.
##########
datafusion/functions/src/math/signum.rs:
##########
@@ -98,6 +98,34 @@ impl ScalarUDFImpl for SignumFunc {
}
fn invoke_with_args(&self, args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
+ let arg = &args.args[0];
+
+ // Scalar fast path for float types - avoid array conversion overhead
+ if let ColumnarValue::Scalar(scalar) = arg {
+ if scalar.is_null() {
+ return ColumnarValue::Scalar(ScalarValue::Null)
+ .cast_to(args.return_type(), None);
+ }
+
+ match scalar {
+ ScalarValue::Float64(Some(v)) => {
+ let result = if *v == 0.0 { 0.0 } else { v.signum() };
+ return
Ok(ColumnarValue::Scalar(ScalarValue::Float64(Some(result))));
+ }
+ ScalarValue::Float32(Some(v)) => {
+ let result = if *v == 0.0 { 0.0 } else { v.signum() };
+ return
Ok(ColumnarValue::Scalar(ScalarValue::Float32(Some(result))));
+ }
+ _ => {
+ return internal_err!(
+ "Unexpected scalar type for signum: {:?}",
+ scalar.data_type()
+ );
+ }
+ }
+ }
+
+ // Array path
Review Comment:
We might as well change the `if let` to a match statement, and inline the
contents of `signum` here to avoid use of `make_scalar_function` to simplify
the code
##########
datafusion/functions/src/math/signum.rs:
##########
@@ -98,6 +98,34 @@ impl ScalarUDFImpl for SignumFunc {
}
fn invoke_with_args(&self, args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
+ let arg = &args.args[0];
Review Comment:
```suggestion
let arg = take_function_args(self.name(), args.args)?;
```
--
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]
