alamb commented on code in PR #27022:
URL: https://github.com/apache/datafusion/pull/27022#discussion_r4473264038


##########
datafusion/core/tests/parquet/io/bloom_filter_io.rs:
##########
@@ -3,6 +3,24 @@
 // SPDX-FileContributor: Licensed to the Apache Software Foundation (ASF) 
under one or more contributor license agreements
 
 //! Tests tracking storage I/O events during Parquet Bloom filter evaluation.
+//!
+//! The file has a single row group with Bloom filters on both columns and no
+//! statistics, so only the Bloom filters are used to prune.
+//!
+//! ```text
+//!   row group 0
+//! ┌──────┬───────┐

Review Comment:
   I do like ascii art



##########
datafusion/core/tests/parquet/io/mod.rs:
##########
@@ -27,31 +28,95 @@ use object_store::{
     ObjectStore, PutMultipartOptions, PutOptions, PutPayload, PutResult,
 };
 use parquet::file::reader::{FileReader, SerializedFileReader};
+use parquet::file::serialized_reader::ReadOptionsBuilder;
 use url::Url;
 
 // Tests are in submodules
 mod bloom_filter_io;
+mod filter_pushdown_io;
+mod page_pruning_io;
+mod row_group_pruning_io;
+mod topk_io;
 
 #[derive(Debug, Clone)]
 struct ByteRangeDescriptor {
     range: Range<u64>,
     description: String,
 }
 
+/// Byte ranges of one column chunk:

Review Comment:
   this is infrastructure to handle tracking what byte ranges mean



##########
datafusion/core/tests/parquet/io/bloom_filter_io.rs:
##########
@@ -23,12 +41,17 @@ async fn test_parquet_bloom_filter_io_matching() {
     // produces a batch.
     assert_snapshot!(test.query("SELECT * FROM t WHERE a = 10 AND b = 
100").await, @r"
     --- Query: SELECT * FROM t WHERE a = 10 AND b = 100 (1 rows) ---
-    Read Parquet footer (offsets 0..678)
-    Read OffsetIndex for column 'a' (rg 0) (offsets 192..213)
-    Read Bloom filter for column 'a' (rg 0) (offsets 98..145)
-    Read Bloom filter for column 'b' (rg 0) (offsets 145..192)
-    Read data page for column 'a' (rg 0) (offsets 4..51)
-    Read data page for column 'b' (rg 0) (offsets 51..98)
+    Request 1

Review Comment:
   I think showing the distinct requests is what is important as that is the 
actual IO that is generated



##########
datafusion/core/tests/parquet/io/filter_pushdown_io.rs:
##########
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: See the NOTICE file distributed with this work for 
additional information regarding copyright ownership
+// SPDX-FileContributor: Licensed to the Apache Software Foundation (ASF) 
under one or more contributor license agreements
+
+//! Tests tracking I/O events with and without Parquet filter pushdown
+//! (`datafusion.execution.parquet.pushdown_filters`, see
+//! [`ParquetOptions::pushdown_filters`]).
+//!
+//! Both tests run `val = 7`, which statistics cannot prune, but which only
+//! matches a row in row group 1:
+//!
+//! ```text
+//!   row group 0        row group 1        row group 2
+//! ┌──────┬───────┐   ┌──────┬───────┐   ┌──────┬───────┐
+//! │  id  │  val  │   │  id  │  val  │   │  id  │  val  │
+//! ├──────┼───────┤   ├──────┼───────┤   ├──────┼───────┤
+//! │   1  │   0   │   │   4  │   0   │   │   7  │   0   │
+//! │   2  │   5   │   │   5  │   7 * │   │   8  │   5   │
+//! │   3  │  10   │   │   6  │  10   │   │   9  │  10   │
+//! └──────┴───────┘   └──────┴───────┘   └──────┴───────┘
+//!  val: min 0 max 10  val: min 0 max 10  val: min 0 max 10
+//!
+//!  * the only row matching `val = 7`
+//! ```
+//!
+//! [`ParquetOptions::pushdown_filters`]: 
datafusion::config::ParquetOptions::pushdown_filters
+
+use std::sync::Arc;
+
+use arrow::array::{ArrayRef, Int32Array, RecordBatch};
+use bytes::Bytes;
+use datafusion::prelude::SessionConfig;
+use insta::assert_snapshot;
+use parquet::arrow::ArrowWriter;
+use parquet::file::properties::WriterProperties;
+
+use super::ParquetIoTest;
+
+/// Without filter pushdown, the scan fetches every column of every row group
+#[tokio::test]
+async fn test_filter_pushdown_io_disabled() {
+    let test = filter_pushdown_test(false).await;
+    assert_snapshot!(test.query("SELECT * FROM t WHERE val = 7").await, @r"
+    --- Query: SELECT * FROM t WHERE val = 7 (1 rows) ---
+    Request 1
+        Read Parquet footer (offsets 0..1402)
+    Request 2
+        Read dictionary page and data page 0 for column 'id' (rg 0) (offsets 
4..51)
+        Read dictionary page and data page 0 for column 'val' (rg 0) (offsets 
51..98)
+    Request 3
+        Read dictionary page and data page 0 for column 'id' (rg 1) (offsets 
98..145)
+        Read dictionary page and data page 0 for column 'val' (rg 1) (offsets 
145..192)
+    Request 4
+        Read dictionary page and data page 0 for column 'id' (rg 2) (offsets 
192..239)
+        Read dictionary page and data page 0 for column 'val' (rg 2) (offsets 
239..286)
+    Produce batch (1 rows)
+    ");
+}
+
+/// **With** filter pushdown, the scan first fetches and evaluates the filter
+/// column (`val`) for each row group, and then fetches the remaining columns
+/// (`id`) only for the row groups with matching rows (row group 1).
+#[tokio::test]
+async fn test_filter_pushdown_io_enabled() {
+    let test = filter_pushdown_test(true).await;
+    assert_snapshot!(test.query("SELECT * FROM t WHERE val = 7").await, @r"
+    --- Query: SELECT * FROM t WHERE val = 7 (1 rows) ---
+    Request 1

Review Comment:
   I do love seeing this difference



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to