XiangpengHao commented on code in PR #8887:
URL: https://github.com/apache/arrow-rs/pull/8887#discussion_r2550759267


##########
parquet-variant-compute/src/variant_get.rs:
##########
@@ -109,6 +110,30 @@ pub(crate) fn follow_shredded_path_element<'a>(
     }
 }
 
+/// Returns a cloned `ArrayRef` whose null mask is the union of the array's 
existing mask and
+/// `parent_nulls`. If `parent_nulls` is `None` or contains no nulls, the 
original array is returned.
+///
+/// This necessary because the null of the shredded value is the union of 
parent null and current nulls.
+fn clone_with_parent_nulls(
+    array: &ArrayRef,
+    parent_nulls: Option<&NullBuffer>,
+) -> Result<ArrayRef> {
+    let Some(parent_nulls) = parent_nulls else {
+        return Ok(array.clone());
+    };
+    if parent_nulls.null_count() == 0 {
+        return Ok(array.clone());
+    }
+
+    let combined_nulls = NullBuffer::union(array.as_ref().nulls(), 
Some(parent_nulls));

Review Comment:
   > I don't think we need to actually combine nulls?
    For all types other than variant object (which we explicitly skip), the 
shredding spec requires exactly one of value or typed_value to be non-NULL in 
each row
   
   Yes, nice catch!



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