comphead commented on code in PR #16203:
URL: https://github.com/apache/datafusion/pull/16203#discussion_r2124930000
##########
datafusion/functions-nested/src/extract.rs:
##########
@@ -225,44 +252,37 @@ where
return Ok(Arc::new(NullArray::new(array.len())));
}
+ // Check if we have a struct with null fields
+ let has_struct_with_null_fields = if let DataType::Struct(fields) =
values.data_type()
+ {
+ fields.iter().any(|field| field.data_type() == &Null)
+ } else {
+ false
+ };
+
+ // If we have problematic struct fields, we'll need to handle this
differently
+ if has_struct_with_null_fields {
+ return handle_struct_with_null_fields(array, indexes);
+ }
+
let original_data = values.to_data();
let capacity = Capacities::Array(original_data.len());
// use_nulls: true, we don't construct List for array_element, so we need
explicit nulls.
let mut mutable =
MutableArrayData::with_capacities(vec![&original_data], true,
capacity);
- fn adjusted_array_index<O: OffsetSizeTrait>(index: i64, len: O) ->
Result<Option<O>>
- where
- i64: TryInto<O>,
- {
- let index: O = index.try_into().map_err(|_| {
- DataFusionError::Execution(format!(
- "array_element got invalid index: {index}"
- ))
- })?;
- // 0 ~ len - 1
- let adjusted_zero_index = if index < O::usize_as(0) {
- index + len
- } else {
- index - O::usize_as(1)
- };
-
- if O::usize_as(0) <= adjusted_zero_index && adjusted_zero_index < len {
- Ok(Some(adjusted_zero_index))
- } else {
- // Out of bounds
- Ok(None)
- }
- }
-
for (row_index, offset_window) in array.offsets().windows(2).enumerate() {
let start = offset_window[0];
let end = offset_window[1];
let len = end - start;
- // array is null
- if len == O::usize_as(0) {
+ if array.is_null(row_index) || len == O::usize_as(0) {
Review Comment:
seems these 2 if statements can be merged?
##########
datafusion/functions-nested/src/extract.rs:
##########
@@ -225,44 +252,37 @@ where
return Ok(Arc::new(NullArray::new(array.len())));
}
+ // Check if we have a struct with null fields
+ let has_struct_with_null_fields = if let DataType::Struct(fields) =
values.data_type()
+ {
+ fields.iter().any(|field| field.data_type() == &Null)
+ } else {
+ false
+ };
+
+ // If we have problematic struct fields, we'll need to handle this
differently
+ if has_struct_with_null_fields {
+ return handle_struct_with_null_fields(array, indexes);
+ }
+
let original_data = values.to_data();
let capacity = Capacities::Array(original_data.len());
// use_nulls: true, we don't construct List for array_element, so we need
explicit nulls.
let mut mutable =
MutableArrayData::with_capacities(vec![&original_data], true,
capacity);
- fn adjusted_array_index<O: OffsetSizeTrait>(index: i64, len: O) ->
Result<Option<O>>
- where
- i64: TryInto<O>,
- {
- let index: O = index.try_into().map_err(|_| {
- DataFusionError::Execution(format!(
- "array_element got invalid index: {index}"
- ))
- })?;
- // 0 ~ len - 1
- let adjusted_zero_index = if index < O::usize_as(0) {
- index + len
- } else {
- index - O::usize_as(1)
- };
-
- if O::usize_as(0) <= adjusted_zero_index && adjusted_zero_index < len {
- Ok(Some(adjusted_zero_index))
- } else {
- // Out of bounds
- Ok(None)
- }
- }
-
for (row_index, offset_window) in array.offsets().windows(2).enumerate() {
let start = offset_window[0];
let end = offset_window[1];
let len = end - start;
- // array is null
- if len == O::usize_as(0) {
+ if array.is_null(row_index) || len == O::usize_as(0) {
Review Comment:
seems these 2 if statements can be merged into 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]
