fix: remove with_encoding_stats_as_mask and modify benchmarks

Author: WaterWhisperer

parquet/benches/metadata.rs

@@ -21,7 +21,7 @@ use std::sync::Arc;
 use parquet::basic::{Encoding, PageType, Type as PhysicalType};
 use parquet::file::metadata::{
     ColumnChunkMetaData, FileMetaData, PageEncodingStats, ParquetMetaData, ParquetMetaDataOptions,
-    ParquetMetaDataReader, ParquetMetaDataWriter, ParquetStatisticsPolicy, RowGroupMetaData,
+    ParquetMetaDataReader, ParquetMetaDataWriter, RowGroupMetaData,
 };
 use parquet::file::statistics::Statistics;
 use parquet::file::writer::TrackedWrite;
@@ -164,26 +164,17 @@ fn criterion_benchmark(c: &mut Criterion) {
         })
     });
 
-    let schema = ParquetMetaDataReader::decode_schema(&meta_data).unwrap();
-    let options = ParquetMetaDataOptions::new().with_schema(schema);
-    c.bench_function("decode metadata with schema", |b| {
-        b.iter(|| {
-            ParquetMetaDataReader::decode_metadata_with_options(&meta_data, Some(&options))
-                .unwrap();
-        })
-    });
-
-    let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(true);
-    c.bench_function("decode metadata with stats mask", |b| {
+    let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(false);
+    c.bench_function("decode metadata (full stats)", |b| {
         b.iter(|| {
             ParquetMetaDataReader::decode_metadata_with_options(&meta_data, Some(&options))
                 .unwrap();
         })
     });
 
-    let options =
-        ParquetMetaDataOptions::new().with_encoding_stats_policy(ParquetStatisticsPolicy::SkipAll);
-    c.bench_function("decode metadata with skip PES", |b| {
+    let schema = ParquetMetaDataReader::decode_schema(&meta_data).unwrap();
+    let options = ParquetMetaDataOptions::new().with_schema(schema);
+    c.bench_function("decode metadata with schema", |b| {
         b.iter(|| {
             ParquetMetaDataReader::decode_metadata_with_options(&meta_data, Some(&options))
                 .unwrap();
@@ -197,24 +188,16 @@ fn criterion_benchmark(c: &mut Criterion) {
         })
     });
 
-    let schema = ParquetMetaDataReader::decode_schema(&buf).unwrap();
-    let options = ParquetMetaDataOptions::new().with_schema(schema);
-    c.bench_function("decode metadata (wide) with schema", |b| {
+    let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(false);
+    c.bench_function("decode metadata (wide) (full stats)", |b| {
         b.iter(|| {
             ParquetMetaDataReader::decode_metadata_with_options(&buf, Some(&options)).unwrap();
         })
     });
 
-    let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(true);
-    c.bench_function("decode metadata (wide) with stats mask", |b| {
-        b.iter(|| {
-            ParquetMetaDataReader::decode_metadata_with_options(&buf, Some(&options)).unwrap();
-        })
-    });
-
-    let options =
-        ParquetMetaDataOptions::new().with_encoding_stats_policy(ParquetStatisticsPolicy::SkipAll);
-    c.bench_function("decode metadata (wide) with skip PES", |b| {
+    let schema = ParquetMetaDataReader::decode_schema(&buf).unwrap();
+    let options = ParquetMetaDataOptions::new().with_schema(schema);
+    c.bench_function("decode metadata (wide) with schema", |b| {
         b.iter(|| {
             ParquetMetaDataReader::decode_metadata_with_options(&buf, Some(&options)).unwrap();
         })

parquet/src/arrow/mod.rs

@@ -494,7 +494,9 @@ pub fn parquet_column<'a>(
 #[cfg(test)]
 mod test {
     use crate::arrow::ArrowWriter;
-    use crate::file::metadata::{ParquetMetaData, ParquetMetaDataReader, ParquetMetaDataWriter};
+    use crate::file::metadata::{
+        ParquetMetaData, ParquetMetaDataOptions, ParquetMetaDataReader, ParquetMetaDataWriter,
+    };
     use crate::file::properties::{EnabledStatistics, WriterProperties};
     use crate::schema::parser::parse_message_type;
     use crate::schema::types::SchemaDescriptor;
@@ -511,15 +513,17 @@ mod test {
         let parquet_bytes = create_parquet_file();
 
         // read the metadata from the file WITHOUT the page index structures
+        let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(false);
         let original_metadata = ParquetMetaDataReader::new()
-            .with_encoding_stats_as_mask(false)
+            .with_metadata_options(Some(options))
             .parse_and_finish(&parquet_bytes)
             .unwrap();
 
         // this should error because the page indexes are not present, but have offsets specified
         let metadata_bytes = metadata_to_bytes(&original_metadata);
+        let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(false);
         let err = ParquetMetaDataReader::new()
-            .with_encoding_stats_as_mask(false)
+            .with_metadata_options(Some(options))
             .with_page_indexes(true) // there are no page indexes in the metadata
             .parse_and_finish(&metadata_bytes)
             .err()
@@ -535,8 +539,9 @@ mod test {
         let parquet_bytes = create_parquet_file();
 
         // read the metadata from the file
+        let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(false);
         let original_metadata = ParquetMetaDataReader::new()
-            .with_encoding_stats_as_mask(false)
+            .with_metadata_options(Some(options))
             .parse_and_finish(&parquet_bytes)
             .unwrap();
 
@@ -548,8 +553,9 @@ mod test {
             "metadata is subset of parquet"
         );
 
+        let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(false);
         let roundtrip_metadata = ParquetMetaDataReader::new()
-            .with_encoding_stats_as_mask(false)
+            .with_metadata_options(Some(options))
             .parse_and_finish(&metadata_bytes)
             .unwrap();
 
@@ -563,16 +569,18 @@ mod test {
 
         // read the metadata from the file including the page index structures
         // (which are stored elsewhere in the footer)
+        let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(false);
         let original_metadata = ParquetMetaDataReader::new()
-            .with_encoding_stats_as_mask(false)
+            .with_metadata_options(Some(options))
             .with_page_indexes(true)
             .parse_and_finish(&parquet_bytes)
             .unwrap();
 
         // read metadata back from the serialized bytes and ensure it is the same
         let metadata_bytes = metadata_to_bytes(&original_metadata);
+        let options = ParquetMetaDataOptions::new().with_encoding_stats_as_mask(false);
         let roundtrip_metadata = ParquetMetaDataReader::new()
-            .with_encoding_stats_as_mask(false)
+            .with_metadata_options(Some(options))
             .with_page_indexes(true)
             .parse_and_finish(&metadata_bytes)
             .unwrap();

parquet/src/file/metadata/reader.rs

@@ -152,20 +152,6 @@ impl ParquetMetaDataReader {
             .with_offset_index_policy(policy)
     }
 
-    /// Sets whether to read [`PageEncodingStats`] as a bitmask.
-    ///
-    /// See [`ParquetMetaDataOptions::with_encoding_stats_as_mask`] for more details.
-    pub fn with_encoding_stats_as_mask(mut self, val: bool) -> Self {
-        let mut options = self
-            .metadata_options
-            .as_deref()
-            .cloned()
-            .unwrap_or_default();
-        options = options.with_encoding_stats_as_mask(val);
-        self.metadata_options = Some(Arc::new(options));
-        self
-    }
-
     /// Sets the [`PageIndexPolicy`] for the column index
     pub fn with_column_index_policy(mut self, policy: PageIndexPolicy) -> Self {
         self.column_index = policy;