Skip to content

bench: add parquet_row_filter_skip suite for the fully-matched RowFilter skip - #24328

Merged
adriangb merged 4 commits into
apache:mainfrom
zhuqi-lucas:bench/parquet-row-filter-skip
Aug 14, 2026
Merged

bench: add parquet_row_filter_skip suite for the fully-matched RowFilter skip#24328
adriangb merged 4 commits into
apache:mainfrom
zhuqi-lucas:bench/parquet-row-filter-skip

Conversation

@zhuqi-lucas

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

#23696 adds a per-row-group fully-matched RowFilter skip, but none of the existing benchmarks exercise it:

  • sort_tpch / tpch don't enable pushdown_filters by default, so there is no RowFilter to skip;
  • ClickBench's 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:

  • The load SQL enables pushdown_filters and COPYs 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.
  • The query applies a low-selectivity range filter (skey >= '0000100000', skey not projected). The first row group straddles the threshold; every later RG is fully matched by statistics, so the per-row RowFilter is skipped on the fully-matched run (and skey isn't decoded there).
  • bench.sh integration: ./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'):

  • 9 of 10 row groups fully matchedrow_filter_skipped_fully_matched=9;
  • ~18% faster with the optimization (main ~0.143s → branch ~0.117s); an int64-key variant is ~12%.

Once this lands, run benchmark parquet_row_filter_skip will compare any PR (e.g. #23696) against main in CI.

Are there any user-facing changes?

No — benchmark only.

…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).
Copilot AI lite review requested due to automatic review settings August 13, 2026 14:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_skip benchmark suite (suite metadata, template, load SQL, query SQL, and benchmark entry).
  • Generates clustered Parquet data inline via COPY ... STORED AS PARQUET with configurable row count (PRED_ROWS) and row-group size (RG_SIZE).
  • Integrates the suite into benchmarks/bench.sh for ./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.

Comment on lines +33 to +35
TO 'sql_benchmarks/parquet_row_filter_skip/scratch/clustered.parquet'
STORED AS PARQUET
OPTIONS ('max_row_group_size' '${RG_SIZE:-1000000}');
Comment thread benchmarks/bench.sh Outdated
Comment on lines 107 to 113
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.
claude and others added 2 commits August 14, 2026 04:38
- 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

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.17%. Comparing base (9b3b518) to head (811dfb0).
⚠️ Report is 20 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@adriangb
adriangb added this pull request to the merge queue Aug 14, 2026
Merged via the queue into apache:main with commit 6adb504 Aug 14, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants