feat(querier): extract row-group pruning stats from DataFusion — B1 live (slice 2)

[jensholdgaard]

What

Slice 2 of the querier execution path — B1 is live. Querier::run now reports how much data the query skipped, and a test proves a selective query prunes row groups via Parquet statistics. This is the first measured evidence for the query half of the thesis: pushdown skips data, it doesn't scan it.

How

• run now builds + executes the physical plan itself (df.aggregate(count) → create_physical_plan → collect) instead of df.count() — same projection-pushdown efficiency (heavy columns never materialised), but the executed ExecutionPlan is retained so its metrics can be read.
• Walks the plan tree and maps the ParquetExec metrics into QueryStats:
  • row_groups_pruned / row_groups_scanned ← the row_groups_pruned_statistics PruningMetrics (.pruned() / .matched()).
  • bytes_read ← bytes_scanned.
• Metric names + the MetricValue::PruningMetrics { pruning_metrics } shape were confirmed empirically against datafusion 53 (dumped the live metric set, then matched the typed variant — not guessed).
• §4.6 held: only plain integers cross the public boundary; the matching on datafusion types is entirely internal.

B1 test (RFC0007.1, now live)

rfc0007_1_pushdown_prunes_row_groups: tenant "a" gets two files (one per template_id, in different hours). A template_id = 1 query must skip the template-2 file's row group (its min/max can't satisfy = 1) → asserts row_groups_pruned ≥ 1 and ≥ 1 scanned. Flipped from #[ignore] stub to live.

Verification

Workspace 333 passed / 0 failed / 47 ignored; fmt/clippy/mdbook green.

Next

RFC0007.2 (B2) — wire the latency-vs-corpus-size measurement into ourios-bench against corpus/otel-demo-v*: median template-exact latency should track result size, not corpus size. That's the headline query-thesis number.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Performance Improvements

  • Query execution now reports row-group pruning and bytes-scanned metrics, improving visibility into I/O and pruning effectiveness.

• Documentation

  • Execution-slice docs advanced to the next slice and mark row-group pruning as live and tested.

• Tests

  • Replaced an ignored stub with a live integration test that verifies row-group pruning, bytes-read reporting, and correct query result counts.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cc4202b1-b4ed-41b9-9ba0-6dd56b6e524a

📥 Commits

Reviewing files that changed from the base of the PR and between 11e37fd and 70e04ad.

📒 Files selected for processing (1)

• crates/ourios-querier/src/lib.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/ourios-querier/src/lib.rs

---

📝 Walkthrough

Walkthrough

The querier builds and executes a count aggregation physical plan, extracts the resulting row count from the collected RecordBatch, and computes QueryStats by traversing the executed physical plan to sum Parquet row-group pruning and bytes-scanned metrics. A new integration test verifies pruning occurred.

Changes

RFC0007.1 row-group pruning metrics collection

Layer / File(s)	Summary
Documentation and import setup crates/ourios-querier/src/lib.rs	Crate docs updated to "execution slice 2"; imports expanded to include Arrow/DataFusion types for physical-plan introspection (RecordBatch, Int64Array, ExecutionPlan, collect, MetricValue).
Metrics extraction helper functions crates/ourios-querier/src/lib.rs	New internal helpers extract the single aggregate count from collected RecordBatchs and recursively traverse an executed ExecutionPlan to accumulate row_groups_pruned_statistics and bytes_scanned into QueryStats; includes a unit test pinning the metric-name/value contract.
Querier::run physical-plan execution crates/ourios-querier/src/lib.rs	Replace df.count() with building/executing count(lit(1_i64)) aggregation, derive rows from the aggregate result via count_value, and populate stats by walking plan metrics.
Test updates and RFC0007.1 validation crates/ourios-querier/tests/acceptance.rs, crates/ourios-querier/tests/execution.rs	Remove the RFC0007.1 test stub and add a Tokio integration test that writes two Parquet partitions, runs a filtered query, asserts one matching row, and checks r.stats.row_groups_pruned >= 1, row_groups_scanned + row_groups_pruned > row_groups_pruned, and bytes_read > 0.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• Epic: Querier — DataFusion frontend + logs DSL (RFC 0007) #82: Implements RFC0007 querier behavior and metrics folding that this PR adds tests and metric extraction for.

Possibly related PRs

• jensholdgaard/ourios#87: Prior work that moved from df.count() toward a custom aggregation and started adding metric-aware execution changes.
• jensholdgaard/ourios#86: Introduced the Querier::run scaffold and RFC0007 test stubs that this PR implements and validates.

Poem

🐰 I hopped through plans with a careful glance,
Counting each row in a tidy dance,
Row groups pruned, bytes tallied true,
RFC0007 sings, the tests approve too! 📊

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: extracting row-group pruning stats from DataFusion with B1 test now live.
Description check	✅ Passed	The PR description comprehensively covers the What, How, B1 test details, and next steps. It includes all template sections (Summary via What/How, Related context, and verification results). Minor note: the explicit checklist boxes are not filled in, but content addresses all checklist items.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/querier-pruning-stats

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR advances the ourios-querier execution path to slice 2 by surfacing Parquet row-group pruning/IO metrics from DataFusion, and by promoting RFC0007.1 (B1) from an ignored acceptance stub into a live execution test that demonstrates statistics-based pruning.

Changes:

• Update Querier::run to execute an explicit aggregate physical plan (instead of df.count()) so the executed ExecutionPlan is retained and its metrics can be read.
• Walk the physical plan tree and map DataFusion Parquet metrics into QueryStats (row groups pruned/scanned, bytes read).
• Add a live B1 test that asserts a selective template_id = 1 query prunes at least one row group via Parquet statistics.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
crates/ourios-querier/src/lib.rs	Executes an explicit physical plan and extracts Parquet pruning + IO metrics into QueryStats.
crates/ourios-querier/tests/execution.rs	Adds a live RFC0007.1 test proving row-group pruning via Parquet statistics.
crates/ourios-querier/tests/acceptance.rs	Removes the ignored RFC0007.1 stub and points to the live execution test.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ourios-querier/src/lib.rs`:
- Around line 193-229: Add colocated unit tests in the same file beneath a
#[cfg(test)] mod that exercise scan_stats and accumulate_scan_stats: implement
minimal fake ExecutionPlan(s) (or use a TestPlan helper) that return metrics
with aggregate_by_name entries named "row_groups_pruned_statistics"
(PruningMetrics with pruned() and matched()) and "bytes_scanned" (Count),
include child plans to verify recursive aggregation, and assert the resulting
QueryStats.bytes_read, row_groups_pruned, and row_groups_scanned match expected
totals; keep tests adjacent to the functions scan_stats and
accumulate_scan_stats so metric-name/aggregation behavior is pinned locally and
resilient to DataFusion-version differences.
- Around line 184-190: count_value currently masks malformed or unexpected
aggregate output by returning 0; change it to fail closed by returning a
Result<u64, E> (e.g., Result<u64, DataFusionError> or anyhow::Error) and return
Err with a clear message when the batches array is empty, the first batch has no
rows, the first column cannot be downcast to Int64Array, or the Int64 value
cannot be converted to u64; update all call sites of count_value to handle the
Result (propagate the error or map it into the surrounding execution error) so
unexpected DataFusion shapes surface as execution failures instead of silent
zero counts.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4bbad017-c226-4015-8e64-4ba0898309ef

📥 Commits

Reviewing files that changed from the base of the PR and between b287e0c and 22b92f7.

📒 Files selected for processing (3)

• crates/ourios-querier/src/lib.rs
• crates/ourios-querier/tests/acceptance.rs
• crates/ourios-querier/tests/execution.rs

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/ourios-querier/src/lib.rs (1)

193-214: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fail closed on extra aggregate columns in count_value.

count_value documents that COUNT(*) (no grouping) returns “exactly one Int64 row”, but the current check only rejects num_columns() == 0 and will still accept > 1 columns, silently reading column(0). Tighten the guard to require exactly 1 column.

Suggested fix

-    if batch.num_rows() != 1 || batch.num_columns() == 0 {
+    if batch.num_rows() != 1 || batch.num_columns() != 1 {
         return Err(bad(format!(
-            "expected 1 row × ≥1 column, got {}×{}",
+            "expected exactly 1 row × 1 column, got {}×{}",
             batch.num_rows(),
             batch.num_columns(),
         )));
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ourios-querier/src/lib.rs` around lines 193 - 214, The code currently
allows extra aggregate columns by only rejecting zero columns; tighten the guard
to require exactly one column: replace the check using batch.num_rows() != 1 ||
batch.num_columns() == 0 with batch.num_rows() != 1 || batch.num_columns() != 1
(and update the error text to "expected 1 row × 1 column, got {}×{}"); this
ensures the subsequent column(0) downcast to Int64Array and null check for count
remain valid when implementing count_value.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/ourios-querier/src/lib.rs`:
- Around line 193-214: The code currently allows extra aggregate columns by only
rejecting zero columns; tighten the guard to require exactly one column: replace
the check using batch.num_rows() != 1 || batch.num_columns() == 0 with
batch.num_rows() != 1 || batch.num_columns() != 1 (and update the error text to
"expected 1 row × 1 column, got {}×{}"); this ensures the subsequent column(0)
downcast to Int64Array and null check for count remain valid when implementing
count_value.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b5eae36a-ab4a-4c08-ab32-0480fdf5a2d5

📥 Commits

Reviewing files that changed from the base of the PR and between f2b2236 and 11e37fd.

📒 Files selected for processing (2)

• crates/ourios-querier/src/lib.rs
• crates/ourios-querier/tests/execution.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/ourios-querier/tests/execution.rs