scovich commented on code in PR #8887:
URL: https://github.com/apache/arrow-rs/pull/8887#discussion_r2550298276
##########
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:
Meanwhile: This is a single-caller function with _very_ specific semantics.
Perhaps it's better to fold it in at the call site, where the necessary
semantics are more obvious? Especially if we're able to simplify it following
the above observation?
Something like:
```rust
if let Some(typed_value) = target.typed_value_field() {
let perfectly_shredded = typed_value.null_count() == 0;
let as_type = as_field.data_type();
if perfectly_shredded && can_cast_types(typed_value.data_type(),
as_type) {
let mut array = match target.nulls() {
None => typed_value.clone(),
nulls => {
let builder = array.to_data().into_builder();
make_array(builder.nulls(nulls.clone()).build()?)
}
};
if typed_value.data_type() != as_type {
array = cast(array, as_type)?;
}
return Ok(array);
}
}
```
--
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]
