jhorstmann commented on code in PR #11136:
URL: https://github.com/apache/arrow-rs/pull/11136#discussion_r4429109207
##########
parquet/src/encodings/decoding.rs:
##########
@@ -2190,6 +2190,35 @@ mod tests {
test_byte_stream_split_decode::<Int64Type>(data, -1);
}
+ #[test]
+ fn test_byte_stream_split_uses_encoded_value_count() {
+ let mut decoder = ByteStreamSplitDecoder::<DoubleType>::new();
+
+ decoder.set_data(Bytes::from(vec![0; 64]), 63).unwrap();
Review Comment:
I think the check for `data.len()` being a multiple of type_size can stay in
`set_data`, but the `num_values` parameter there can overestimate, since it is
really the number of levels. Other decoders already have to treat it as an
estimate. We only know how many values are really read inside `get` and the
missing check there probably looks something like
```rust
if self.encoded_bytes.len() / type_size < self.values_decoded +
num_values {
return Err(general_err!("Invalid BYTE_STREAM_SPLIT data length
for {} values: {}", self.values_decoded + num_values,
self.encoded_bytes.len()));
}
```
Or in `set_data` the `num_values` parameter could be ignored and the total
instead calculated from `data.len()`, that should then also fail later when
trying to read more than the available values.
--
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]
