You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "erenavsarogullari (via GitHub)" <gi...@apache.org> on 2026/02/21 22:19:06 UTC
[PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
erenavsarogullari opened a new pull request, #20469:
URL: https://github.com/apache/datafusion/pull/20469
## Which issue does this PR close?
- Closes #20466.
## Rationale for this change
Currently, Spark `slice` function accepts Null Arrays and return `Null` for this particular queries. DataFusion-Spark `slice` function also needs to return `NULL` when Null Array is set.
**Spark Behavior** (tested with latest Spark master):
```
> SELECT slice(NULL, 1, 2);
+-----------------+
|slice(NULL, 1, 2)|
+-----------------+
| null|
+-----------------+
```
**DF Behaviour:**
Current:
```
query error
SELECT slice(NULL, 1, 2);
----
DataFusion error: Internal error: could not cast array of type Null to arrow_array::array::list_array::GenericListArray<i32>.
This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues
```
New:
```
query ?
SELECT slice(NULL, 1, 2);
----
NULL
```
## What changes are included in this PR?
Explained under first section.
## Are these changes tested?
Added new UT cases for both `slice.rs` and `slice.slt`.
## Are there any user-facing changes?
Yes, currently, `slice` function returns error message for `Null` Array inputs, however, expected behavior is to be returned `NULL` so end-user will get expected result instead of error message.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#issuecomment-4258134788
Thanks @erenavsarogullari & @martin-g
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "erenavsarogullari (via GitHub)" <gi...@apache.org>.
erenavsarogullari commented on PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#issuecomment-3977590348
> This seems to suggest return type should be list of nulls instead of just null
Yes, latest fix aims to address this by returning `NullArray` instead of Scalar `Null` value.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey merged PR #20469:
URL: https://github.com/apache/datafusion/pull/20469
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "erenavsarogullari (via GitHub)" <gi...@apache.org>.
erenavsarogullari commented on PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#issuecomment-3941503978
Spark returns `Array as (nullable = true / false)` and there are 2 representations as follows in terms of `input array` so it returns input array for both cases as either `NULL` or `[NULL]`:
**Case1:** input array is null (`array (nullable = true)`):
```
+-----------------+
|slice(NULL, 1, 2)|
+-----------------+
| NULL|
+-----------------+
root
|-- slice(NULL, 1, 2): array (nullable = true)
| |-- element: void (containsNull = true)
```
**Case2:** input array has null element (`array (nullable = false)`):
```
+------------------------+
|slice(array(NULL), 1, 2)|
+------------------------+
| [NULL]|
+------------------------+
root
|-- slice(array(NULL), 1, 2): array (nullable = false)
| |-- element: void (containsNull = true)
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#issuecomment-3942104070
This seems to suggest return type should be list of nulls instead of just null
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#issuecomment-3939817162
Does Spark return null (void) or an array of null (void)?
I tested on PySpark 4.1.1
```python
>>> spark.sql("select slice(NULL, 1, 2) as a").printSchema()
root
|-- a: array (nullable = true)
| |-- element: void (containsNull = true)
```
- Seems to suggest the latter?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "erenavsarogullari (via GitHub)" <gi...@apache.org>.
erenavsarogullari commented on PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#issuecomment-4257632486
Last review comment has been addressed and last CI run has been successful. Thanks for the reviews.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#discussion_r3090554334
##########
datafusion/spark/src/function/array/slice.rs:
##########
@@ -78,17 +78,28 @@ impl ScalarUDFImpl for SparkSlice {
fn return_field_from_args(&self, args: ReturnFieldArgs) -> Result<FieldRef> {
let nullable = args.arg_fields.iter().any(|f| f.is_nullable());
- Ok(Arc::new(Field::new(
- "slice",
- args.arg_fields[0].data_type().clone(),
- nullable,
- )))
+ let data_type = match args.arg_fields[0].data_type() {
+ DataType::Null => {
+ DataType::List(Arc::new(Field::new_list_field(DataType::Null, true)))
+ }
+ dt => dt.clone(),
+ };
+
+ Ok(Arc::new(Field::new("slice", data_type, nullable)))
}
fn invoke_with_args(
&self,
mut func_args: ScalarFunctionArgs,
) -> Result<ColumnarValue> {
+ if func_args.args[0].data_type() == DataType::Null {
+ let len = match &func_args.args[0] {
+ ColumnarValue::Array(a) => a.len(),
+ ColumnarValue::Scalar(_) => func_args.number_rows,
+ };
+ return Ok(ColumnarValue::Array(list_null_array(len)));
Review Comment:
```suggestion
return Ok(ColumnarValue::Scalar(ScalarValue::new_null_list(
DataType::Null,
true,
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#issuecomment-3984103917
> > This seems to suggest return type should be list of nulls instead of just null
>
> Yes, latest fix aims to address this by returning `NullArray` instead of Scalar `Null` value.
Sorry I don't see how this relates? I mean it seems to suggest that a `DataType::Null` input has a `DataType::List(Null)` output, according to my PySpark test. But maybe the test was incorrect, since perhaps Spark introduces some casts? Could we confirm the behaviour if we want to align with Spark here?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Fix Spark `slice` function `Null` type to `GenericListArray` casting issue [datafusion]
Posted by "erenavsarogullari (via GitHub)" <gi...@apache.org>.
erenavsarogullari commented on PR #20469:
URL: https://github.com/apache/datafusion/pull/20469#issuecomment-4189630198
@Jefffrey Sorry for the delay. I have just submitted the latest commit which returns `DataType::List(Null)` for `DataType::Null` input and and added test cases verifying the output type.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org