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 17e8fa75af Remove deprecated function `OptimizerRule::try_optimize`
(#15051)
17e8fa75af is described below
commit 17e8fa75af36e4911b59c3ef757523dedc026182
Author: YuNing Chen <[email protected]>
AuthorDate: Fri Mar 7 22:27:31 2025 +0800
Remove deprecated function `OptimizerRule::try_optimize` (#15051)
* chore: cleanup deprecated optimizer API since version <= 40
follow up of #15027
* chore: inlined `optimize_plan_node`
And also removed out dated comment
* chore: deprecate `supports_rewrite` function
---
datafusion/optimizer/src/lib.rs | 2 -
datafusion/optimizer/src/optimizer.rs | 56 ++++------------------
.../src/simplify_expressions/simplify_exprs.rs | 2 -
datafusion/optimizer/src/utils.rs | 40 ----------------
4 files changed, 10 insertions(+), 90 deletions(-)
diff --git a/datafusion/optimizer/src/lib.rs b/datafusion/optimizer/src/lib.rs
index 1280bf2f46..ce19856080 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 018ad8ace0..3a69bd91e7 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<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,7 +284,9 @@ 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))
}
@@ -312,35 +294,15 @@ impl TreeNodeRewriter for Rewriter<'_> {
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
diff --git a/datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs
b/datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs
index 6a56c17533..709d8f79c3 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 39f8cf285d..c734d908f6 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<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();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
