alamb commented on PR #8046: URL: https://github.com/apache/arrow-datafusion/pull/8046#issuecomment-1795967063
> Thank you, this looks great. I have several questions/suggestions: > > 1. > to support the expr_fns [encode](https://docs.rs/datafusion/latest/datafusion/logical_expr/expr_fn/fn.encode.html) and [decode](https://docs.rs/datafusion/latest/datafusion/logical_expr/expr_fn/fn.decode.html), I think we will need a Expr::ScalarFunction call or something that can take a function by name rather than fully resolved function > > Now constructing an `Expr` for built-in functions is stateless (does not require context), so it's tricky to be backwards compatible for `Expr` API, the best solution I can think of is to also support initializing a UDF `Expr` with only name string, and resolve them during logical plan optimization. Yes, I agree this approach is the best I can come up with. > 2. > Extract registration functions from SessionContext into their own trait / consolidate the function registry code rather than passing > > around a set of HahsMaps.... And make a way to actually modify them > > It's a good idea to pack 3 HashMaps for scalar/aggr/window UDFs into a new struct like `FunctionRegistry` 👍🏼 👍 Unfortunately that name is already taken :) Maybe `MemoryFunctionRegistry` 🤔 -- 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]
