jorgecarleitao commented on pull request #8018: URL: https://github.com/apache/arrow/pull/8018#issuecomment-678032001
Thank you both for the help. IMO having this in pure functions make it more explicit about the functionality's dependency on "self" (via the arguments it needs). For example, it is now clear that `create_physical_expr` and `create_aggregate_expr` only need the scalar functions, not everything from "self". If we `pub` those functions, others can also re-use them without having to use the struct ExecutionContext from DataFusion. I agree with you @alamb that the create_* may end up clustered with arguments. I think that there may be a change that would place all those in a struct a-la contextState that does not need to lock (e.g. just `HashMap`), so that they can be easily reused on a recursion without locking on every recursion, both of the plan and every expression on it. @andygrove , feel free to pick either commit to push, or if you prefer the third option you mentioned, that I will work it out and push. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
