docs: revise OptimizerRule trait method descriptions#22582
Conversation
github-actions
Bot
added
the
documentation
|
|
||
| - `name` - returns the name of the rule | ||
| - `try_optimize` - takes a `LogicalPlan` and returns an `Option<LogicalPlan>`. If the rule is able to optimize the plan, it returns `Some(LogicalPlan)` with the optimized plan. If the rule is not able to optimize the plan, it returns `None`. | ||
| - `rewrite` - takes a `LogicalPlan` and returns an `Transformed<LogicalPlan>`. If the rule is able to optimize the plan, it returns `Transformed::yes` with the optimized plan. If the rule is not able to optimize the plan, it returns `Transformed::no`. |
There was a problem hiding this comment.
Thanks for updating this section. I think the rewrite description still needs one more tweak to match the actual OptimizerRule::rewrite API.
The method also receives &dyn OptimizerConfig and returns Result<Transformed<LogicalPlan>>, rather than just Transformed<LogicalPlan>. Since this PR is focused on fixing stale optimizer rule docs, it would be good to make the signature complete here so users implementing the trait have the right shape to follow.
Suggested wording: rewrite takes a LogicalPlan and &dyn OptimizerConfig, and returns Result<Transformed<LogicalPlan>>.
Small grammar note while you are here: this should be a Transformed<LogicalPlan> rather than an Transformed<LogicalPlan>.
There was a problem hiding this comment.
Sure, thanks for your suggestion. I'll make it clearer here.
|
@kosiew Please have a look at this version. |
Updated the description of the `OptimizerRule` trait methods to reflect changes in the `try_optimize` and `rewrite` methods in `Working with `Expr`s` doc.
3 participants
Updated the description of the
OptimizerRuletrait methods to reflect changes in thetry_optimizeandrewritemethods inWorking withExprsdoc.Which issue does this PR close?
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?