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 65e281a85b Improve UserDefinedLogicalNodeCore::from_template API to
return Result (#10597)
65e281a85b is described below
commit 65e281a85bb656a0dd5eb4c213920b0d96620f87
Author: 张林伟 <[email protected]>
AuthorDate: Wed May 22 05:52:23 2024 +0800
Improve UserDefinedLogicalNodeCore::from_template API to return Result
(#10597)
Co-authored-by: Andrew Lamb <[email protected]>
---
datafusion/core/src/physical_planner.rs | 6 ++++-
.../core/tests/user_defined/user_defined_plan.rs | 14 ++++++----
datafusion/expr/src/logical_plan/extension.rs | 30 ++++++++++++----------
datafusion/optimizer/src/analyzer/subquery.rs | 10 +++++---
.../optimizer/src/optimize_projections/mod.rs | 30 ++++++++++++++--------
datafusion/optimizer/src/push_down_filter.rs | 12 ++++++---
datafusion/optimizer/src/test/user_defined.rs | 12 ++++++---
.../proto/tests/cases/roundtrip_logical_plan.rs | 14 ++++++----
8 files changed, 82 insertions(+), 46 deletions(-)
diff --git a/datafusion/core/src/physical_planner.rs
b/datafusion/core/src/physical_planner.rs
index 597a03a52f..090b1d59d9 100644
--- a/datafusion/core/src/physical_planner.rs
+++ b/datafusion/core/src/physical_planner.rs
@@ -2819,7 +2819,11 @@ mod tests {
write!(f, "NoOp")
}
- fn from_template(&self, _exprs: &[Expr], _inputs: &[LogicalPlan]) ->
Self {
+ fn with_exprs_and_inputs(
+ &self,
+ _exprs: Vec<Expr>,
+ _inputs: Vec<LogicalPlan>,
+ ) -> Result<Self> {
unimplemented!("NoOp");
}
}
diff --git a/datafusion/core/tests/user_defined/user_defined_plan.rs
b/datafusion/core/tests/user_defined/user_defined_plan.rs
index 2c12e108bb..54dcffe35f 100644
--- a/datafusion/core/tests/user_defined/user_defined_plan.rs
+++ b/datafusion/core/tests/user_defined/user_defined_plan.rs
@@ -365,14 +365,18 @@ impl UserDefinedLogicalNodeCore for TopKPlanNode {
write!(f, "TopK: k={}", self.k)
}
- fn from_template(&self, exprs: &[Expr], inputs: &[LogicalPlan]) -> Self {
+ fn with_exprs_and_inputs(
+ &self,
+ mut exprs: Vec<Expr>,
+ mut inputs: Vec<LogicalPlan>,
+ ) -> Result<Self> {
assert_eq!(inputs.len(), 1, "input size inconsistent");
assert_eq!(exprs.len(), 1, "expression size inconsistent");
- Self {
+ Ok(Self {
k: self.k,
- input: inputs[0].clone(),
- expr: exprs[0].clone(),
- }
+ input: inputs.swap_remove(0),
+ expr: exprs.swap_remove(0),
+ })
}
}
diff --git a/datafusion/expr/src/logical_plan/extension.rs
b/datafusion/expr/src/logical_plan/extension.rs
index 918e290ee4..2f581c1928 100644
--- a/datafusion/expr/src/logical_plan/extension.rs
+++ b/datafusion/expr/src/logical_plan/extension.rs
@@ -215,7 +215,7 @@ impl Eq for dyn UserDefinedLogicalNode {}
/// [user_defined_plan.rs](../../tests/user_defined_plan.rs) for an
/// example of how to use this extension API.
pub trait UserDefinedLogicalNodeCore:
- fmt::Debug + Eq + Hash + Send + Sync + 'static
+ fmt::Debug + Eq + Hash + Sized + Send + Sync + 'static
{
/// Return the plan's name.
fn name(&self) -> &str;
@@ -248,23 +248,27 @@ pub trait UserDefinedLogicalNodeCore:
/// For example: `TopK: k=10`
fn fmt_for_explain(&self, f: &mut fmt::Formatter) -> fmt::Result;
- /// Create a new `ExtensionPlanNode` with the specified children
+ #[deprecated(since = "39.0.0", note = "use with_exprs_and_inputs instead")]
+ #[allow(clippy::wrong_self_convention)]
+ fn from_template(&self, exprs: &[Expr], inputs: &[LogicalPlan]) -> Self {
+ self.with_exprs_and_inputs(exprs.to_vec(), inputs.to_vec())
+ .unwrap()
+ }
+
+ /// Create a new `UserDefinedLogicalNode` with the specified children
/// and expressions. This function is used during optimization
/// when the plan is being rewritten and a new instance of the
- /// `ExtensionPlanNode` must be created.
+ /// `UserDefinedLogicalNode` must be created.
///
/// Note that exprs and inputs are in the same order as the result
/// of self.inputs and self.exprs.
///
- /// So, `self.from_template(exprs, ..).expressions() == exprs
- //
- // TODO(clippy): This should probably be renamed to use a `with_*` prefix.
Something
- // like `with_template`, or `with_exprs_and_inputs`.
- //
- // Also, I think `ExtensionPlanNode` has been renamed to
`UserDefinedLogicalNode`
- // but the doc comments have not been updated.
- #[allow(clippy::wrong_self_convention)]
- fn from_template(&self, exprs: &[Expr], inputs: &[LogicalPlan]) -> Self;
+ /// So, `self.with_exprs_and_inputs(exprs, ..).expressions() == exprs
+ fn with_exprs_and_inputs(
+ &self,
+ exprs: Vec<Expr>,
+ inputs: Vec<LogicalPlan>,
+ ) -> Result<Self>;
/// Returns the necessary input columns for this node required to compute
/// the columns in the output schema
@@ -321,7 +325,7 @@ impl<T: UserDefinedLogicalNodeCore> UserDefinedLogicalNode
for T {
exprs: Vec<Expr>,
inputs: Vec<LogicalPlan>,
) -> Result<Arc<dyn UserDefinedLogicalNode>> {
- Ok(Arc::new(self.from_template(&exprs, &inputs)))
+ Ok(Arc::new(self.with_exprs_and_inputs(exprs, inputs)?))
}
fn necessary_children_exprs(
diff --git a/datafusion/optimizer/src/analyzer/subquery.rs
b/datafusion/optimizer/src/analyzer/subquery.rs
index b46516017a..444ee94c42 100644
--- a/datafusion/optimizer/src/analyzer/subquery.rs
+++ b/datafusion/optimizer/src/analyzer/subquery.rs
@@ -370,10 +370,14 @@ mod test {
write!(f, "MockUserDefinedLogicalPlan")
}
- fn from_template(&self, _exprs: &[Expr], _inputs: &[LogicalPlan]) ->
Self {
- Self {
+ fn with_exprs_and_inputs(
+ &self,
+ _exprs: Vec<Expr>,
+ _inputs: Vec<LogicalPlan>,
+ ) -> Result<Self> {
+ Ok(Self {
empty_schema: self.empty_schema.clone(),
- }
+ })
}
}
diff --git a/datafusion/optimizer/src/optimize_projections/mod.rs
b/datafusion/optimizer/src/optimize_projections/mod.rs
index a13ae4bd41..49b52aa53a 100644
--- a/datafusion/optimizer/src/optimize_projections/mod.rs
+++ b/datafusion/optimizer/src/optimize_projections/mod.rs
@@ -870,12 +870,16 @@ mod tests {
write!(f, "NoOpUserDefined")
}
- fn from_template(&self, exprs: &[Expr], inputs: &[LogicalPlan]) ->
Self {
- Self {
- exprs: exprs.to_vec(),
- input: Arc::new(inputs[0].clone()),
+ fn with_exprs_and_inputs(
+ &self,
+ exprs: Vec<Expr>,
+ mut inputs: Vec<LogicalPlan>,
+ ) -> Result<Self> {
+ Ok(Self {
+ exprs,
+ input: Arc::new(inputs.swap_remove(0)),
schema: self.schema.clone(),
- }
+ })
}
fn necessary_children_exprs(
@@ -932,14 +936,18 @@ mod tests {
write!(f, "UserDefinedCrossJoin")
}
- fn from_template(&self, exprs: &[Expr], inputs: &[LogicalPlan]) ->
Self {
+ fn with_exprs_and_inputs(
+ &self,
+ exprs: Vec<Expr>,
+ mut inputs: Vec<LogicalPlan>,
+ ) -> Result<Self> {
assert_eq!(inputs.len(), 2);
- Self {
- exprs: exprs.to_vec(),
- left_child: Arc::new(inputs[0].clone()),
- right_child: Arc::new(inputs[1].clone()),
+ Ok(Self {
+ exprs,
+ left_child: Arc::new(inputs.remove(0)),
+ right_child: Arc::new(inputs.remove(0)),
schema: self.schema.clone(),
- }
+ })
}
fn necessary_children_exprs(
diff --git a/datafusion/optimizer/src/push_down_filter.rs
b/datafusion/optimizer/src/push_down_filter.rs
index ad9294dd6a..e56bfd051f 100644
--- a/datafusion/optimizer/src/push_down_filter.rs
+++ b/datafusion/optimizer/src/push_down_filter.rs
@@ -1364,11 +1364,15 @@ mod tests {
write!(f, "NoopPlan")
}
- fn from_template(&self, _exprs: &[Expr], inputs: &[LogicalPlan]) ->
Self {
- Self {
- input: inputs.to_vec(),
+ fn with_exprs_and_inputs(
+ &self,
+ _exprs: Vec<Expr>,
+ inputs: Vec<LogicalPlan>,
+ ) -> Result<Self> {
+ Ok(Self {
+ input: inputs,
schema: Arc::clone(&self.schema),
- }
+ })
}
}
diff --git a/datafusion/optimizer/src/test/user_defined.rs
b/datafusion/optimizer/src/test/user_defined.rs
index c60342fa00..d040fa2bae 100644
--- a/datafusion/optimizer/src/test/user_defined.rs
+++ b/datafusion/optimizer/src/test/user_defined.rs
@@ -65,11 +65,15 @@ impl UserDefinedLogicalNodeCore for TestUserDefinedPlanNode
{
write!(f, "TestUserDefined")
}
- fn from_template(&self, exprs: &[Expr], inputs: &[LogicalPlan]) -> Self {
+ fn with_exprs_and_inputs(
+ &self,
+ exprs: Vec<Expr>,
+ mut inputs: Vec<LogicalPlan>,
+ ) -> datafusion_common::Result<Self> {
assert_eq!(inputs.len(), 1, "input size inconsistent");
assert_eq!(exprs.len(), 0, "expression size inconsistent");
- Self {
- input: inputs[0].clone(),
- }
+ Ok(Self {
+ input: inputs.swap_remove(0),
+ })
}
}
diff --git a/datafusion/proto/tests/cases/roundtrip_logical_plan.rs
b/datafusion/proto/tests/cases/roundtrip_logical_plan.rs
index 472d64905b..6e819ef5bf 100644
--- a/datafusion/proto/tests/cases/roundtrip_logical_plan.rs
+++ b/datafusion/proto/tests/cases/roundtrip_logical_plan.rs
@@ -728,14 +728,18 @@ impl UserDefinedLogicalNodeCore for TopKPlanNode {
write!(f, "TopK: k={}", self.k)
}
- fn from_template(&self, exprs: &[Expr], inputs: &[LogicalPlan]) -> Self {
+ fn with_exprs_and_inputs(
+ &self,
+ mut exprs: Vec<Expr>,
+ mut inputs: Vec<LogicalPlan>,
+ ) -> Result<Self> {
assert_eq!(inputs.len(), 1, "input size inconsistent");
assert_eq!(exprs.len(), 1, "expression size inconsistent");
- Self {
+ Ok(Self {
k: self.k,
- input: inputs[0].clone(),
- expr: exprs[0].clone(),
- }
+ input: inputs.swap_remove(0),
+ expr: exprs.swap_remove(0),
+ })
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
