Uh oh!
There was an error while loading. Please reload this page.
bench: add parquet_row_filter_skip suite for the fully-matched RowFilter skip - #24328
Conversation
…ter skip Adds a sql_benchmark suite that exercises the per-row-group fully-matched RowFilter skip (apache#23696). None of the existing benchmarks trigger it: sort_tpch/tpch do not enable pushdown_filters by default (no RowFilter to skip) and ClickBench's predicates rarely fully cover a row group. The load SQL enables pushdown_filters and COPYs a clustered Parquet file (monotonic fixed-width string key, one disjoint sorted range per row group). A low-selectivity range filter then leaves the first row group straddling and the rest fully matched by statistics, so the per-row RowFilter is skipped on the fully-matched run (and the filter column is not decoded there). Run: ./bench.sh run parquet_row_filter_skip (knobs: PRED_ROWS, RG_SIZE).
There was a problem hiding this comment.
Pull request overview
Adds a new DataFusion SQL benchmark suite intended to mechanically exercise the Parquet “fully-matched row group” RowFilter skip optimization (#23696) by generating a clustered Parquet file and running a low-selectivity range predicate that makes most row groups fully matched by statistics.
Changes:
- Introduces a new
parquet_row_filter_skipbenchmark suite (suite metadata, template, load SQL, query SQL, and benchmark entry). - Generates clustered Parquet data inline via
COPY ... STORED AS PARQUETwith configurable row count (PRED_ROWS) and row-group size (RG_SIZE). - Integrates the suite into
benchmarks/bench.shfor./bench.sh run parquet_row_filter_skip.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| benchmarks/sql_benchmarks/parquet_row_filter_skip/scratch/.gitignore | Ignores generated Parquet artifacts in the suite scratch directory. |
| benchmarks/sql_benchmarks/parquet_row_filter_skip/queries/skip/q01.sql | Defines the range-filter query intended to trigger fully-matched RG skips. |
| benchmarks/sql_benchmarks/parquet_row_filter_skip/parquet_row_filter_skip.suite | Registers the new benchmark suite and documents its intent/knobs. |
| benchmarks/sql_benchmarks/parquet_row_filter_skip/parquet_row_filter_skip.benchmark.template | Adds a reusable benchmark template (load/assert/run/cleanup). |
| benchmarks/sql_benchmarks/parquet_row_filter_skip/load/clustered.sql | Generates clustered Parquet data and creates an external table over it. |
| benchmarks/sql_benchmarks/parquet_row_filter_skip/init/cleanup.sql | Drops the benchmark table between runs. |
| benchmarks/sql_benchmarks/parquet_row_filter_skip/benchmarks/skip/q01.benchmark | Wires the query+dataset into a runnable benchmark. |
| benchmarks/bench.sh | Adds parquet_row_filter_skip to the benchmark list and run dispatch. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| TO 'sql_benchmarks/parquet_row_filter_skip/scratch/clustered.parquet' | ||
| STORED AS PARQUET | ||
| OPTIONS ('max_row_group_size' '${RG_SIZE:-1000000}'); |
| predicate_eval: Conjunctive (AND) filter-evaluation micro-benchmarks; each subgroup is a different predicate pattern, to test how an | ||
| adaptive predicate-ordering system behaves across them (see https://github.com/apache/datafusion/issues/11262) | ||
| (subgroups via BENCH_SUBGROUP: costsel, cost, selectivity, cardinality, width, scale, neutral, correlation, drift) | ||
| parquet_row_filter_skip: Per-RG fully-matched RowFilter skip on Parquet (apache/datafusion#23696); clustered string key + low-selectivity | ||
| range filter + pushdown, so most row groups are fully matched and the per-row RowFilter is skipped on them | ||
| (data generated inline by the suite's load SQL; knobs: PRED_ROWS, RG_SIZE) | ||
| (toggle a system under test with its native DATAFUSION_* env var; size data with PRED_ROWS, string width with PRED_FILL) |
…p grouping - load/clustered.sql: max_row_group_size -> format.max_row_group_size so RG_SIZE actually takes effect (verified: 1M rows / RG_SIZE=100000 -> 10 row groups). - bench.sh: move the predicate_eval toggle/PRED_FILL help line back under predicate_eval instead of the new suite's description.
- Remove the invalid top-level 'name' key from the .suite file: RawSuite is #[serde(deny_unknown_fields)] and discover_suites propagates the parse error, so benchmark_runner failed for every suite, not just this one (bench.sh drives 'cargo bench --bench sql' directly and never parses .suite files, which is why runs through bench.sh still worked). Declare the PRED_ROWS/RG_SIZE knobs as suite options while there, so benchmark_runner exposes --rows/--rg-size. - Make the clustering invariant explicit: add ORDER BY to the COPY so the file is guaranteed to be written in key order instead of relying on the physical planner never repartitioning the generate_series scan. - Strengthen the asserts: exact row count, exact matching-row count (also a correctness canary for the skip itself), and a check that pushdown_filters actually took effect so config drift cannot silently turn the suite into a no-op. Settings move from the load script to an init script, matching the clickbench suites. - Add coverage beyond the single best-case query: skip/q02 (predicate matches everything, all row groups fully matched), skip/q03 (filter column projected, so the skip cannot avoid decoding it), and a control subgroup on a scrambled key where no row group is ever fully matched, measuring the overhead of the check when it cannot fire. - Document the suite and the RG_SIZE knob in sql_benchmarks/README.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1i7A2gz58sXRWiDyk9ijj
…arks-tmnv3b bench: harden and extend the parquet_row_filter_skip suite
codecov-commenter
commented
Aug 14, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #24328 +/- ##
==========================================
- Coverage 81.29% 81.17% -0.13%
==========================================
Files 1110 1109 -1 Lines 385336 388038 +2702 Branches 385336 388038 +2702 ==========================================
+ Hits 313261 314974 +1713 - Misses 53594 54517 +923 - Partials 18481 18547 +66 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Uh oh!
There was an error while loading. Please reload this page.
Which issue does this PR close?
RowFilterskip.Rationale for this change
#23696 adds a per-row-group fully-matched
RowFilterskip, but none of the existing benchmarks exercise it:sort_tpch/tpchdon't enablepushdown_filtersby default, so there is noRowFilterto skip;URL LIKE …/ equality predicates rarely make a row group's min/max fall entirely inside the satisfying range, so fully-matched RGs are rare.As raised in review (we should verify the optimization improves something mechanistically, otherwise add a benchmark first), this adds a suite that necessarily triggers the skip.
What changes are included in this PR?
A new
sql_benchmarks/parquet_row_filter_skip/suite:pushdown_filtersandCOPYs a clustered Parquet file — a fixed-width, zero-padded, monotonically increasing string key (skey) so each row group holds a disjoint, sorted range — plus 14 payload columns.skey >= '0000100000',skeynot projected). The first row group straddles the threshold; every later RG is fully matched by statistics, so the per-rowRowFilteris skipped on the fully-matched run (andskeyisn't decoded there).bench.shintegration:./bench.sh run parquet_row_filter_skip, data generated inline by the load SQL. Knobs:PRED_ROWS(row count),RG_SIZE(parquet row-group size).Are these changes tested?
Smoke-tested locally via
cargo bench --bench sql -- --test.Local A/B (main vs #23696, 10M rows / 10 RGs /
skey >= '0000100000'):row_filter_skipped_fully_matched=9;Once this lands,
run benchmark parquet_row_filter_skipwill compare any PR (e.g. #23696) againstmainin CI.Are there any user-facing changes?
No — benchmark only.