martin-g commented on code in PR #19395:
URL: https://github.com/apache/datafusion/pull/19395#discussion_r2635035277
##########
datafusion/spark/src/function/math/abs.rs:
##########
@@ -69,8 +70,32 @@ impl ScalarUDFImpl for SparkAbs {
&self.signature
}
- fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
- Ok(arg_types[0].clone())
+ fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
+ internal_err!(
+ "SparkAbs: return_type() is not used; return_field_from_args() is
implemented"
+ )
+ }
+
+ fn return_field_from_args(&self, args: ReturnFieldArgs) ->
Result<FieldRef> {
+ if args.arg_fields.is_empty() {
+ return internal_err!("abs expects at least 1 argument");
Review Comment:
There is no really need for this check. The method won't be called at all if
the signature does not match.
But if you prefer to have it then let's synchronise it with the
`spark_abs()` error message:
```suggestion
return internal_err!("abs takes exactly 1 argument, but got:
{}", args.arg_fields.len());
```
##########
datafusion/spark/src/function/math/abs.rs:
##########
@@ -375,4 +400,92 @@ mod tests {
as_decimal256_array
);
}
+
+ #[test]
+ fn test_abs_nullability() {
+ use arrow::datatypes::{DataType, Field};
+ use datafusion_expr::ReturnFieldArgs;
+ use std::sync::Arc;
+
+ let abs = SparkAbs::new();
+
+ // --- non-nullable Int32 input ---
+ let non_nullable_i32 = Arc::new(Field::new("c", DataType::Int32,
false));
+ let out_non_null = abs
+ .return_field_from_args(ReturnFieldArgs {
+ arg_fields: &[Arc::clone(&non_nullable_i32)],
+ scalar_arguments: &[None],
+ })
+ .unwrap();
+
+ // result should be non-nullable and the same DataType as input
+ assert!(!out_non_null.is_nullable());
+ assert_eq!(out_non_null.data_type(), &DataType::Int32);
+
+ // --- nullable Int32 input ---
+ let nullable_i32 = Arc::new(Field::new("c", DataType::Int32, true));
+ let out_nullable = abs
+ .return_field_from_args(ReturnFieldArgs {
+ arg_fields: &[Arc::clone(&nullable_i32)],
+ scalar_arguments: &[None],
+ })
+ .unwrap();
+
+ // result should be nullable and the same DataType as input
+ assert!(out_nullable.is_nullable());
+ assert_eq!(out_nullable.data_type(), &DataType::Int32);
+
+ // --- non-nullable Float64 input ---
+ let non_nullable_f64 = Arc::new(Field::new("c", DataType::Float64,
false));
+ let out_f64 = abs
+ .return_field_from_args(ReturnFieldArgs {
+ arg_fields: &[Arc::clone(&non_nullable_f64)],
+ scalar_arguments: &[None],
+ })
+ .unwrap();
+
+ assert!(!out_f64.is_nullable());
+ assert_eq!(out_f64.data_type(), &DataType::Float64);
+
+ // --- nullable Float64 input ---
+ let nullable_f64 = Arc::new(Field::new("c", DataType::Float64, true));
+ let out_f64_null = abs
+ .return_field_from_args(ReturnFieldArgs {
+ arg_fields: &[Arc::clone(&nullable_f64)],
+ scalar_arguments: &[None],
+ })
+ .unwrap();
+
+ assert!(out_f64_null.is_nullable());
+ assert_eq!(out_f64_null.data_type(), &DataType::Float64);
+ }
+
+ #[test]
+ fn test_abs_nullability_with_null_scalar() -> Result<()> {
+ use arrow::datatypes::{DataType, Field};
+ use datafusion_expr::ReturnFieldArgs;
+ use std::sync::Arc;
+
+ let func = SparkAbs::new();
+
+ // Non-nullable field with non-null scalar argument -> non-nullable
result
+ let non_nullable: FieldRef = Arc::new(Field::new("col",
DataType::Int32, false));
+ let out = func.return_field_from_args(ReturnFieldArgs {
+ arg_fields: &[Arc::clone(&non_nullable)],
+ scalar_arguments: &[None],
Review Comment:
```suggestion
scalar_arguments: &[Some(ScalarValue::Int32(Some(1)))],
```
--
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]
