alamb commented on code in PR #11136:
URL: https://github.com/apache/arrow-rs/pull/11136#discussion_r4480354841
##########
parquet/tests/arrow_reader/bad_data.rs:
##########
@@ -149,6 +155,56 @@ fn test_arrow_gh_47662() {
);
}
+#[test]
+fn test_byte_stream_split_truncated_page() {
Review Comment:
how about we just check in the 488 byte bad file rahter than trying t
manually re-create it each time?
##########
parquet/src/encodings/decoding/byte_stream_split_decoder.rs:
##########
@@ -86,9 +86,18 @@ fn join_streams_variable(
}
impl<T: DataType> Decoder<T> for ByteStreamSplitDecoder<T> {
- fn set_data(&mut self, data: Bytes, num_values: usize) -> Result<()> {
+ fn set_data(&mut self, data: Bytes, _num_values: usize) -> Result<()> {
Review Comment:
Why not check that num_values is is consistent with `data.len() /
type_size;`? It seems like by ignorning it we may be allowing other
inconsistencies?
##########
parquet/src/encodings/decoding/byte_stream_split_decoder.rs:
##########
@@ -164,7 +173,7 @@ impl<T: DataType> VariableWidthByteStreamSplitDecoder<T> {
}
impl<T: DataType> Decoder<T> for VariableWidthByteStreamSplitDecoder<T> {
- fn set_data(&mut self, data: Bytes, num_values: usize) -> Result<()> {
+ fn set_data(&mut self, data: Bytes, _num_values: usize) -> Result<()> {
Review Comment:
same comment as above
##########
parquet/src/column/reader/decoder.rs:
##########
@@ -684,8 +684,49 @@ impl RepetitionLevelDecoder for RepetitionLevelDecoderImpl
{
#[cfg(test)]
mod tests {
use super::*;
+ use crate::data_type::DoubleType;
use crate::encodings::rle::RleEncoder;
+ use crate::schema::types::{ColumnDescriptor, ColumnPath, Type as
SchemaType};
use rand::{prelude::*, rng};
+ use std::sync::Arc;
+
+ #[test]
+ fn test_byte_stream_split_v1_nulls_use_encoded_len() {
Review Comment:
What is the value of this test? Does it add significant coverage over an end
to end reproducer?
--
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]
