Remove deprecated function OptimizerRule::try_optimize (apache#15051)

* chore: cleanup deprecated optimizer API since version <= 40

follow up of apache#15027

* chore: inlined `optimize_plan_node`

And also removed out dated comment

* chore: deprecate `supports_rewrite` function

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;

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<Option<LogicalPlan>> {
-        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,43 +284,25 @@ impl TreeNodeRewriter for Rewriter<'_> {
 
     fn f_down(&mut self, node: LogicalPlan) -> Result<Transformed<LogicalPlan>> {
         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))
         }
     }
 
     fn f_up(&mut self, node: LogicalPlan) -> Result<Transformed<LogicalPlan>> {
         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<Transformed<LogicalPlan>> {
-    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

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,

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<Option<LogicalPlan>> {
-    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<Column>) -> bool {
     let column_refs = expr.column_refs();