adriangb commented on PR #21563: URL: https://github.com/apache/datafusion/pull/21563#issuecomment-4232743989
@kosiew one thought that occurs to me - is there some sense in leaving the symbol around for another release marked as deprecated? Thinking through it there's basically two uses of this: 1. Constructing it (adding a cast, etc.). This use we need to get rid of immediately: we no longer handle this expression in various code paths. So arguably we should immediately remove constructors, etc otherwise users will end up with confusing cast errors in weird places. 2. Matching on it (like we used to do in various code paths). We could in theory leave the struct around and it would be, in theory, less churn for users. But given that the solution is either "delete the block of code" or "replace with `CastExpr`" it's really not much more work to just fix the issue than it is to add an allow for the deprecation lint. So with that said I think it makes sense to just remove this completely as this PR does. It's helped by the fact that it has only been around for 1 or 2 releases. But I do want to maybe get an okay from @comphead before we merge knowing that they've dealt with a lot of issues w.r.t. `PhysicalExprAdapter` and struct casting in particular w/ 52.0.0. -- 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]
