milenkovicm commented on PR #10354:
URL: https://github.com/apache/datafusion/pull/10354#issuecomment-2100015817
## Option 1 - Original expression as a parameter
```rust
fn simplify(
&self,
expr: Expr,
) -> Result<datafusion_common::tree_node::Transformed<Expr>> {
// we know it'll always be AggregateFunction but we have to match on
it
// which is not terribly hard but it makes api a bit odd
if let Expr::AggregateFunction(agg) = expr {
...
Ok(datafusion_common::tree_node::Transformed::yes(...))
} else {
// no re-creation of expression if not used
Ok(datafusion_common::tree_node::Transformed::no(expr))
}
}
```
default implementation is just:
```rust
fn simplify(
&self,
expr: Expr,
) -> Result<datafusion_common::tree_node::Transformed<Expr>> {
// no re-creation of expression if not used
Ok(datafusion_common::tree_node::Transformed::no(expr))
}
```
## Option 2 - AggregationFunction as a parameter
```rust
fn simplify(
&self,
agg: AggregateFunction,
) -> Result<datafusion_common::tree_node::Transformed<Expr>> {
// we know its aggregate function, no need to do extra matching, but
we need to re-create expression
// issue user can return `no` with new expression in it
Ok(datafusion_common::tree_node::Transformed::no(Expr::AggregateFunction(agg)))
}
```
## Option 3 - AggregationFunction as a parameter with parametrised
`ExprSimplifyResult`
```rust
fn simplify(
&self,
agg: AggregateFunction,
) -> Result<ExprSimplifyResult<AggregateFunction>> {
// user cant return `no` with new expression
// simplifier will re-crete expression in case original has been
returned
Ok(ExprSimplifyResult::Original(agg))
}
```
IMHO, I prefer option number 3, which would involve parametrizing
`ExprSimplifyResult`, second best is option number 2.
wdyt @jayzhan211, @alamb
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
