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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
