martin-g commented on code in PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#discussion_r2839937051
##########
datafusion/spark/src/function/array/slice.rs:
##########
@@ -170,3 +176,40 @@ fn calculate_start_end(args: &[ArrayRef]) ->
Result<(ArrayRef, ArrayRef)> {
Ok((Arc::new(adjusted_start.finish()), Arc::new(end.finish())))
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use arrow::array::NullArray;
+ use arrow::datatypes::DataType::List;
+ use arrow::datatypes::Field;
+ use datafusion_common::ScalarValue;
+
+ #[test]
+ fn test_spark_slice_function_when_input_array_is_null() {
+ let input_args = vec![
+ ColumnarValue::Array(Arc::new(NullArray::new(1))),
+ ColumnarValue::Scalar(ScalarValue::Int64(Some(1))),
+ ColumnarValue::Scalar(ScalarValue::Int64(Some(3))),
+ ];
+
+ let args = ScalarFunctionArgs {
+ args: input_args.to_owned(),
+ arg_fields: vec![Arc::new(Field::new(
+ "item",
+ List(FieldRef::new(Field::new("", DataType::Int64, true))),
+ false,
+ ))],
+ number_rows: 0,
Review Comment:
```suggestion
number_rows: 1,
```
to match with the length of `NullArray::new(1)`
##########
datafusion/spark/src/function/array/slice.rs:
##########
@@ -94,6 +96,10 @@ impl ScalarUDFImpl for SparkSlice {
&self,
mut func_args: ScalarFunctionArgs,
) -> Result<ColumnarValue> {
+ if func_args.args[0].data_type() == DataType::Null {
+ return Ok::<ColumnarValue,
DataFusionError>(func_args.args[0].clone());
Review Comment:
Is there a need of the turbofish here ?
```suggestion
return Ok(func_args.args[0].clone());
```
This will also make the new import of `DataFusionError` not needed
##########
datafusion/spark/src/function/array/slice.rs:
##########
@@ -170,3 +176,40 @@ fn calculate_start_end(args: &[ArrayRef]) ->
Result<(ArrayRef, ArrayRef)> {
Ok((Arc::new(adjusted_start.finish()), Arc::new(end.finish())))
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use arrow::array::NullArray;
+ use arrow::datatypes::DataType::List;
+ use arrow::datatypes::Field;
+ use datafusion_common::ScalarValue;
+
+ #[test]
+ fn test_spark_slice_function_when_input_array_is_null() {
+ let input_args = vec![
+ ColumnarValue::Array(Arc::new(NullArray::new(1))),
+ ColumnarValue::Scalar(ScalarValue::Int64(Some(1))),
+ ColumnarValue::Scalar(ScalarValue::Int64(Some(3))),
+ ];
+
+ let args = ScalarFunctionArgs {
+ args: input_args.to_owned(),
+ arg_fields: vec![Arc::new(Field::new(
+ "item",
+ List(FieldRef::new(Field::new("", DataType::Int64, true))),
Review Comment:
```suggestion
List(FieldRef::new(Field::new("f", DataType::Int64, true))),
```
It looks weird to use an empty name for a field.
##########
datafusion/spark/src/function/array/slice.rs:
##########
@@ -94,6 +96,10 @@ impl ScalarUDFImpl for SparkSlice {
&self,
mut func_args: ScalarFunctionArgs,
) -> Result<ColumnarValue> {
+ if func_args.args[0].data_type() == DataType::Null {
+ return Ok::<ColumnarValue,
DataFusionError>(func_args.args[0].clone());
+ };
Review Comment:
```suggestion
}
```
##########
datafusion/spark/src/function/array/slice.rs:
##########
@@ -170,3 +176,40 @@ fn calculate_start_end(args: &[ArrayRef]) ->
Result<(ArrayRef, ArrayRef)> {
Ok((Arc::new(adjusted_start.finish()), Arc::new(end.finish())))
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use arrow::array::NullArray;
+ use arrow::datatypes::DataType::List;
+ use arrow::datatypes::Field;
+ use datafusion_common::ScalarValue;
+
+ #[test]
+ fn test_spark_slice_function_when_input_array_is_null() {
+ let input_args = vec![
+ ColumnarValue::Array(Arc::new(NullArray::new(1))),
+ ColumnarValue::Scalar(ScalarValue::Int64(Some(1))),
+ ColumnarValue::Scalar(ScalarValue::Int64(Some(3))),
+ ];
+
+ let args = ScalarFunctionArgs {
+ args: input_args.to_owned(),
+ arg_fields: vec![Arc::new(Field::new(
+ "item",
+ List(FieldRef::new(Field::new("", DataType::Int64, true))),
+ false,
+ ))],
+ number_rows: 0,
+ return_field: Arc::new(Field::new(
+ "item",
+ List(FieldRef::new(Field::new_list_field(DataType::Int64,
true))),
+ false,
+ )),
+ config_options: Arc::new(Default::default()),
+ };
+ let slice = SparkSlice::new();
+ let result = slice.invoke_with_args(args).unwrap();
+ assert!(result.to_array(1).unwrap() == Arc::new(NullArray::new(1)));
Review Comment:
```suggestion
assert_eq!(result.to_array(1).unwrap(), Arc::new(NullArray::new(1)));
```
##########
datafusion/spark/src/function/array/slice.rs:
##########
@@ -170,3 +176,40 @@ fn calculate_start_end(args: &[ArrayRef]) ->
Result<(ArrayRef, ArrayRef)> {
Ok((Arc::new(adjusted_start.finish()), Arc::new(end.finish())))
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use arrow::array::NullArray;
+ use arrow::datatypes::DataType::List;
+ use arrow::datatypes::Field;
+ use datafusion_common::ScalarValue;
+
+ #[test]
+ fn test_spark_slice_function_when_input_array_is_null() {
+ let input_args = vec![
+ ColumnarValue::Array(Arc::new(NullArray::new(1))),
+ ColumnarValue::Scalar(ScalarValue::Int64(Some(1))),
+ ColumnarValue::Scalar(ScalarValue::Int64(Some(3))),
+ ];
+
+ let args = ScalarFunctionArgs {
+ args: input_args.to_owned(),
Review Comment:
```suggestion
args: input_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]
