This is an automated email from the ASF dual-hosted git repository.
viirya pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new bf3bd9259a Cleanup TreeNode implementations (#8672)
bf3bd9259a is described below
commit bf3bd9259aa0e93ccc2c79a606207add30d004a4
Author: Liang-Chi Hsieh <[email protected]>
AuthorDate: Mon Jan 1 00:22:18 2024 -0800
Cleanup TreeNode implementations (#8672)
* Refactor TreeNode and cleanup some implementations
* More
* More
* Fix clippy
* avoid cloning in `TreeNode.children_nodes()` implementations where
possible using `Cow`
* Remove more unnecessary apply_children
* Fix clippy
* Remove
---------
Co-authored-by: Peter Toth <[email protected]>
---
datafusion/common/src/tree_node.rs | 33 ++++----
.../src/physical_optimizer/enforce_distribution.rs | 32 ++------
.../core/src/physical_optimizer/enforce_sorting.rs | 33 ++------
.../src/physical_optimizer/pipeline_checker.rs | 18 +----
.../replace_with_order_preserving_variants.rs | 17 +---
.../core/src/physical_optimizer/sort_pushdown.rs | 19 ++---
datafusion/expr/src/tree_node/expr.rs | 93 ++++++++++------------
datafusion/expr/src/tree_node/plan.rs | 20 ++---
datafusion/physical-expr/src/sort_properties.rs | 19 ++---
datafusion/physical-expr/src/utils/mod.rs | 17 +---
10 files changed, 97 insertions(+), 204 deletions(-)
diff --git a/datafusion/common/src/tree_node.rs
b/datafusion/common/src/tree_node.rs
index 5da9636ffe..5f11c8cc1d 100644
--- a/datafusion/common/src/tree_node.rs
+++ b/datafusion/common/src/tree_node.rs
@@ -18,6 +18,7 @@
//! This module provides common traits for visiting or rewriting tree
//! data structures easily.
+use std::borrow::Cow;
use std::sync::Arc;
use crate::Result;
@@ -32,7 +33,10 @@ use crate::Result;
/// [`PhysicalExpr`]:
https://docs.rs/datafusion/latest/datafusion/physical_plan/trait.PhysicalExpr.html
/// [`LogicalPlan`]:
https://docs.rs/datafusion-expr/latest/datafusion_expr/logical_plan/enum.LogicalPlan.html
/// [`Expr`]:
https://docs.rs/datafusion-expr/latest/datafusion_expr/expr/enum.Expr.html
-pub trait TreeNode: Sized {
+pub trait TreeNode: Sized + Clone {
+ /// Returns all children of the TreeNode
+ fn children_nodes(&self) -> Vec<Cow<Self>>;
+
/// Use preorder to iterate the node on the tree so that we can
/// stop fast for some cases.
///
@@ -211,7 +215,17 @@ pub trait TreeNode: Sized {
/// Apply the closure `F` to the node's children
fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
where
- F: FnMut(&Self) -> Result<VisitRecursion>;
+ F: FnMut(&Self) -> Result<VisitRecursion>,
+ {
+ for child in self.children_nodes() {
+ match op(&child)? {
+ VisitRecursion::Continue => {}
+ VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
+ VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
+ }
+ }
+ Ok(VisitRecursion::Continue)
+ }
/// Apply transform `F` to the node's children, the transform `F` might
have a direction(Preorder or Postorder)
fn map_children<F>(self, transform: F) -> Result<Self>
@@ -342,19 +356,8 @@ pub trait DynTreeNode {
/// Blanket implementation for Arc for any tye that implements
/// [`DynTreeNode`] (such as [`Arc<dyn PhysicalExpr>`])
impl<T: DynTreeNode + ?Sized> TreeNode for Arc<T> {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in self.arc_children() {
- match op(&child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
-
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.arc_children().into_iter().map(Cow::Owned).collect()
}
fn map_children<F>(self, transform: F) -> Result<Self>
diff --git a/datafusion/core/src/physical_optimizer/enforce_distribution.rs
b/datafusion/core/src/physical_optimizer/enforce_distribution.rs
index d5a0862273..bf5aa7d022 100644
--- a/datafusion/core/src/physical_optimizer/enforce_distribution.rs
+++ b/datafusion/core/src/physical_optimizer/enforce_distribution.rs
@@ -21,6 +21,7 @@
//! according to the configuration), this rule increases partition counts in
//! the physical plan.
+use std::borrow::Cow;
use std::fmt;
use std::fmt::Formatter;
use std::sync::Arc;
@@ -47,7 +48,7 @@ use crate::physical_plan::{
};
use arrow::compute::SortOptions;
-use datafusion_common::tree_node::{Transformed, TreeNode, VisitRecursion};
+use datafusion_common::tree_node::{Transformed, TreeNode};
use datafusion_expr::logical_plan::JoinType;
use datafusion_physical_expr::expressions::{Column, NoOp};
use datafusion_physical_expr::utils::map_columns_before_projection;
@@ -1409,18 +1410,8 @@ impl DistributionContext {
}
impl TreeNode for DistributionContext {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in &self.children_nodes {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.children_nodes.iter().map(Cow::Borrowed).collect()
}
fn map_children<F>(mut self, transform: F) -> Result<Self>
@@ -1483,19 +1474,8 @@ impl PlanWithKeyRequirements {
}
impl TreeNode for PlanWithKeyRequirements {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in &self.children {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
-
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.children.iter().map(Cow::Borrowed).collect()
}
fn map_children<F>(mut self, transform: F) -> Result<Self>
diff --git a/datafusion/core/src/physical_optimizer/enforce_sorting.rs
b/datafusion/core/src/physical_optimizer/enforce_sorting.rs
index 77d04a61c5..f609ddea66 100644
--- a/datafusion/core/src/physical_optimizer/enforce_sorting.rs
+++ b/datafusion/core/src/physical_optimizer/enforce_sorting.rs
@@ -34,6 +34,7 @@
//! in the physical plan. The first sort is unnecessary since its result is
overwritten
//! by another [`SortExec`]. Therefore, this rule removes it from the physical
plan.
+use std::borrow::Cow;
use std::sync::Arc;
use crate::config::ConfigOptions;
@@ -57,7 +58,7 @@ use crate::physical_plan::{
with_new_children_if_necessary, Distribution, ExecutionPlan,
InputOrderMode,
};
-use datafusion_common::tree_node::{Transformed, TreeNode, VisitRecursion};
+use datafusion_common::tree_node::{Transformed, TreeNode};
use datafusion_common::{plan_err, DataFusionError};
use datafusion_physical_expr::{PhysicalSortExpr, PhysicalSortRequirement};
use datafusion_physical_plan::repartition::RepartitionExec;
@@ -145,19 +146,8 @@ impl PlanWithCorrespondingSort {
}
impl TreeNode for PlanWithCorrespondingSort {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in &self.children_nodes {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
-
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.children_nodes.iter().map(Cow::Borrowed).collect()
}
fn map_children<F>(mut self, transform: F) -> Result<Self>
@@ -237,19 +227,8 @@ impl PlanWithCorrespondingCoalescePartitions {
}
impl TreeNode for PlanWithCorrespondingCoalescePartitions {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in &self.children_nodes {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
-
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.children_nodes.iter().map(Cow::Borrowed).collect()
}
fn map_children<F>(mut self, transform: F) -> Result<Self>
diff --git a/datafusion/core/src/physical_optimizer/pipeline_checker.rs
b/datafusion/core/src/physical_optimizer/pipeline_checker.rs
index 9e9f647d07..e281d0e7c2 100644
--- a/datafusion/core/src/physical_optimizer/pipeline_checker.rs
+++ b/datafusion/core/src/physical_optimizer/pipeline_checker.rs
@@ -19,6 +19,7 @@
//! infinite sources, if there are any. It will reject non-runnable query plans
//! that use pipeline-breaking operators on infinite input(s).
+use std::borrow::Cow;
use std::sync::Arc;
use crate::config::ConfigOptions;
@@ -27,7 +28,7 @@ use crate::physical_optimizer::PhysicalOptimizerRule;
use crate::physical_plan::{with_new_children_if_necessary, ExecutionPlan};
use datafusion_common::config::OptimizerOptions;
-use datafusion_common::tree_node::{Transformed, TreeNode, VisitRecursion};
+use datafusion_common::tree_node::{Transformed, TreeNode};
use datafusion_common::{plan_err, DataFusionError};
use datafusion_physical_expr::intervals::utils::{check_support,
is_datatype_supported};
use datafusion_physical_plan::joins::SymmetricHashJoinExec;
@@ -91,19 +92,8 @@ impl PipelineStatePropagator {
}
impl TreeNode for PipelineStatePropagator {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in &self.children {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
-
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.children.iter().map(Cow::Borrowed).collect()
}
fn map_children<F>(mut self, transform: F) -> Result<Self>
diff --git
a/datafusion/core/src/physical_optimizer/replace_with_order_preserving_variants.rs
b/datafusion/core/src/physical_optimizer/replace_with_order_preserving_variants.rs
index 91f3d2abc6..e49b358608 100644
---
a/datafusion/core/src/physical_optimizer/replace_with_order_preserving_variants.rs
+++
b/datafusion/core/src/physical_optimizer/replace_with_order_preserving_variants.rs
@@ -19,6 +19,7 @@
//! order-preserving variants when it is helpful; either in terms of
//! performance or to accommodate unbounded streams by fixing the pipeline.
+use std::borrow::Cow;
use std::sync::Arc;
use super::utils::is_repartition;
@@ -29,7 +30,7 @@ use
crate::physical_plan::sorts::sort_preserving_merge::SortPreservingMergeExec;
use crate::physical_plan::{with_new_children_if_necessary, ExecutionPlan};
use datafusion_common::config::ConfigOptions;
-use datafusion_common::tree_node::{Transformed, TreeNode, VisitRecursion};
+use datafusion_common::tree_node::{Transformed, TreeNode};
use datafusion_physical_plan::unbounded_output;
/// For a given `plan`, this object carries the information one needs from its
@@ -104,18 +105,8 @@ impl OrderPreservationContext {
}
impl TreeNode for OrderPreservationContext {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in &self.children_nodes {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.children_nodes.iter().map(Cow::Borrowed).collect()
}
fn map_children<F>(mut self, transform: F) -> Result<Self>
diff --git a/datafusion/core/src/physical_optimizer/sort_pushdown.rs
b/datafusion/core/src/physical_optimizer/sort_pushdown.rs
index b001386301..97ca47baf0 100644
--- a/datafusion/core/src/physical_optimizer/sort_pushdown.rs
+++ b/datafusion/core/src/physical_optimizer/sort_pushdown.rs
@@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.
+use std::borrow::Cow;
use std::sync::Arc;
use crate::physical_optimizer::utils::{
@@ -28,7 +29,7 @@ use crate::physical_plan::repartition::RepartitionExec;
use crate::physical_plan::sorts::sort::SortExec;
use crate::physical_plan::{with_new_children_if_necessary, ExecutionPlan};
-use datafusion_common::tree_node::{Transformed, TreeNode, VisitRecursion};
+use datafusion_common::tree_node::{Transformed, TreeNode};
use datafusion_common::{plan_err, DataFusionError, JoinSide, Result};
use datafusion_expr::JoinType;
use datafusion_physical_expr::expressions::Column;
@@ -71,20 +72,10 @@ impl SortPushDown {
}
impl TreeNode for SortPushDown {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in &self.children_nodes {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
-
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.children_nodes.iter().map(Cow::Borrowed).collect()
}
+
fn map_children<F>(mut self, transform: F) -> Result<Self>
where
F: FnMut(Self) -> Result<Self>,
diff --git a/datafusion/expr/src/tree_node/expr.rs
b/datafusion/expr/src/tree_node/expr.rs
index 1098842716..56388be58b 100644
--- a/datafusion/expr/src/tree_node/expr.rs
+++ b/datafusion/expr/src/tree_node/expr.rs
@@ -23,17 +23,15 @@ use crate::expr::{
ScalarFunction, ScalarFunctionDefinition, Sort, TryCast, WindowFunction,
};
use crate::{Expr, GetFieldAccess};
+use std::borrow::Cow;
-use datafusion_common::tree_node::{TreeNode, VisitRecursion};
+use datafusion_common::tree_node::TreeNode;
use datafusion_common::{internal_err, DataFusionError, Result};
impl TreeNode for Expr {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- let children = match self {
- Expr::Alias(Alias{expr,..})
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ match self {
+ Expr::Alias(Alias { expr, .. })
| Expr::Not(expr)
| Expr::IsNotNull(expr)
| Expr::IsTrue(expr)
@@ -47,28 +45,26 @@ impl TreeNode for Expr {
| Expr::Cast(Cast { expr, .. })
| Expr::TryCast(TryCast { expr, .. })
| Expr::Sort(Sort { expr, .. })
- | Expr::InSubquery(InSubquery{ expr, .. }) =>
vec![expr.as_ref().clone()],
+ | Expr::InSubquery(InSubquery { expr, .. }) =>
vec![Cow::Borrowed(expr)],
Expr::GetIndexedField(GetIndexedField { expr, field }) => {
- let expr = expr.as_ref().clone();
+ let expr = Cow::Borrowed(expr.as_ref());
match field {
- GetFieldAccess::ListIndex {key} => {
- vec![key.as_ref().clone(), expr]
- },
- GetFieldAccess::ListRange {start, stop} => {
- vec![start.as_ref().clone(), stop.as_ref().clone(),
expr]
+ GetFieldAccess::ListIndex { key } => {
+ vec![Cow::Borrowed(key.as_ref()), expr]
}
- GetFieldAccess::NamedStructField {name: _name} => {
+ GetFieldAccess::ListRange { start, stop } => {
+ vec![Cow::Borrowed(start), Cow::Borrowed(stop), expr]
+ }
+ GetFieldAccess::NamedStructField { name: _name } => {
vec![expr]
}
}
}
Expr::GroupingSet(GroupingSet::Rollup(exprs))
- | Expr::GroupingSet(GroupingSet::Cube(exprs)) => exprs.clone(),
- Expr::ScalarFunction (ScalarFunction{ args, .. } ) => {
- args.clone()
- }
+ | Expr::GroupingSet(GroupingSet::Cube(exprs)) =>
exprs.iter().map(Cow::Borrowed).collect(),
+ Expr::ScalarFunction(ScalarFunction { args, .. }) =>
args.iter().map(Cow::Borrowed).collect(),
Expr::GroupingSet(GroupingSet::GroupingSets(lists_of_exprs)) => {
- lists_of_exprs.clone().into_iter().flatten().collect()
+ lists_of_exprs.iter().flatten().map(Cow::Borrowed).collect()
}
Expr::Column(_)
// Treat OuterReferenceColumn as a leaf expression
@@ -77,45 +73,49 @@ impl TreeNode for Expr {
| Expr::Literal(_)
| Expr::Exists { .. }
| Expr::ScalarSubquery(_)
- | Expr::Wildcard {..}
- | Expr::Placeholder (_) => vec![],
+ | Expr::Wildcard { .. }
+ | Expr::Placeholder(_) => vec![],
Expr::BinaryExpr(BinaryExpr { left, right, .. }) => {
- vec![left.as_ref().clone(), right.as_ref().clone()]
+ vec![Cow::Borrowed(left), Cow::Borrowed(right)]
}
Expr::Like(Like { expr, pattern, .. })
| Expr::SimilarTo(Like { expr, pattern, .. }) => {
- vec![expr.as_ref().clone(), pattern.as_ref().clone()]
+ vec![Cow::Borrowed(expr), Cow::Borrowed(pattern)]
}
Expr::Between(Between {
expr, low, high, ..
}) => vec![
- expr.as_ref().clone(),
- low.as_ref().clone(),
- high.as_ref().clone(),
+ Cow::Borrowed(expr),
+ Cow::Borrowed(low),
+ Cow::Borrowed(high),
],
Expr::Case(case) => {
let mut expr_vec = vec![];
if let Some(expr) = case.expr.as_ref() {
- expr_vec.push(expr.as_ref().clone());
+ expr_vec.push(Cow::Borrowed(expr.as_ref()));
};
for (when, then) in case.when_then_expr.iter() {
- expr_vec.push(when.as_ref().clone());
- expr_vec.push(then.as_ref().clone());
+ expr_vec.push(Cow::Borrowed(when));
+ expr_vec.push(Cow::Borrowed(then));
}
if let Some(else_expr) = case.else_expr.as_ref() {
- expr_vec.push(else_expr.as_ref().clone());
+ expr_vec.push(Cow::Borrowed(else_expr));
}
expr_vec
}
- Expr::AggregateFunction(AggregateFunction { args, filter,
order_by, .. })
- => {
- let mut expr_vec = args.clone();
+ Expr::AggregateFunction(AggregateFunction {
+ args,
+ filter,
+ order_by,
+ ..
+ }) => {
+ let mut expr_vec: Vec<_> =
args.iter().map(Cow::Borrowed).collect();
if let Some(f) = filter {
- expr_vec.push(f.as_ref().clone());
+ expr_vec.push(Cow::Borrowed(f));
}
if let Some(o) = order_by {
- expr_vec.extend(o.clone());
+
expr_vec.extend(o.iter().map(Cow::Borrowed).collect::<Vec<_>>());
}
expr_vec
@@ -126,28 +126,17 @@ impl TreeNode for Expr {
order_by,
..
}) => {
- let mut expr_vec = args.clone();
- expr_vec.extend(partition_by.clone());
- expr_vec.extend(order_by.clone());
+ let mut expr_vec: Vec<_> =
args.iter().map(Cow::Borrowed).collect();
+
expr_vec.extend(partition_by.iter().map(Cow::Borrowed).collect::<Vec<_>>());
+
expr_vec.extend(order_by.iter().map(Cow::Borrowed).collect::<Vec<_>>());
expr_vec
}
Expr::InList(InList { expr, list, .. }) => {
- let mut expr_vec = vec![];
- expr_vec.push(expr.as_ref().clone());
- expr_vec.extend(list.clone());
+ let mut expr_vec = vec![Cow::Borrowed(expr.as_ref())];
+
expr_vec.extend(list.iter().map(Cow::Borrowed).collect::<Vec<_>>());
expr_vec
}
- };
-
- for child in children.iter() {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
}
-
- Ok(VisitRecursion::Continue)
}
fn map_children<F>(self, transform: F) -> Result<Self>
diff --git a/datafusion/expr/src/tree_node/plan.rs
b/datafusion/expr/src/tree_node/plan.rs
index c7621bc178..217116530d 100644
--- a/datafusion/expr/src/tree_node/plan.rs
+++ b/datafusion/expr/src/tree_node/plan.rs
@@ -20,8 +20,13 @@
use crate::LogicalPlan;
use datafusion_common::tree_node::{TreeNodeVisitor, VisitRecursion};
use datafusion_common::{tree_node::TreeNode, Result};
+use std::borrow::Cow;
impl TreeNode for LogicalPlan {
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.inputs().into_iter().map(Cow::Borrowed).collect()
+ }
+
fn apply<F>(&self, op: &mut F) -> Result<VisitRecursion>
where
F: FnMut(&Self) -> Result<VisitRecursion>,
@@ -91,21 +96,6 @@ impl TreeNode for LogicalPlan {
visitor.post_visit(self)
}
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in self.inputs() {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
-
- Ok(VisitRecursion::Continue)
- }
-
fn map_children<F>(self, transform: F) -> Result<Self>
where
F: FnMut(Self) -> Result<Self>,
diff --git a/datafusion/physical-expr/src/sort_properties.rs
b/datafusion/physical-expr/src/sort_properties.rs
index 91238e5b04..0205f85dce 100644
--- a/datafusion/physical-expr/src/sort_properties.rs
+++ b/datafusion/physical-expr/src/sort_properties.rs
@@ -15,12 +15,13 @@
// specific language governing permissions and limitations
// under the License.
+use std::borrow::Cow;
use std::{ops::Neg, sync::Arc};
use arrow_schema::SortOptions;
use crate::PhysicalExpr;
-use datafusion_common::tree_node::{TreeNode, VisitRecursion};
+use datafusion_common::tree_node::TreeNode;
use datafusion_common::Result;
/// To propagate [`SortOptions`] across the [`PhysicalExpr`], it is
insufficient
@@ -147,7 +148,7 @@ impl Neg for SortProperties {
/// It encapsulates the orderings (`state`) associated with the expression
(`expr`), and
/// orderings of the children expressions (`children_states`). The
[`ExprOrdering`] of a parent
/// expression is determined based on the [`ExprOrdering`] states of its
children expressions.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
pub struct ExprOrdering {
pub expr: Arc<dyn PhysicalExpr>,
pub state: SortProperties,
@@ -173,18 +174,8 @@ impl ExprOrdering {
}
impl TreeNode for ExprOrdering {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in &self.children {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.children.iter().map(Cow::Borrowed).collect()
}
fn map_children<F>(mut self, transform: F) -> Result<Self>
diff --git a/datafusion/physical-expr/src/utils/mod.rs
b/datafusion/physical-expr/src/utils/mod.rs
index 87ef36558b..64a62dc782 100644
--- a/datafusion/physical-expr/src/utils/mod.rs
+++ b/datafusion/physical-expr/src/utils/mod.rs
@@ -18,7 +18,7 @@
mod guarantee;
pub use guarantee::{Guarantee, LiteralGuarantee};
-use std::borrow::Borrow;
+use std::borrow::{Borrow, Cow};
use std::collections::{HashMap, HashSet};
use std::sync::Arc;
@@ -154,19 +154,8 @@ impl<T> ExprTreeNode<T> {
}
impl<T: Clone> TreeNode for ExprTreeNode<T> {
- fn apply_children<F>(&self, op: &mut F) -> Result<VisitRecursion>
- where
- F: FnMut(&Self) -> Result<VisitRecursion>,
- {
- for child in self.children() {
- match op(child)? {
- VisitRecursion::Continue => {}
- VisitRecursion::Skip => return Ok(VisitRecursion::Continue),
- VisitRecursion::Stop => return Ok(VisitRecursion::Stop),
- }
- }
-
- Ok(VisitRecursion::Continue)
+ fn children_nodes(&self) -> Vec<Cow<Self>> {
+ self.children().iter().map(Cow::Borrowed).collect()
}
fn map_children<F>(mut self, transform: F) -> Result<Self>
