Remove deprecated function OptimizerRule::try_optimize
#15051
Conversation
alamb
changed the title
optimizeroptimizer: remove OptimizerRule::try_optimize
There was a problem hiding this comment.
Thank you @qazxcdswe123 and @ctsk for the review
I think @ctsk 's comments and suggestions are worth considering and ideally doing before we merge this PR but I don't think they are strictly necessary and we could do them as follow on PRs as well
| /// 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 |
There was a problem hiding this comment.
I think after this PR supports_rewrite becomes a noop (doesn't do anything) so I recommend we also mark it as deprecated as well (deprecated as of 47.0.0)
There was a problem hiding this comment.
Done
I also marked deprecated supports_rewrite in this PR as it's only a one line change
datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs
Outdated
Show resolved
Hide resolved
alamb
changed the title
optimizer: remove OptimizerRule::try_optimizeOptimizerRule::try_optimize
qazxcdswe123
force-pushed
the
cleanup-optimizer
branch
2 times, most recently
from
c50f52e to
9870236
Compare
And also removed out dated comment
qazxcdswe123
force-pushed
the
cleanup-optimizer
branch
from
b0c5f2a to
3f2428b
Compare
There was a problem hiding this comment.
Thank you @qazxcdswe123 and @comphead -- this is a very nice improcement
| None | ||
| } | ||
|
|
||
| /// Does this rule support rewriting owned plans (rather than by reference)? |
| fn f_down(&mut self, node: LogicalPlan) -> Result<Transformed<LogicalPlan>> { | ||
| if self.apply_order == ApplyOrder::TopDown { | ||
| optimize_plan_node(node, self.rule, self.config) | ||
| { |
There was a problem hiding this comment.
minor is that it is strange to have this second level of nesting braces -- we can remove it as a follow on PR I think
* 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
3 participants
Follow up for #15027
optimizeroptimize_plan_nodeI've also noticed that, all
supports_rewritereturn true and not used anywhere, should we also remove it?