alamb commented on code in PR #15735:
URL: https://github.com/apache/datafusion/pull/15735#discussion_r2049565548
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -198,6 +198,7 @@ impl<S: SimplifyInfo> ExprSimplifier<S> {
///
/// See [Self::simplify] for details and usage examples.
///
+ #[deprecated(since = "48.0.0", note = "")]
Review Comment:
You could reduce the amount of code duplication here by rewriting this
function to be in terms of `simplify_with_cycle_count`. Something like:
```rust
#[deprecated(since = "48.0.0", note = "")]
pub fn simplify_with_cycle_count(&self, expr: Expr) -> Result<(Expr,
u32)> {
let (transformed, cycle_count) =
self.simplify_with_cycle_count(expr)?;
Ok((transformed.data, cycle_count))
}
```
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -188,7 +188,7 @@ impl<S: SimplifyInfo> ExprSimplifier<S> {
/// assert_eq!(expr, b_lt_2);
/// ```
pub fn simplify(&self, expr: Expr) -> Result<Expr> {
- Ok(self.simplify_with_cycle_count(expr)?.0)
+ Ok(self.simplify_with_cycle_count_transformed(expr)?.0.data)
Review Comment:
Maybe after we merge this one in, it would be worth considering changing the
simplify API as well -- but I agree that would be good as a follow on pR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
