adriangb commented on PR #23421: URL: https://github.com/apache/datafusion/pull/23421#issuecomment-4938004252
@timsaucer I tried https://github.com/apache/datafusion/pull/23449 as an alternative, it seems the worst of all worlds: - Still need a downcast match & special casing - Introduces bridge traits / more complexity The one genuine win it has over `main` is moving code closer to the expression. But that's minor in this case. So the real choice is between this PR and doing nothing / leaving it as it is on main. I think it's actually okay to leave it as is and close this as a `wontfix` until someone comes along and says "I have an expression that wraps ScalarUDF I would like to serialize" or "I have another shape of expression that needs `SessionContext` / codecs to de-serialize itself. My guess is we'll end up with a clean implementation for expressions and execution plans but functions (including window, aggregates) will be special handled until we come up with some bigger picture refactor. So my proposal would be to close this PR, mark https://github.com/apache/datafusion/issues/22430 as `wontfix` and move onto working on execution plans (where I think there is a much more obvious win). Does that sound good to you? -- 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]
