Add metric category filtering for EXPLAIN ANALYZE#21160
Open
adriangb wants to merge 4 commits intoapache:mainfrom
Open
Add metric category filtering for EXPLAIN ANALYZE#21160adriangb wants to merge 4 commits intoapache:mainfrom
adriangb wants to merge 4 commits intoapache:mainfrom
Conversation
Introduces `MetricCategory` (Rows, Bytes, Timing) so that EXPLAIN ANALYZE output can be narrowed to only deterministic metrics, which is especially useful in sqllogictest (.slt) files where timing values would otherwise require `<slt:ignore>` markers everywhere. Each `Metric` now optionally declares a category via `MetricBuilder::with_category()`. Well-known builder methods (`output_rows`, `elapsed_compute`, …) set the category automatically; custom counters/gauges default to "always included". A new session config `datafusion.explain.analyze_categories` accepts `all` (default), `none`, or a comma-separated list of `rows`, `bytes`, `timing` to control which categories appear. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
bot
added
physical-expr
Use a parquet table with multiple row groups and a TopK ORDER BY LIMIT query that triggers DynamicFilter pushdown. This makes the slt examples much more realistic — they show pruning metrics, row group statistics, and the resolved DynamicFilter predicate. Add a 'timing' category example that shows only elapsed_compute and metadata_load_time (with <slt:ignore> since they are non-deterministic). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Opt in every custom counter/gauge created by DataFusion's own operators (parquet, file_stream, joins, aggregates, topk, unnest, buffer) so that category filtering works cleanly out of the box. For example `bytes_scanned` → Bytes, `pushdown_rows_pruned` → Rows, `peak_mem_used` → Bytes, `row_replacements` → Rows, etc. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2010YOUY01
reviewed
Mar 26, 2026
| /// | ||
| /// **Non-deterministic** — varies across runs even on the same hardware. | ||
| Timing, | ||
| } |
Contributor
There was a problem hiding this comment.
Perhaps we can add another variant for all uncategorized metrics, so we can do
set datafusion.explain.analyze_category = 'rows, bytes, uncategorized' -- Only exclude `Timing` metrics category
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MetricCategoryenum (Rows,Bytes,Timing) classifying metrics by what they measure and, critically, their determinism: rows/bytes are deterministic given the same plan+data; timing varies across runs.Metriccan now declare its category viaMetricBuilder::with_category(). Well-known builder methods (output_rows,elapsed_compute,output_bytes, etc.) set the category automatically. Custom counters/gauges default to "always included".datafusion.explain.analyze_categoriesacceptsall(default),none, or comma-separatedrows,bytes,timing.analyze_level(summary/dev) which controls verbosity.Motivation
Running
EXPLAIN ANALYZEin.slttests currently requires liberal use of<slt:ignore>for every non-deterministic timing metric. With this change, a test can simply:In particular, for dynamic filters we have relatively complex integration tests that exist mostly to assert the plan shapes and state of the dynamic filters after the plan has been executed. For example #21059. With this change I think most of those can be moved to SLT tests. I've also wanted to e.g. make assertions about pruning effectiveness without having timing information included.
Test plan
explain_analyze_categoriescovering all combos (rows, none, all, rows+bytes).slttests inexplain_analyze.sltforrows,none,rows,bytes, androwswith dev levelexplain_analyzeintegration tests pass (24/24)information_schemaslt updated for new config entrycore_integrationsuite passes (918 tests)🤖 Generated with Claude Code