This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new a19fc621d1 fix: remove the Sized requirement on ExecutionPlan::name()
(#11047)
a19fc621d1 is described below
commit a19fc621d102b52c1827ed7a9ca81848afcf2bf6
Author: Ruihang Xia <[email protected]>
AuthorDate: Sun Jun 23 18:20:19 2024 +0800
fix: remove the Sized requirement on ExecutionPlan::name() (#11047)
* fix: remove the Sized requirement on ExecutionPlan::name()
Signed-off-by: Ruihang Xia <[email protected]>
* Update datafusion/physical-plan/src/lib.rs
Co-authored-by: Andrew Lamb <[email protected]>
* add document and test
Signed-off-by: Ruihang Xia <[email protected]>
---------
Signed-off-by: Ruihang Xia <[email protected]>
Co-authored-by: Andrew Lamb <[email protected]>
---
datafusion/core/src/test/mod.rs | 4 +++
datafusion/core/src/test_util/mod.rs | 4 +++
datafusion/core/tests/custom_sources_cases/mod.rs | 4 +++
.../provider_filter_pushdown.rs | 4 +++
.../core/tests/custom_sources_cases/statistics.rs | 4 +++
.../core/tests/user_defined/user_defined_plan.rs | 4 +++
datafusion/physical-plan/src/lib.rs | 29 +++++++++++++++++-----
datafusion/physical-plan/src/test/exec.rs | 24 ++++++++++++++++++
8 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/datafusion/core/src/test/mod.rs b/datafusion/core/src/test/mod.rs
index e91f83f119..2515b8a4e0 100644
--- a/datafusion/core/src/test/mod.rs
+++ b/datafusion/core/src/test/mod.rs
@@ -399,6 +399,10 @@ impl DisplayAs for StatisticsExec {
}
impl ExecutionPlan for StatisticsExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
diff --git a/datafusion/core/src/test_util/mod.rs
b/datafusion/core/src/test_util/mod.rs
index e876cfe465..059fa8fc6d 100644
--- a/datafusion/core/src/test_util/mod.rs
+++ b/datafusion/core/src/test_util/mod.rs
@@ -285,6 +285,10 @@ impl DisplayAs for UnboundedExec {
}
impl ExecutionPlan for UnboundedExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
diff --git a/datafusion/core/tests/custom_sources_cases/mod.rs
b/datafusion/core/tests/custom_sources_cases/mod.rs
index 7901998ebd..eebc946ccb 100644
--- a/datafusion/core/tests/custom_sources_cases/mod.rs
+++ b/datafusion/core/tests/custom_sources_cases/mod.rs
@@ -140,6 +140,10 @@ impl DisplayAs for CustomExecutionPlan {
}
impl ExecutionPlan for CustomExecutionPlan {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
diff --git
a/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs
b/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs
index 068383b200..b5506b7c12 100644
--- a/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs
+++ b/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs
@@ -94,6 +94,10 @@ impl DisplayAs for CustomPlan {
}
impl ExecutionPlan for CustomPlan {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn std::any::Any {
self
}
diff --git a/datafusion/core/tests/custom_sources_cases/statistics.rs
b/datafusion/core/tests/custom_sources_cases/statistics.rs
index c7be89533f..2d42b03bfe 100644
--- a/datafusion/core/tests/custom_sources_cases/statistics.rs
+++ b/datafusion/core/tests/custom_sources_cases/statistics.rs
@@ -145,6 +145,10 @@ impl DisplayAs for StatisticsValidation {
}
impl ExecutionPlan for StatisticsValidation {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
diff --git a/datafusion/core/tests/user_defined/user_defined_plan.rs
b/datafusion/core/tests/user_defined/user_defined_plan.rs
index 13e67de3df..4b5bd3a28d 100644
--- a/datafusion/core/tests/user_defined/user_defined_plan.rs
+++ b/datafusion/core/tests/user_defined/user_defined_plan.rs
@@ -507,6 +507,10 @@ impl DisplayAs for TopKExec {
#[async_trait]
impl ExecutionPlan for TopKExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
/// Return a reference to Any that can be used for downcasting
fn as_any(&self) -> &dyn Any {
self
diff --git a/datafusion/physical-plan/src/lib.rs
b/datafusion/physical-plan/src/lib.rs
index bd77814bbb..c648547c98 100644
--- a/datafusion/physical-plan/src/lib.rs
+++ b/datafusion/physical-plan/src/lib.rs
@@ -116,12 +116,13 @@ pub mod udaf {
/// [`required_input_ordering`]: ExecutionPlan::required_input_ordering
pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync {
/// Short name for the ExecutionPlan, such as 'ParquetExec'.
- fn name(&self) -> &'static str
- where
- Self: Sized,
- {
- Self::static_name()
- }
+ ///
+ /// Implementation note: this method can just proxy to
+ /// [`static_name`](ExecutionPlan::static_name) if no special action is
+ /// needed. It doesn't provide a default implementation like that because
+ /// this method doesn't require the `Sized` constrain to allow a wilder
+ /// range of use cases.
+ fn name(&self) -> &str;
/// Short name for the ExecutionPlan, such as 'ParquetExec'.
/// Like [`name`](ExecutionPlan::name) but can be called without an
instance.
@@ -829,6 +830,10 @@ mod tests {
}
impl ExecutionPlan for EmptyExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
@@ -881,6 +886,10 @@ mod tests {
}
impl ExecutionPlan for RenamedEmptyExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn static_name() -> &'static str
where
Self: Sized,
@@ -931,6 +940,14 @@ mod tests {
assert_eq!(renamed_exec.name(), "MyRenamedEmptyExec");
assert_eq!(RenamedEmptyExec::static_name(), "MyRenamedEmptyExec");
}
+
+ /// A compilation test to ensure that the `ExecutionPlan::name()` method
can
+ /// be called from a trait object.
+ /// Related ticket: https://github.com/apache/datafusion/pull/11047
+ #[allow(dead_code)]
+ fn use_execution_plan_as_trait_object(plan: &dyn ExecutionPlan) {
+ let _ = plan.name();
+ }
}
pub mod test;
diff --git a/datafusion/physical-plan/src/test/exec.rs
b/datafusion/physical-plan/src/test/exec.rs
index d5ad9292b4..ad47a484c9 100644
--- a/datafusion/physical-plan/src/test/exec.rs
+++ b/datafusion/physical-plan/src/test/exec.rs
@@ -177,6 +177,10 @@ impl DisplayAs for MockExec {
}
impl ExecutionPlan for MockExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
@@ -335,6 +339,10 @@ impl DisplayAs for BarrierExec {
}
impl ExecutionPlan for BarrierExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
@@ -444,6 +452,10 @@ impl DisplayAs for ErrorExec {
}
impl ExecutionPlan for ErrorExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
@@ -527,6 +539,10 @@ impl DisplayAs for StatisticsExec {
}
impl ExecutionPlan for StatisticsExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
@@ -619,6 +635,10 @@ impl DisplayAs for BlockingExec {
}
impl ExecutionPlan for BlockingExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
@@ -760,6 +780,10 @@ impl DisplayAs for PanicExec {
}
impl ExecutionPlan for PanicExec {
+ fn name(&self) -> &'static str {
+ Self::static_name()
+ }
+
fn as_any(&self) -> &dyn Any {
self
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
