carpecodeum commented on code in PR #8021:
URL: https://github.com/apache/arrow-rs/pull/8021#discussion_r2249639145
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -186,13 +340,11 @@ impl Array for VariantArray {
}
fn slice(&self, offset: usize, length: usize) -> ArrayRef {
- let slice = self.inner.slice(offset, length);
- let met = self.metadata_ref.slice(offset, length);
- let val = self.value_ref.slice(offset, length);
+ let inner = self.inner.slice(offset, length);
+ let shredding_state = self.shredding_state.slice(offset, length);
Arc::new(Self {
Review Comment:
can we avoid unnecessary allocations for common cases, like -
```
let shredding_state = match (&self.shredding_state, offset, length) {
// Fast path: no slice needed for full array
(state, 0, len) if len == self.len() => state.clone(),
// Fast path: uniform shredding doesn't need slicing
(ShreddingState::None, _, _) => ShreddingState::None,
(ShreddingState::All, _, _) => ShreddingState::All,
// Only slice for mixed case
(ShreddingState::Mixed(bitmap), offset, length) => {
ShreddingState::Mixed(bitmap.slice(offset, length))
}
};
```
##########
parquet-variant-compute/src/from_json.rs:
##########
@@ -69,8 +69,8 @@ mod test {
let array_ref: ArrayRef = Arc::new(input);
let variant_array = batch_json_string_to_variant(&array_ref).unwrap();
- let metadata_array = variant_array.metadata_field().as_binary_view();
- let value_array = variant_array.value_field().as_binary_view();
+ let metadata_array = variant_array.metadata_field();
+ let value_array = variant_array.value_field().expect("value field");
Review Comment:
Using expect() will cause panics. Since this is a library function,
shouldn't it return Result<VariantArray, ArrowError> and let callers handle the
error gracefully? Something like
```
let value_array = variant_array.value_field()
.ok_or_else(|| ArrowError::InvalidArgumentError(
"value field not available - array may be fully shredded".to_string()
))?;
```
--
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
