diff --git a/datafusion/optimizer/src/lib.rs b/datafusion/optimizer/src/lib.rs index 1280bf2f466e..ce198560805a 100644 --- a/datafusion/optimizer/src/lib.rs +++ b/datafusion/optimizer/src/lib.rs @@ -69,8 +69,6 @@ pub use analyzer::{Analyzer, AnalyzerRule}; pub use optimizer::{ ApplyOrder, Optimizer, OptimizerConfig, OptimizerContext, OptimizerRule, }; -#[allow(deprecated)] -pub use utils::optimize_children; pub(crate) mod join_key_set; mod plan_signature; diff --git a/datafusion/optimizer/src/optimizer.rs b/datafusion/optimizer/src/optimizer.rs index 018ad8ace0e3..3a69bd91e749 100644 --- a/datafusion/optimizer/src/optimizer.rs +++ b/datafusion/optimizer/src/optimizer.rs @@ -69,24 +69,6 @@ use crate::utils::log_plan; /// [`AnalyzerRule`]: crate::analyzer::AnalyzerRule /// [`SessionState::add_optimizer_rule`]: https://docs.rs/datafusion/latest/datafusion/execution/session_state/struct.SessionState.html#method.add_optimizer_rule pub trait OptimizerRule: Debug { - /// Try and rewrite `plan` to an optimized form, returning None if the plan - /// cannot be optimized by this rule. - /// - /// Note this API will be deprecated in the future as it requires `clone`ing - /// the input plan, which can be expensive. OptimizerRules should implement - /// [`Self::rewrite`] instead. - #[deprecated( - since = "40.0.0", - note = "please implement supports_rewrite and rewrite instead" - )] - fn try_optimize( - &self, - _plan: &LogicalPlan, - _config: &dyn OptimizerConfig, - ) -> Result> { - internal_err!("Should have called rewrite") - } - /// A human readable name for this optimizer rule fn name(&self) -> &str; @@ -99,15 +81,13 @@ pub trait OptimizerRule: Debug { } /// Does this rule support rewriting owned plans (rather than by reference)? + #[deprecated(since = "47.0.0", note = "This method is no longer used")] fn supports_rewrite(&self) -> bool { true } /// Try to rewrite `plan` to an optimized form, returning `Transformed::yes` /// if the plan was rewritten and `Transformed::no` if it was not. - /// - /// Note: this function is only called if [`Self::supports_rewrite`] returns - /// true. Otherwise the Optimizer calls [`Self::try_optimize`] fn rewrite( &self, _plan: LogicalPlan, @@ -304,7 +284,9 @@ impl TreeNodeRewriter for Rewriter<'_> { fn f_down(&mut self, node: LogicalPlan) -> Result> { if self.apply_order == ApplyOrder::TopDown { - optimize_plan_node(node, self.rule, self.config) + { + self.rule.rewrite(node, self.config) + } } else { Ok(Transformed::no(node)) } @@ -312,35 +294,15 @@ impl TreeNodeRewriter for Rewriter<'_> { fn f_up(&mut self, node: LogicalPlan) -> Result> { if self.apply_order == ApplyOrder::BottomUp { - optimize_plan_node(node, self.rule, self.config) + { + self.rule.rewrite(node, self.config) + } } else { Ok(Transformed::no(node)) } } } -/// Invokes the Optimizer rule to rewrite the LogicalPlan in place. -fn optimize_plan_node( - plan: LogicalPlan, - rule: &dyn OptimizerRule, - config: &dyn OptimizerConfig, -) -> Result> { - if rule.supports_rewrite() { - return rule.rewrite(plan, config); - } - - #[allow(deprecated)] - rule.try_optimize(&plan, config).map(|maybe_plan| { - match maybe_plan { - Some(new_plan) => { - // if the node was rewritten by the optimizer, replace the node - Transformed::yes(new_plan) - } - None => Transformed::no(plan), - } - }) -} - impl Optimizer { /// Optimizes the logical plan by applying optimizer rules, and /// invoking observer function after each call @@ -386,7 +348,9 @@ impl Optimizer { &mut Rewriter::new(apply_order, rule.as_ref(), config), ), // rule handles recursion itself - None => optimize_plan_node(new_plan, rule.as_ref(), config), + None => { + rule.rewrite(new_plan, config) + }, } .and_then(|tnr| { // run checks optimizer invariant checks, per optimizer rule applied diff --git a/datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs b/datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs index 6a56c1753328..709d8f79c3d9 100644 --- a/datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs +++ b/datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs @@ -62,8 +62,6 @@ impl OptimizerRule for SimplifyExpressions { true } - /// if supports_owned returns true, the Optimizer calls - /// [`Self::rewrite`] instead of [`Self::try_optimize`] fn rewrite( &self, plan: LogicalPlan, diff --git a/datafusion/optimizer/src/utils.rs b/datafusion/optimizer/src/utils.rs index 39f8cf285d17..c734d908f6d6 100644 --- a/datafusion/optimizer/src/utils.rs +++ b/datafusion/optimizer/src/utils.rs @@ -19,8 +19,6 @@ use std::collections::{BTreeSet, HashMap, HashSet}; -use crate::{OptimizerConfig, OptimizerRule}; - use crate::analyzer::type_coercion::TypeCoercionRewriter; use arrow::array::{new_null_array, Array, RecordBatch}; use arrow::datatypes::{DataType, Field, Schema}; @@ -38,44 +36,6 @@ use std::sync::Arc; /// as it was initially placed here and then moved elsewhere. pub use datafusion_expr::expr_rewriter::NamePreserver; -/// Convenience rule for writing optimizers: recursively invoke -/// optimize on plan's children and then return a node of the same -/// type. Useful for optimizer rules which want to leave the type -/// of plan unchanged but still apply to the children. -/// This also handles the case when the `plan` is a [`LogicalPlan::Explain`]. -/// -/// Returning `Ok(None)` indicates that the plan can't be optimized by the `optimizer`. -#[deprecated( - since = "40.0.0", - note = "please use OptimizerRule::apply_order with ApplyOrder::BottomUp instead" -)] -pub fn optimize_children( - optimizer: &impl OptimizerRule, - plan: &LogicalPlan, - config: &dyn OptimizerConfig, -) -> Result> { - let mut new_inputs = Vec::with_capacity(plan.inputs().len()); - let mut plan_is_changed = false; - for input in plan.inputs() { - if optimizer.supports_rewrite() { - let new_input = optimizer.rewrite(input.clone(), config)?; - plan_is_changed = plan_is_changed || new_input.transformed; - new_inputs.push(new_input.data); - } else { - #[allow(deprecated)] - let new_input = optimizer.try_optimize(input, config)?; - plan_is_changed = plan_is_changed || new_input.is_some(); - new_inputs.push(new_input.unwrap_or_else(|| input.clone())) - } - } - if plan_is_changed { - let exprs = plan.expressions(); - plan.with_new_exprs(exprs, new_inputs).map(Some) - } else { - Ok(None) - } -} - /// Returns true if `expr` contains all columns in `schema_cols` pub(crate) fn has_all_column_refs(expr: &Expr, schema_cols: &HashSet) -> bool { let column_refs = expr.column_refs();