Skip to content

perf(query): skip Parquet row groups that bloom filters rule out - #204

Merged
s-prosvirnin merged 1 commit into
mainfrom
read-parquet-bloom-filter
Sep 28, 2026
Merged

s-prosvirnin merged 1 commit into
mainfrom
read-parquet-bloom-filter

Conversation

@s-prosvirnin

@s-prosvirnin s-prosvirnin commented Sep 28, 2026 •

Copy link
Copy Markdown
Member
  • New Features
    • Added row_groups_pruned_bloom_filter and bloom_filter_read_errors to the Iceberg scan in EXPLAIN ANALYZE; an unreadable bloom filter keeps its row group and is counted as a read error.
  • Performance
    • Improved Iceberg reads with = or IN filters on bloom filter columns (trace_id, span_id) by skipping row groups whose bloom filters prove the value absent, with unchanged query results.
  • Chores
    • Bumped the iceberg fork dependencies to a revision with bloom filter support.

Summary by CodeRabbit

  • New Features
    • Iceberg queries now use bloom filters to skip row groups that cannot match a query’s filters, including equality, IN, and combined trace/span ID conditions.
    • Iceberg scan metrics now report bloom-filter pruning and read errors alongside output-row counts, including when a scan ends early.

* New Features
   * Added `row_groups_pruned_bloom_filter` and `bloom_filter_read_errors` to the Iceberg scan in `EXPLAIN ANALYZE`; an unreadable bloom filter keeps its row group and is counted as a read error.
* Performance
   * Improved Iceberg reads with `=` or `IN` filters on bloom filter columns (`trace_id`, `span_id`) by skipping row groups whose bloom filters prove the value absent, with unchanged query results.
* Chores
   * Bumped the `iceberg` fork dependencies to a revision with bloom filter support.
@s-prosvirnin
s-prosvirnin requested review from a team and frisbeeman September 28, 2026 15:12
@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: a60c695d-3d6f-42cc-94f5-60db64a2db55

📥 Commits

Reviewing files that changed from the base of the PR and between 2f77765 and 1aa874c.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • Cargo.toml
  • crates/icegate-query/src/engine/provider/iceberg_scan_metrics.rs
  • crates/icegate-query/src/engine/provider/metrics.rs
  • crates/icegate-query/src/engine/provider/mod.rs
  • crates/icegate-query/src/engine/provider/scan.rs
  • crates/icegate-query/tests/flight_sql/bloom_filter.rs
  • crates/icegate-query/tests/flight_sql/harness.rs
  • crates/icegate-query/tests/flight_sql/mod.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The Iceberg scan now enables bloom-filter row-group pruning and reports reader metrics through DataFusion. New unit and Flight SQL integration tests check metric transfer, query results, pruning counts, and read errors.

Changes

Iceberg bloom-filter scan

Layer / File(s) Summary
Per-partition metric tracking
crates/icegate-query/src/engine/provider/iceberg_scan_metrics.rs
Adds per-partition output, pruning, matching, and read-error metrics. The stream wrapper transfers reader counters when the stream ends or is dropped, and unit tests cover batch counts, reader errors, early drop, and unreadable bloom filters.
Bloom-filter scan wiring
Cargo.toml, crates/icegate-query/src/engine/provider/{mod.rs,metrics.rs,scan.rs}
Updates the six Iceberg dependencies to revision dbc1452a0ecb79c043c6078257d110cb9b152b3c. The scan enables bloom-filter pruning and tracks reader metrics; the metric documentation describes the Iceberg compressed-bytes metric as unfilled by the scan.
Flight SQL test fixtures and validation
crates/icegate-query/tests/flight_sql/{harness.rs,mod.rs,bloom_filter.rs}
Allows fixture callers to supply Parquet writer properties and log IDs. Adds SQL checks for returned span IDs, expected pruning counts, and zero bloom-filter read errors.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant FlightSQLTest
  participant IcebergScan
  participant IcebergReader
  participant DataFusionMetrics
  FlightSQLTest->>IcebergScan: Run predicate query
  IcebergScan->>IcebergReader: Read planned files with bloom-filter pruning
  IcebergReader-->>IcebergScan: Return batches and ScanMetrics
  IcebergScan->>DataFusionMetrics: Record output and bloom-filter counters
  IcebergScan-->>FlightSQLTest: Return query results and EXPLAIN ANALYZE metrics
Loading

Suggested reviewers: frisbeeman

Merge Risk: ⚪ Minimal · up to 1aa87

The changed scan preserves query behavior and has coverage for pruning and unreadable-filter fallback. It is ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 1aa87

Bloom-filter pruning can affect which log rows a query sees. The available tests support correct results for the covered filters and show that an unreadable filter does not discard its row group. No security failure was established, but the reader dependency change leaves some data-visibility behavior unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The newly enabled decision affects row groups in Iceberg queries reached through the existing Flight SQL query path. The examined scan change does not add a table-selection, identity, credential, or cross-service access path; authorization outside this provider was not assessed.

Trust Boundaries and Controls

  • inferred — SQL predicate values can influence bloom-filter pruning inside the existing table scan, while the scan retains its snapshot, projection, and predicate construction. An unreadable filter is shown to keep, rather than exclude, its row group.

Resilience and Maintainability Implications

  • observed — Per-stream ownership prevents duplicate metric transfer on completion followed by drop or another poll. Read-error visibility is deferred until completion or drop if an error item is emitted first.

Hardening Proposals

  • proposed — Before relying on unchanged data visibility, compare equality- and position-delete results between the prior scan path and the direct reader at the pinned fork revision.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: skipping Parquet row groups when bloom filters rule out matching values.
Docstring Coverage ✅ Passed Docstring coverage is 85.29% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 7 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@s-prosvirnin
s-prosvirnin merged commit df68773 into main Sep 28, 2026
7 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.

2 participants