alamb commented on code in PR #9174:
URL: https://github.com/apache/arrow-datafusion/pull/9174#discussion_r1484814513
##########
datafusion/common/src/scalar.rs:
##########
@@ -1591,10 +1476,34 @@ impl ScalarValue {
DataType::Interval(IntervalUnit::MonthDayNano) => {
build_array_primitive!(IntervalMonthDayNanoArray,
IntervalMonthDayNano)
}
- DataType::Struct(_) => build_struct_array(scalars)?,
- DataType::List(_)
- | DataType::LargeList(_)
- | DataType::FixedSizeList(_, _) => build_list_array(scalars)?,
+ DataType::FixedSizeList(_, _) => {
+ // arrow::compute::concat does not allow inconsistent types
including the size of FixedSizeList.
+ // The length of nulls here we got is 1, so we need to resize
the length of nulls to
+ // the length of non-nulls.
+ let mut arrays =
+ scalars.map(|s| s.to_array()).collect::<Result<Vec<_>>>()?;
+ let first_non_null_data_type = arrays
+ .iter()
+ .find(|sv| !sv.is_null(0))
+ .map(|sv| sv.data_type().to_owned());
+ if let Some(DataType::FixedSizeList(f, l)) =
first_non_null_data_type {
+ for array in arrays.iter_mut() {
+ if array.is_null(0) {
+ *array =
+
Arc::new(FixedSizeListArray::new_null(f.clone(), l, 1));
+ }
+ }
+ }
+ let arrays = arrays.iter().map(|a|
a.as_ref()).collect::<Vec<_>>();
+ arrow::compute::concat(arrays.as_slice())
+ .map_err(|e| arrow_datafusion_err!(e))?
Review Comment:
Since there is an automatic conversion from `ArrowError` to
`DataFusionError` you can do this:
```suggestion
arrow::compute::concat(arrays.as_slice())?
```
I verified that this works locally. However I think the code in this PR just
follows the pattern as the reset of this file. I'll make a PR to clean that up
##########
datafusion/common/src/scalar.rs:
##########
@@ -1591,10 +1476,34 @@ impl ScalarValue {
DataType::Interval(IntervalUnit::MonthDayNano) => {
build_array_primitive!(IntervalMonthDayNanoArray,
IntervalMonthDayNano)
}
- DataType::Struct(_) => build_struct_array(scalars)?,
- DataType::List(_)
- | DataType::LargeList(_)
- | DataType::FixedSizeList(_, _) => build_list_array(scalars)?,
+ DataType::FixedSizeList(_, _) => {
+ // arrow::compute::concat does not allow inconsistent types
including the size of FixedSizeList.
+ // The length of nulls here we got is 1, so we need to resize
the length of nulls to
+ // the length of non-nulls.
+ let mut arrays =
+ scalars.map(|s| s.to_array()).collect::<Result<Vec<_>>>()?;
+ let first_non_null_data_type = arrays
+ .iter()
+ .find(|sv| !sv.is_null(0))
+ .map(|sv| sv.data_type().to_owned());
+ if let Some(DataType::FixedSizeList(f, l)) =
first_non_null_data_type {
+ for array in arrays.iter_mut() {
+ if array.is_null(0) {
+ *array =
+
Arc::new(FixedSizeListArray::new_null(f.clone(), l, 1));
+ }
+ }
+ }
+ let arrays = arrays.iter().map(|a|
a.as_ref()).collect::<Vec<_>>();
+ arrow::compute::concat(arrays.as_slice())
+ .map_err(|e| arrow_datafusion_err!(e))?
+ }
+ DataType::List(_) | DataType::LargeList(_) | DataType::Struct(_)
=> {
+ let arrays = scalars.map(|s|
s.to_array()).collect::<Result<Vec<_>>>()?;
+ let arrays = arrays.iter().map(|a|
a.as_ref()).collect::<Vec<_>>();
+ arrow::compute::concat(arrays.as_slice())
+ .map_err(|e| arrow_datafusion_err!(e))?
Review Comment:
```suggestion
arrow::compute::concat(arrays.as_slice())?
```
--
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]
