Jefffrey commented on code in PR #10726:
URL: https://github.com/apache/arrow-rs/pull/10726#discussion_r4102307389


##########
arrow-cast/src/cast/list.rs:
##########
@@ -15,8 +15,110 @@
 // specific language governing permissions and limitations
 // under the License.
 
+use arrow_buffer::{BooleanBufferBuilder, NullBuffer};
+
 use crate::cast::*;
 
+/// Returns a clone of `values` with parent nulls propagated to the child 
elements.
+pub(crate) fn mask_fixed_size_list_values(array: &FixedSizeListArray) -> 
ArrayRef {
+    let size = array.value_length() as usize;
+    let Some(nulls) = array.nulls() else {
+        return Arc::clone(array.values());
+    };
+    if size == 0 {
+        return Arc::clone(array.values());
+    }
+
+    let parent_nulls = nulls.expand(size);
+    let combined_nulls = NullBuffer::union(Some(&parent_nulls), 
array.values().nulls());
+    if combined_nulls.as_ref() != array.values().nulls() {
+        let data = array
+            .values()
+            .to_data()
+            .into_builder()
+            .nulls(combined_nulls)
+            .build();
+        match data {
+            Ok(d) => make_array(d),
+            Err(_) => Arc::clone(array.values()),
+        }
+    } else {
+        Arc::clone(array.values())
+    }
+}
+
+/// Returns a clone of `values` with parent nulls propagated to the child 
elements for a ListArray.
+pub(crate) fn mask_list_values<O: OffsetSizeTrait>(array: 
&GenericListArray<O>) -> ArrayRef {
+    if array.null_count() == 0 || array.values().is_empty() {
+        return Arc::clone(array.values());
+    }
+
+    let mut builder = BooleanBufferBuilder::new(array.values().len());
+    let offsets = array.value_offsets();
+    for i in 0..array.len() {
+        let is_valid = array.is_valid(i);
+        let len = offsets[i + 1].as_usize() - offsets[i].as_usize();
+        builder.append_n(len, is_valid);
+    }
+    if builder.len() < array.values().len() {
+        builder.append_n(array.values().len() - builder.len(), false);
+    }
+    let parent_nulls = NullBuffer::from(builder.finish());
+    let combined_nulls = NullBuffer::union(Some(&parent_nulls), 
array.values().nulls());
+    if combined_nulls.as_ref() != array.values().nulls() {
+        let data = array
+            .values()
+            .to_data()

Review Comment:
   i wonder if we can use the 
[`nullif`](https://docs.rs/arrow/latest/arrow/compute/fn.nullif.html) kernel 
here actually. we can skip it for fixedsizelist since that is a simple 
nullbuffer replacement
   
   and if we can use `nullif` here we can probably use it for list view, maps, 
structs, etc.



##########
arrow-cast/src/cast/list.rs:
##########
@@ -76,8 +178,9 @@ pub(crate) fn cast_single_element_fixed_size_list_to_values(
     to: &DataType,
     cast_options: &CastOptions,
 ) -> Result<ArrayRef, ArrowError> {
-    let values = array.as_fixed_size_list().values();
-    cast_with_options(values, to, cast_options)
+    let array = array.as_fixed_size_list();
+    let values = mask_fixed_size_list_values(array);

Review Comment:
   we should probably mask only on cast failure (and then retry the cast); this 
avoids affecting the fast (optimistic) path
   
   and consider we only need to do this masking for `safe: false` casting



##########
arrow-cast/src/cast/list.rs:
##########
@@ -15,8 +15,110 @@
 // specific language governing permissions and limitations
 // under the License.
 
+use arrow_buffer::{BooleanBufferBuilder, NullBuffer};
+
 use crate::cast::*;
 
+/// Returns a clone of `values` with parent nulls propagated to the child 
elements.
+pub(crate) fn mask_fixed_size_list_values(array: &FixedSizeListArray) -> 
ArrayRef {
+    let size = array.value_length() as usize;
+    let Some(nulls) = array.nulls() else {
+        return Arc::clone(array.values());
+    };
+    if size == 0 {
+        return Arc::clone(array.values());
+    }
+
+    let parent_nulls = nulls.expand(size);
+    let combined_nulls = NullBuffer::union(Some(&parent_nulls), 
array.values().nulls());
+    if combined_nulls.as_ref() != array.values().nulls() {
+        let data = array
+            .values()
+            .to_data()
+            .into_builder()
+            .nulls(combined_nulls)
+            .build();
+        match data {
+            Ok(d) => make_array(d),
+            Err(_) => Arc::clone(array.values()),

Review Comment:
   i think we should propagate the error here instead of silently returning 
original



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

Reply via email to