martin-g commented on code in PR #19323:
URL: https://github.com/apache/datafusion/pull/19323#discussion_r2622132402


##########
datafusion/spark/src/function/hash/sha2.rs:
##########
@@ -225,3 +264,94 @@ fn compute_sha2(
     }
     .map(|hashed| spark_sha2_hex(&[hashed]).unwrap())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use arrow::datatypes::Field;
+
+    #[test]
+    fn test_sha2_nullability() -> Result<()> {
+        let func = SparkSha2::new();
+
+        let non_nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, false));
+        let non_nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, false));
+
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(!out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let null_expr: FieldRef = Arc::new(Field::new("expr", DataType::Null, 
true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[null_expr, Arc::clone(&non_nullable_bit_length)],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Null);
+
+        let null_bit_length: FieldRef =

Review Comment:
   What is the difference with 
https://github.com/apache/datafusion/pull/19323/changes#diff-288c6716efda738c6ce1f71da2e679e600cfa79b78aeadd7871ce063d701d334R304-R314
 ?



##########
datafusion/spark/src/function/hash/sha2.rs:
##########
@@ -225,3 +264,94 @@ fn compute_sha2(
     }
     .map(|hashed| spark_sha2_hex(&[hashed]).unwrap())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use arrow::datatypes::Field;
+
+    #[test]
+    fn test_sha2_nullability() -> Result<()> {
+        let func = SparkSha2::new();
+
+        let non_nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, false));
+        let non_nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, false));
+
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(!out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let null_expr: FieldRef = Arc::new(Field::new("expr", DataType::Null, 
true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[null_expr, Arc::clone(&non_nullable_bit_length)],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Null);
+
+        let null_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Null, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[Arc::clone(&non_nullable_expr), null_bit_length],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Null);
+
+        let null_scalar = ScalarValue::Int32(None);
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::new(Field::new("bit_length", DataType::Int32, false)),

Review Comment:
   ```suggestion
                   Arc::new(&non_nullable_bit_length),
   ```



##########
datafusion/spark/src/function/hash/sha2.rs:
##########
@@ -225,3 +264,94 @@ fn compute_sha2(
     }
     .map(|hashed| spark_sha2_hex(&[hashed]).unwrap())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use arrow::datatypes::Field;
+
+    #[test]
+    fn test_sha2_nullability() -> Result<()> {
+        let func = SparkSha2::new();
+
+        let non_nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, false));
+        let non_nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, false));
+
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(!out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let null_expr: FieldRef = Arc::new(Field::new("expr", DataType::Null, 
true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[null_expr, Arc::clone(&non_nullable_bit_length)],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Null);
+
+        let null_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Null, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[Arc::clone(&non_nullable_expr), null_bit_length],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Null);
+
+        let null_scalar = ScalarValue::Int32(None);
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::new(Field::new("bit_length", DataType::Int32, false)),
+            ],
+            scalar_arguments: &[None, Some(&null_scalar)],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let unsupported_scalar = ScalarValue::Int32(Some(128));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::new(Field::new("expr", DataType::Binary, false)),
+                Arc::new(Field::new("bit_length", DataType::Int32, false)),

Review Comment:
   ```suggestion
                   Arc::new(&non_nullable_bit_length),
   ```



##########
datafusion/spark/src/function/hash/sha2.rs:
##########
@@ -225,3 +264,94 @@ fn compute_sha2(
     }
     .map(|hashed| spark_sha2_hex(&[hashed]).unwrap())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use arrow::datatypes::Field;
+
+    #[test]
+    fn test_sha2_nullability() -> Result<()> {
+        let func = SparkSha2::new();
+
+        let non_nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, false));
+        let non_nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, false));
+
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(!out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let null_expr: FieldRef = Arc::new(Field::new("expr", DataType::Null, 
true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[null_expr, Arc::clone(&non_nullable_bit_length)],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Null);

Review Comment:
   Please add comments above each case.
   I don't see what is the difference between nullable_expr 
(https://github.com/apache/datafusion/pull/19323/changes#diff-288c6716efda738c6ce1f71da2e679e600cfa79b78aeadd7871ce063d701d334R292-R302)
 and null_expr 
(https://github.com/apache/datafusion/pull/19323/changes#diff-288c6716efda738c6ce1f71da2e679e600cfa79b78aeadd7871ce063d701d334R316-R322)



##########
datafusion/spark/src/function/hash/sha2.rs:
##########
@@ -225,3 +264,94 @@ fn compute_sha2(
     }
     .map(|hashed| spark_sha2_hex(&[hashed]).unwrap())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use arrow::datatypes::Field;
+
+    #[test]
+    fn test_sha2_nullability() -> Result<()> {
+        let func = SparkSha2::new();
+
+        let non_nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, false));
+        let non_nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, false));
+
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(!out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_expr: FieldRef =
+            Arc::new(Field::new("expr", DataType::Binary, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&nullable_expr),
+                Arc::clone(&non_nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let nullable_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Int32, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::clone(&nullable_bit_length),
+            ],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let null_expr: FieldRef = Arc::new(Field::new("expr", DataType::Null, 
true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[null_expr, Arc::clone(&non_nullable_bit_length)],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Null);
+
+        let null_bit_length: FieldRef =
+            Arc::new(Field::new("bit_length", DataType::Null, true));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[Arc::clone(&non_nullable_expr), null_bit_length],
+            scalar_arguments: &[None, None],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Null);
+
+        let null_scalar = ScalarValue::Int32(None);
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::clone(&non_nullable_expr),
+                Arc::new(Field::new("bit_length", DataType::Int32, false)),
+            ],
+            scalar_arguments: &[None, Some(&null_scalar)],
+        })?;
+        assert!(out.is_nullable());
+        assert_eq!(out.data_type(), &DataType::Utf8);
+
+        let unsupported_scalar = ScalarValue::Int32(Some(128));
+        let out = func.return_field_from_args(ReturnFieldArgs {
+            arg_fields: &[
+                Arc::new(Field::new("expr", DataType::Binary, false)),

Review Comment:
   ```suggestion
                   Arc::new(&non_nullable_expr),
   ```



-- 
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]

Reply via email to