alamb commented on issue #13842: URL: https://github.com/apache/datafusion/issues/13842#issuecomment-2561469759
> @alamb > > Sorry for the delay🙏 But I have a quick question! I’m not too sure about this, but should the return type of `AggregateFunctionSimplification` perhaps be `Result<ExprSimplifyResult>`(or a similar enum that can indicate whether or not simplification occurred)? > > Currently, it doesn’t seem possible to skip simplification, as it always returns a simplified `Expr` (or Err). The `simplify` method can return an `Option`, but the decision to simplify seems to rely on information passed to `AggregateFunctionSimplification`, so might it be better if this method can choose whether or not to simplify..? The rationale for returning ExprSimplifyResult is so that the argument doesn't have to be cloned -- aka to avoid coping the ownership of argument is passed to the function and there needs to be some way to get the original structuer back -- 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
