jaideeppyne commented on code in PR #11136:
URL: https://github.com/apache/arrow-rs/pull/11136#discussion_r4533020286
##########
parquet/src/encodings/decoding.rs:
##########
@@ -2175,6 +2175,70 @@ mod tests {
test_byte_stream_split_decode::<Int64Type>(data, -1);
}
+ #[test]
+ fn test_byte_stream_split_uses_encoded_value_count() {
Review Comment:
The file-level test checks the error returned by the reader. This test
checks the fixed-width decoder's read and skip counts directly. With eight
encoded values, reading three and skipping two must leave exactly three, even
when the page count is larger. The next oversized read returns those three and
subsequent reads return zero. Reusing the decoder in the loop also checks that
setting a new page resets its position.
##########
parquet/src/encodings/decoding.rs:
##########
@@ -2175,6 +2175,70 @@ 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();
+
+ for count in [8, 63, usize::MAX] {
+ decoder.set_data(Bytes::from(vec![0; 64]), count).unwrap();
+ assert_eq!(decoder.values_left(), 8);
+ let mut values = vec![0.; 63];
+ assert_eq!(decoder.get(&mut values[..3]).unwrap(), 3);
+ assert_eq!(decoder.skip(2).unwrap(), 2);
+ assert_eq!(decoder.get(&mut values).unwrap(), 3);
+ assert_eq!(decoder.values_left(), 0);
+ assert_eq!(decoder.get(&mut values).unwrap(), 0);
+ }
+ }
+
+ #[test]
+ fn test_byte_stream_split_rejects_invalid_data_length() {
+ let mut decoder = ByteStreamSplitDecoder::<DoubleType>::new();
+
+ let err = decoder.set_data(Bytes::from(vec![0; 63]), 8).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Parquet error: Invalid BYTE_STREAM_SPLIT data length 63 for
values of size 8"
+ );
+ }
+
+ #[test]
+ fn test_byte_stream_split_rejects_more_values_than_expected() {
+ let mut decoder = ByteStreamSplitDecoder::<DoubleType>::new();
+ let err = decoder.set_data(Bytes::from(vec![0; 64]), 7).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Parquet error: Invalid BYTE_STREAM_SPLIT data: 8 encoded values
exceeds the 7 values expected"
+ );
+
+ let mut decoder =
VariableWidthByteStreamSplitDecoder::<FixedLenByteArrayType>::new(5);
+ let err = decoder.set_data(Bytes::from(vec![0; 40]), 7).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Parquet error: Invalid BYTE_STREAM_SPLIT data: 8 encoded values
exceeds the 7 values expected"
+ );
+ }
+
+ #[test]
+ fn test_byte_stream_split_flba_uses_encoded_value_count() {
Review Comment:
This covers the same read and skip behavior in the separate
FIXED_LEN_BYTE_ARRAY decoder. The file-level regression reads doubles and never
enters this implementation. Width 5 uses the generic byte join, and width 16
uses the specialized join. Both must stop at the physical value count even when
the page count is larger.
##########
parquet/tests/arrow_reader/bad_data.rs:
##########
@@ -136,6 +136,19 @@ fn test_arrow_gh_47662() {
);
}
+#[test]
+fn test_arrow_rs_gh_11136_byte_stream_split_truncated_page() {
+ // Checked-in malformed page: declared value count exceeds
BYTE_STREAM_SPLIT payload.
+ let file =
Bytes::from_static(include_bytes!("ARROW-RS-GH-11136-bss-truncated.parquet"));
+ let reader = ArrowReaderBuilder::try_new(file).unwrap().build().unwrap();
+ let err = reader.collect::<Result<Vec<_>, _>>().unwrap_err();
+ assert!(
+ err.to_string()
+ .contains("insufficient values read from column"),
Review Comment:
The new `set_data` count error is for the opposite mismatch, where the
payload contains more complete values than the supplied upper bound. This
fixture has fewer. That direction can be valid on a nullable V1 page because
the bound includes null levels. The decoder stops at the number of values
actually present, and the column reader returns the insufficient-values error
when it cannot satisfy the read.
`test_byte_stream_split_rejects_more_values_than_expected` checks the new error
in both decoder variants.
--
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]
