alamb commented on code in PR #7945:
URL: https://github.com/apache/arrow-rs/pull/7945#discussion_r2213648198
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -59,6 +59,8 @@ pub struct VariantArray {
/// Dictionary-Encoded, preferably (but not required) with an index type of
/// int8.
inner: StructArray,
+ metadata_ref: ArrayRef,
Review Comment:
It would be nice to add some comment that explain this is a cached copy of
the metadata / value child arrays
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -88,7 +90,10 @@ impl VariantArray {
));
};
// Ensure the StructArray has a metadata field of BinaryView
- let Some(metadata_field) = inner.fields().iter().find(|f| f.name() ==
"metadata") else {
+
+ let cloned = inner.clone();
Review Comment:
I think it is ok to clone the argument here even if it will be wasteful on
the error case, as the error case is not commonly expected
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -169,8 +184,13 @@ impl Array for VariantArray {
}
fn slice(&self, offset: usize, length: usize) -> ArrayRef {
+ let slice = self.inner.slice(offset, length);
+ let met = VariantArray::find_metadata_field(&slice).unwrap();
+ let val = VariantArray::find_value_field(&slice).unwrap();
Review Comment:
I agree it is correct to slice the child arrays
so something like
```rust
let val = self.value_ref.slice(offset, length);
```
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
