friendlymatthew commented on code in PR #7906:
URL: https://github.com/apache/arrow-rs/pull/7906#discussion_r2201515136
##########
parquet-variant/src/variant/list.rs:
##########
@@ -216,28 +216,19 @@ impl<'m, 'v> VariantList<'m, 'v> {
self.header.first_offset_byte() as _..self.first_value_byte as
_,
)?;
- let offsets =
- map_bytes_to_offsets(offset_buffer,
self.header.offset_size).collect::<Vec<_>>();
-
- // Validate offsets are in-bounds and monotonically increasing.
- // Since shallow verification checks whether the first and last
offsets are in-bounds,
- // we can also verify all offsets are in-bounds by checking if
offsets are monotonically increasing.
- let are_offsets_monotonic = offsets.is_sorted_by(|a, b| a < b);
- if !are_offsets_monotonic {
- return Err(ArrowError::InvalidArgumentError(
- "offsets are not monotonically increasing".to_string(),
- ));
- }
-
let value_buffer = slice_from_slice(self.value,
self.first_value_byte as _..)?;
// Validate whether values are valid variant objects
- for i in 1..offsets.len() {
- let start_offset = offsets[i - 1];
- let end_offset = offsets[i];
-
- let value_bytes = slice_from_slice(value_buffer,
start_offset..end_offset)?;
+ //
+ // Since we use offsets to slice into the value buffer, we can
also verify all offsets are in-bounds
+ // and monotonically increasing
Review Comment:
Yes, this was my thinking
`slice_from_slice` will err if the slice range is invalid. A slice range is
invalid when the start offset is greater than the end offset.
So when we iterate through the offsets, we build our slice range
`offsets[i]..offsets[i + 1]`. If every slice attempt is successful, we can
guarantee offsets are non-decreasing.
We still need to check if `offsets[i] != offsets[i + 1]` for any `i`. This
is still a valid range and `slice_from_slice` will return a valid slice (empty
bytes).
This is when the `Variant::try_new_with_metadata` will err.
```rs
#[test]
fn foo() {
let metadata = VariantMetadata::try_new(&[]).unwrap();
// the following will panic:
// called `Result::unwrap()` on an `Err` value:
InvalidArgumentError("Received empty bytes")
let v = Variant::try_new_with_metadata(metadata, &[]).unwrap();
}
```
--
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
