Re: [PR] Add a ScalarUDFImpl::simplfy() API [arrow-datafusion]

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #9304:
URL: https://github.com/apache/arrow-datafusion/pull/9304#discussion_r1509590683


##########
datafusion/expr/src/udf.rs:
##########
@@ -338,6 +354,13 @@ pub trait ScalarUDFImpl: Debug + Send + Sync {
     fn monotonicity(&self) -> Result<Option<FuncMonotonicity>> {
         Ok(None)
     }
+
+    // Do the function rewrite.
+    // 'args': The arguments of the function
+    // 'schema': The schema of the function
+    fn simplify(&self, _args: &[Expr], _schema: DFSchemaRef) -> Result<Simplified> {

Review Comment:
   I played around with this PR today.
   
   I think the core challenge is that the Simplification code uses `SimplifyContext` which purposely doesn't have a reference to `DFSchema`. This means the simplifier can't call `ScalarUDFImpl::simplify` as written.
   
   I tried several options, and here is my suggestion:
   1. Move SimplifyContext into datafusion_expr
   2. Change the signature to `fn simplify(&self, _args: &[Expr], info: &dyn SimplifyContext) -> Result<Simplified>`
   
   Then I think calling `ScalarUDFImpl::simplify` will end up being straightforward to call
   
   The one challenge I found when I tried to do this locally is that SImplifyInfo currently depends on ExecutionPropss which is in datafusion-physical-expr -- we probably have to move that structure into DataFusion common or datafusion expr as well
   
   
   



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org