From 89218bad00068fdad90e389dc95b292a25a904dc Mon Sep 17 00:00:00 2001 From: Gabriel Musat Mestre Date: Tue, 21 Apr 2026 13:11:07 +0200 Subject: [PATCH 1/3] Improve ergonomics for ExecutionPlanMetricsSet and MetricsSet --- .../physical-expr-common/src/metrics/mod.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/datafusion/physical-expr-common/src/metrics/mod.rs b/datafusion/physical-expr-common/src/metrics/mod.rs index 8d877713cae82..dc76f59589e4d 100644 --- a/datafusion/physical-expr-common/src/metrics/mod.rs +++ b/datafusion/physical-expr-common/src/metrics/mod.rs @@ -26,6 +26,7 @@ mod value; use datafusion_common::HashMap; pub use datafusion_common::format::{MetricCategory, MetricType}; use parking_lot::Mutex; +use std::vec::IntoIter; use std::{ borrow::Cow, fmt::{self, Debug, Display}, @@ -225,6 +226,11 @@ impl MetricsSet { self.metrics.push(metric) } + /// Extends the current list of metrics with the provided ones + pub fn extend(&mut self, metrics: impl Iterator>) { + self.metrics.extend(metrics) + } + /// Returns an iterator across all metrics pub fn iter(&self) -> impl Iterator> { self.metrics.iter() @@ -432,6 +438,15 @@ impl Display for MetricsSet { } } +impl IntoIterator for MetricsSet { + type Item = Arc; + type IntoIter = IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.metrics.into_iter() + } +} + /// A set of [`Metric`]s for an individual operator. /// /// This structure is intended as a convenience for execution plan @@ -465,6 +480,14 @@ impl ExecutionPlanMetricsSet { } } +impl From for ExecutionPlanMetricsSet { + fn from(metrics: MetricsSet) -> Self { + Self { + inner: Arc::new(Mutex::new(metrics)), + } + } +} + /// `name=value` pairs identifying a metric. This concept is called various things /// in various different systems: /// From f84d26cbc79c75c41655d369773a7f022a3ecf37 Mon Sep 17 00:00:00 2001 From: Gabriel Musat Mestre Date: Wed, 22 Apr 2026 12:24:13 +0200 Subject: [PATCH 2/3] Answer to feedback --- datafusion/physical-expr-common/src/metrics/mod.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/datafusion/physical-expr-common/src/metrics/mod.rs b/datafusion/physical-expr-common/src/metrics/mod.rs index dc76f59589e4d..3d5bef590106c 100644 --- a/datafusion/physical-expr-common/src/metrics/mod.rs +++ b/datafusion/physical-expr-common/src/metrics/mod.rs @@ -26,11 +26,11 @@ mod value; use datafusion_common::HashMap; pub use datafusion_common::format::{MetricCategory, MetricType}; use parking_lot::Mutex; -use std::vec::IntoIter; use std::{ borrow::Cow, fmt::{self, Debug, Display}, sync::Arc, + vec::IntoIter, }; // public exports @@ -227,7 +227,7 @@ impl MetricsSet { } /// Extends the current list of metrics with the provided ones - pub fn extend(&mut self, metrics: impl Iterator>) { + pub fn extend(&mut self, metrics: impl IntoIterator>) { self.metrics.extend(metrics) } @@ -447,6 +447,14 @@ impl IntoIterator for MetricsSet { } } +impl FromIterator> for MetricsSet { + fn from_iter>>(iter: T) -> Self { + Self { + metrics: iter.into_iter().collect(), + } + } +} + /// A set of [`Metric`]s for an individual operator. /// /// This structure is intended as a convenience for execution plan From 0905a39fa4f2e505248d018c7d924938d43719ec Mon Sep 17 00:00:00 2001 From: Gabriel Musat Mestre Date: Wed, 22 Apr 2026 13:05:29 +0200 Subject: [PATCH 3/3] Add more iterator methods and unit tests --- .../physical-expr-common/src/metrics/mod.rs | 66 +++++++++++++++++-- 1 file changed, 61 insertions(+), 5 deletions(-) diff --git a/datafusion/physical-expr-common/src/metrics/mod.rs b/datafusion/physical-expr-common/src/metrics/mod.rs index 3d5bef590106c..285e8566359ea 100644 --- a/datafusion/physical-expr-common/src/metrics/mod.rs +++ b/datafusion/physical-expr-common/src/metrics/mod.rs @@ -226,11 +226,6 @@ impl MetricsSet { self.metrics.push(metric) } - /// Extends the current list of metrics with the provided ones - pub fn extend(&mut self, metrics: impl IntoIterator>) { - self.metrics.extend(metrics) - } - /// Returns an iterator across all metrics pub fn iter(&self) -> impl Iterator> { self.metrics.iter() @@ -447,6 +442,21 @@ impl IntoIterator for MetricsSet { } } +impl<'a> IntoIterator for &'a MetricsSet { + type Item = &'a Arc; + type IntoIter = std::slice::Iter<'a, Arc>; + + fn into_iter(self) -> Self::IntoIter { + self.metrics.iter() + } +} + +impl Extend> for MetricsSet { + fn extend>>(&mut self, iter: I) { + self.metrics.extend(iter); + } +} + impl FromIterator> for MetricsSet { fn from_iter>>(iter: T) -> Self { Self { @@ -783,6 +793,52 @@ mod tests { }; } + #[test] + fn test_extend() { + let mut metrics = MetricsSet::new(); + let m1 = Arc::new(Metric::new(MetricValue::OutputRows(Count::new()), None)); + let m2 = Arc::new(Metric::new(MetricValue::SpillCount(Count::new()), None)); + + metrics.extend([Arc::clone(&m1), Arc::clone(&m2)]); + assert_eq!(metrics.iter().count(), 2); + + let m3 = Arc::new(Metric::new(MetricValue::SpilledBytes(Count::new()), None)); + metrics.extend(std::iter::once(Arc::clone(&m3))); + assert_eq!(metrics.iter().count(), 3); + } + + #[test] + fn test_collect() { + let m1 = Arc::new(Metric::new(MetricValue::OutputRows(Count::new()), None)); + let m2 = Arc::new(Metric::new(MetricValue::SpillCount(Count::new()), None)); + + let metrics: MetricsSet = + vec![Arc::clone(&m1), Arc::clone(&m2)].into_iter().collect(); + assert_eq!(metrics.iter().count(), 2); + + let empty: MetricsSet = std::iter::empty().collect(); + assert_eq!(empty.iter().count(), 0); + } + + #[test] + fn test_into_iterator_by_ref() { + let mut metrics = MetricsSet::new(); + metrics.push(Arc::new(Metric::new( + MetricValue::OutputRows(Count::new()), + None, + ))); + metrics.push(Arc::new(Metric::new( + MetricValue::SpillCount(Count::new()), + None, + ))); + + let mut count = 0; + for _m in &metrics { + count += 1; + } + assert_eq!(count, 2); + } + #[test] fn test_sorted_for_display() { let metrics = ExecutionPlanMetricsSet::new();