You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "adriangb (via GitHub)" <gi...@apache.org> on 2026/03/25 22:14:15 UTC
[PR] Add metric category filtering for EXPLAIN ANALYZE [datafusion]
adriangb opened a new pull request, #21160:
URL: https://github.com/apache/datafusion/pull/21160
## Summary
- Adds `MetricCategory` enum (`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.
- Each `Metric` can now declare its category via `MetricBuilder::with_category()`. Well-known builder methods (`output_rows`, `elapsed_compute`, `output_bytes`, etc.) set the category automatically. Custom counters/gauges default to "always included".
- New session config `datafusion.explain.analyze_categories` accepts `all` (default), `none`, or comma-separated `rows`, `bytes`, `timing`.
- This is orthogonal to the existing `analyze_level` (summary/dev) which controls verbosity.
## Motivation
Running `EXPLAIN ANALYZE` in `.slt` tests currently requires liberal use of `<slt:ignore>` for every non-deterministic timing metric. With this change, a test can simply:
```sql
SET datafusion.explain.analyze_categories = 'rows';
EXPLAIN ANALYZE SELECT ...;
-- output contains only row-count metrics — fully deterministic, no <slt:ignore> needed
```
## Test plan
- [x] New Rust integration test `explain_analyze_categories` covering all combos (rows, none, all, rows+bytes)
- [x] New `.slt` tests in `explain_analyze.slt` for `rows`, `none`, `rows,bytes`, and `rows` with dev level
- [x] Existing `explain_analyze` integration tests pass (24/24)
- [x] Proto roundtrip test updated and passing
- [x] `information_schema` slt updated for new config entry
- [x] Full `core_integration` suite passes (918 tests)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add metric category filtering for EXPLAIN ANALYZE [datafusion]
Posted by "2010YOUY01 (via GitHub)" <gi...@apache.org>.
2010YOUY01 commented on code in PR #21160:
URL: https://github.com/apache/datafusion/pull/21160#discussion_r2998967487
##########
datafusion/common/src/format.rs:
##########
@@ -206,47 +206,227 @@ impl ConfigField for ExplainFormat {
}
}
-/// Verbosity levels controlling how `EXPLAIN ANALYZE` renders metrics
+/// Categorizes metrics so the display layer can choose the desired verbosity.
+///
+/// The `datafusion.explain.analyze_level` configuration controls which
+/// type is shown:
+/// - `"dev"` (the default): all metrics are shown.
+/// - `"summary"`: only metrics tagged as `Summary` are shown.
+///
+/// This is orthogonal to [`MetricCategory`], which filters by *what kind*
+/// of value a metric represents (rows / bytes / timing).
+///
+/// # Difference from `EXPLAIN ANALYZE VERBOSE`
+///
+/// The `VERBOSE` keyword controls whether per-partition metrics are shown
+/// (when specified) or aggregated metrics are displayed (when omitted).
+/// In contrast, `MetricType` determines which *levels* of metrics are
+/// displayed.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub enum ExplainAnalyzeLevel {
- /// Show a compact view containing high-level metrics
+pub enum MetricType {
Review Comment:
I agree the new category tags are necessary
##########
datafusion/physical-expr-common/src/metrics/builder.rs:
##########
@@ -88,6 +93,15 @@ impl<'a> MetricBuilder<'a> {
self
}
+ /// Set the semantic category for the metric being constructed.
+ ///
+ /// See [`MetricCategory`] for details on the determinism properties
+ /// of each category.
+ pub fn with_category(mut self, category: MetricCategory) -> Self {
Review Comment:
I'm wondering if some metrics may need multiple categories.
Currently `Timing/Bytes/Rows` classify metrics by output type, but we could also group them by functional aspects like `Pruning` or `Spilling`. That would enable more flexible filtering, e.g.:
```sql
set datafusion.explain.analyze_categories = ['Timing', 'Spilling']
```
to show metrics matching both tags.
If this direction is plausible, should we consider storing multiple categories (e.g. `Vec<MetricCategory>`) instead? To be more flexible for the future change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add metric category filtering for EXPLAIN ANALYZE [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #21160:
URL: https://github.com/apache/datafusion/pull/21160#discussion_r2998342077
##########
datafusion/common/src/format.rs:
##########
@@ -206,47 +206,227 @@ impl ConfigField for ExplainFormat {
}
}
-/// Verbosity levels controlling how `EXPLAIN ANALYZE` renders metrics
+/// Categorizes metrics so the display layer can choose the desired verbosity.
+///
+/// The `datafusion.explain.analyze_level` configuration controls which
+/// type is shown:
+/// - `"dev"` (the default): all metrics are shown.
+/// - `"summary"`: only metrics tagged as `Summary` are shown.
+///
+/// This is orthogonal to [`MetricCategory`], which filters by *what kind*
+/// of value a metric represents (rows / bytes / timing).
+///
+/// # Difference from `EXPLAIN ANALYZE VERBOSE`
+///
+/// The `VERBOSE` keyword controls whether per-partition metrics are shown
+/// (when specified) or aggregated metrics are displayed (when omitted).
+/// In contrast, `MetricType` determines which *levels* of metrics are
+/// displayed.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub enum ExplainAnalyzeLevel {
- /// Show a compact view containing high-level metrics
+pub enum MetricType {
Review Comment:
@2010YOUY01 this is now a bigger diff because I refactored `MetricType` into here. It removes the redundancy with `ExplainAnalyzeLevel` (they were the same enum). I really wanted to have `MetricCategory` in this module (or inside `datafusion-common` so that it's available to this module) so that we can parse the strings directly into the enum variants and avoid carrying around `Vec<String>`s. I think this also applies to `MetricType`. Now both code paths are parallel, there is less duplicate code and validation happens earlier.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add metric category filtering for EXPLAIN ANALYZE [datafusion]
Posted by "2010YOUY01 (via GitHub)" <gi...@apache.org>.
2010YOUY01 commented on code in PR #21160:
URL: https://github.com/apache/datafusion/pull/21160#discussion_r2999498026
##########
datafusion/physical-expr-common/src/metrics/builder.rs:
##########
@@ -88,6 +93,15 @@ impl<'a> MetricBuilder<'a> {
self
}
+ /// Set the semantic category for the metric being constructed.
+ ///
+ /// See [`MetricCategory`] for details on the determinism properties
+ /// of each category.
+ pub fn with_category(mut self, category: MetricCategory) -> Self {
Review Comment:
That makes sense, if it won't need breaking changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add metric category filtering for EXPLAIN ANALYZE [datafusion]
Posted by "2010YOUY01 (via GitHub)" <gi...@apache.org>.
2010YOUY01 commented on code in PR #21160:
URL: https://github.com/apache/datafusion/pull/21160#discussion_r2991955660
##########
datafusion/common/src/format.rs:
##########
@@ -206,6 +206,142 @@ impl ConfigField for ExplainFormat {
}
}
+/// Classifies a metric by what it measures.
+///
+/// This is orthogonal to [`MetricType`] (SUMMARY / DEV), which controls
+/// *verbosity*. `MetricCategory` controls *what kind of value* is shown,
+/// so that `EXPLAIN ANALYZE` output can be narrowed to only the categories
+/// that are useful in a given context.
+///
+/// For testing, the key property is **determinism**:
+/// - [`Rows`](Self::Rows) and [`Bytes`](Self::Bytes) depend on the plan
+/// and the data, so they are deterministic across runs (given the same
+/// input).
+/// - [`Timing`](Self::Timing) depends on hardware, system load, scheduling,
+/// etc., so it varies from run to run even on the same machine.
+///
+/// [`MetricCategory`] is especially useful in sqllogictest (`.slt`) files:
+/// setting `datafusion.explain.analyze_categories = 'rows'` lets a test
+/// assert on row-count metrics without sprinkling `<slt:ignore>` over every
+/// timing value.
+///
+/// Metrics that do not declare a category (the default for custom
+/// `Count` / `Gauge` metrics) are **always included** unless the config
+/// is set to `'none'`.
+///
+/// [`MetricType`]: datafusion_physical_expr_common::metrics::MetricType
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum MetricCategory {
+ /// Row counts and related dimensionless counters: `output_rows`,
+ /// `spilled_rows`, `output_batches`, pruning metrics, ratios, etc.
+ ///
+ /// Deterministic given the same plan and data.
+ Rows,
+ /// Byte measurements: `output_bytes`, `spilled_bytes`,
+ /// `current_memory_usage`, `bytes_scanned`, etc.
+ ///
+ /// Deterministic given the same plan and data.
+ Bytes,
+ /// Wall-clock durations and timestamps: `elapsed_compute`,
+ /// operator-defined `Time` metrics, `start_timestamp` /
+ /// `end_timestamp`, etc.
+ ///
+ /// **Non-deterministic** — varies across runs even on the same hardware.
+ Timing,
+}
Review Comment:
Perhaps we can add another variant for all uncategorized metrics, so we can do
```sql
set datafusion.explain.analyze_category = 'rows, bytes, uncategorized' -- Only exclude `Timing` metrics category
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add metric category filtering for EXPLAIN ANALYZE [datafusion]
Posted by "2010YOUY01 (via GitHub)" <gi...@apache.org>.
2010YOUY01 merged PR #21160:
URL: https://github.com/apache/datafusion/pull/21160
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add metric category filtering for EXPLAIN ANALYZE [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #21160:
URL: https://github.com/apache/datafusion/pull/21160#discussion_r2999123206
##########
datafusion/physical-expr-common/src/metrics/builder.rs:
##########
@@ -88,6 +93,15 @@ impl<'a> MetricBuilder<'a> {
self
}
+ /// Set the semantic category for the metric being constructed.
+ ///
+ /// See [`MetricCategory`] for details on the determinism properties
+ /// of each category.
+ pub fn with_category(mut self, category: MetricCategory) -> Self {
Review Comment:
Could we add that as a followup feature? It wouldn't be breaking I think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add metric category filtering for EXPLAIN ANALYZE [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #21160:
URL: https://github.com/apache/datafusion/pull/21160#discussion_r2998342077
##########
datafusion/common/src/format.rs:
##########
@@ -206,47 +206,227 @@ impl ConfigField for ExplainFormat {
}
}
-/// Verbosity levels controlling how `EXPLAIN ANALYZE` renders metrics
+/// Categorizes metrics so the display layer can choose the desired verbosity.
+///
+/// The `datafusion.explain.analyze_level` configuration controls which
+/// type is shown:
+/// - `"dev"` (the default): all metrics are shown.
+/// - `"summary"`: only metrics tagged as `Summary` are shown.
+///
+/// This is orthogonal to [`MetricCategory`], which filters by *what kind*
+/// of value a metric represents (rows / bytes / timing).
+///
+/// # Difference from `EXPLAIN ANALYZE VERBOSE`
+///
+/// The `VERBOSE` keyword controls whether per-partition metrics are shown
+/// (when specified) or aggregated metrics are displayed (when omitted).
+/// In contrast, `MetricType` determines which *levels* of metrics are
+/// displayed.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub enum ExplainAnalyzeLevel {
- /// Show a compact view containing high-level metrics
+pub enum MetricType {
Review Comment:
@2010YOUY01 this is now a bigger diff because I refactored `MetricType` into here. It removes the redundancy with `ExplainAnalyzeLevel` (they were the same enum). I really wanted to have `MetricCategory` in this module (or inside `datafusion-common` so that it's available to this module) so that we can parse the strings directly into the enum variants and avoid carrying around `Vec<String>`s. I think this also applies to `MetricType`. Now both code paths are parallel, there is less duplicate code and validation happens earlier.
One thing to confirm is that we are okay with having `MetricType` and `MetricCategory`. They sound somewhat redundant but I think are actually orthogonal, unless we can say something like "all timing metrics are `Dev`" in which case one is a superset of the other. I'd like your input here since you added `MetricType`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add metric category filtering for EXPLAIN ANALYZE [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #21160:
URL: https://github.com/apache/datafusion/pull/21160#discussion_r2998336701
##########
datafusion/common/src/format.rs:
##########
@@ -206,6 +206,142 @@ impl ConfigField for ExplainFormat {
}
}
+/// Classifies a metric by what it measures.
+///
+/// This is orthogonal to [`MetricType`] (SUMMARY / DEV), which controls
+/// *verbosity*. `MetricCategory` controls *what kind of value* is shown,
+/// so that `EXPLAIN ANALYZE` output can be narrowed to only the categories
+/// that are useful in a given context.
+///
+/// For testing, the key property is **determinism**:
+/// - [`Rows`](Self::Rows) and [`Bytes`](Self::Bytes) depend on the plan
+/// and the data, so they are deterministic across runs (given the same
+/// input).
+/// - [`Timing`](Self::Timing) depends on hardware, system load, scheduling,
+/// etc., so it varies from run to run even on the same machine.
+///
+/// [`MetricCategory`] is especially useful in sqllogictest (`.slt`) files:
+/// setting `datafusion.explain.analyze_categories = 'rows'` lets a test
+/// assert on row-count metrics without sprinkling `<slt:ignore>` over every
+/// timing value.
+///
+/// Metrics that do not declare a category (the default for custom
+/// `Count` / `Gauge` metrics) are **always included** unless the config
+/// is set to `'none'`.
+///
+/// [`MetricType`]: datafusion_physical_expr_common::metrics::MetricType
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum MetricCategory {
+ /// Row counts and related dimensionless counters: `output_rows`,
+ /// `spilled_rows`, `output_batches`, pruning metrics, ratios, etc.
+ ///
+ /// Deterministic given the same plan and data.
+ Rows,
+ /// Byte measurements: `output_bytes`, `spilled_bytes`,
+ /// `current_memory_usage`, `bytes_scanned`, etc.
+ ///
+ /// Deterministic given the same plan and data.
+ Bytes,
+ /// Wall-clock durations and timestamps: `elapsed_compute`,
+ /// operator-defined `Time` metrics, `start_timestamp` /
+ /// `end_timestamp`, etc.
+ ///
+ /// **Non-deterministic** — varies across runs even on the same hardware.
+ Timing,
+}
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org