PR comments

Author: rambleraptor

parquet/src/column/page.rs

@@ -31,7 +31,7 @@ use crate::file::statistics::{Statistics, page_stats_to_thrift};
 /// List of supported pages.
 /// These are 1-to-1 mapped from the equivalent Thrift definitions, except `buf` which
 /// used to store uncompressed bytes of the page.
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub enum Page {
     /// Data page Parquet format v1.
     DataPage {

parquet/src/encodings/decoding.rs

@@ -1394,10 +1394,7 @@ mod tests {
     fn test_dict_decoder_empty_data() {
         let mut decoder = DictDecoder::<Int32Type>::new();
         let err = decoder.set_data(Bytes::new(), 10).unwrap_err();
-        assert_eq!(
-            err.to_string(),
-            "Parquet error: Not enough bytes to decode bit_width"
-        );
+        assert_eq!(err.to_string(), "EOF: Not enough bytes to decode bit_width");
     }
 
     fn test_plain_decode<T: DataType>(

parquet/src/file/reader.rs

@@ -299,7 +299,7 @@ mod tests {
         let err = data.get_read(5).unwrap_err();
         assert_eq!(
             err.to_string(),
-            "Parquet error: Expected to read at offset 5, while file has length 4"
+            "EOF: Expected to read at offset 5, while file has length 4"
         );
     }
 
@@ -309,13 +309,13 @@ mod tests {
         let err = data.get_bytes(5, 1).unwrap_err();
         assert_eq!(
             err.to_string(),
-            "Parquet error: Expected to read 1 bytes at offset 5, while file has length 4"
+            "EOF: Expected to read 1 bytes at offset 5, while file has length 4"
         );
 
         let err = data.get_bytes(2, 3).unwrap_err();
         assert_eq!(
             err.to_string(),
-            "Parquet error: Expected to read 3 bytes at offset 2, while file has length 4"
+            "EOF: Expected to read 3 bytes at offset 2, while file has length 4"
         );
     }
 }

parquet/src/file/serialized_reader.rs

@@ -25,7 +25,7 @@ use crate::compression::{Codec, create_codec};
 #[cfg(feature = "encryption")]
 use crate::encryption::decrypt::{CryptoContext, read_and_decrypt};
 use crate::errors::{ParquetError, Result};
-use crate::file::metadata::thrift::PageHeader;
+use crate::file::metadata::thrift::{DataPageHeaderV2, PageHeader};
 use crate::file::page_index::offset_index::{OffsetIndexMetaData, PageLocation};
 use crate::file::statistics;
 use crate::file::{
@@ -1145,10 +1145,7 @@ mod tests {
 
     use super::*;
 
-    #[test]
     fn test_decode_page_invalid_offset() {
-        use crate::file::metadata::thrift_gen::DataPageHeaderV2;
-
         let page_header = PageHeader {
             r#type: PageType::DATA_PAGE_V2,
             uncompressed_page_size: 10,
@@ -1171,19 +1168,45 @@ mod tests {
 
         let buffer = Bytes::new();
         let err = decode_page(page_header, buffer, Type::INT32, None).unwrap_err();
-        assert_eq!(err.to_string(), "Parquet error: Invalid page header");
+        assert!(
+            err.to_string()
+                .contains("DataPage v2 header contains implausible values")
+        );
     }
 
-    #[test]
     fn test_decode_unsupported_page() {
-        let mut page_header = PageHeader::default();
-        page_header.r#type = PageType::INDEX_PAGE;
+        let mut page_header = PageHeader {
+            r#type: PageType::INDEX_PAGE,
+            uncompressed_page_size: 10,
+            compressed_page_size: 10,
+            data_page_header: None,
+            index_page_header: None,
+            dictionary_page_header: None,
+            crc: None,
+            data_page_header_v2: None,
+        };
         let buffer = Bytes::new();
-        let err = decode_page(page_header, buffer, Type::INT32, None).unwrap_err();
+        let err = decode_page(page_header.clone(), buffer.clone(), Type::INT32, None).unwrap_err();
         assert_eq!(
             err.to_string(),
             "Parquet error: Page type INDEX_PAGE is not supported"
         );
+
+        page_header.data_page_header_v2 = Some(DataPageHeaderV2 {
+            num_nulls: 0,
+            num_rows: 0,
+            num_values: 0,
+            encoding: Encoding::PLAIN,
+            definition_levels_byte_length: 11,
+            repetition_levels_byte_length: 0,
+            is_compressed: None,
+            statistics: None,
+        });
+        let err = decode_page(page_header, buffer, Type::INT32, None).unwrap_err();
+        assert!(
+            err.to_string()
+                .contains("DataPage v2 header contains implausible values")
+        );
     }
 
     #[test]