Blizzara commented on issue #14123: URL: https://github.com/apache/datafusion/issues/14123#issuecomment-2703727887
> I think we should mark it deprecated Filed a PR here: https://github.com/apache/datafusion/pull/15049. Though I see there's already a release branch, how does that work? > (though we had bad luck in the past with the compiler not complaining about implementing a deprecated trait method 🤔 ) Yea, in this case implementing the method is fine, since the way the method chain is setup means calling the newer methods automatically delegates to the older, deprecated methods if needed. However custom code calling those deprecated methods is not fine, and that'll be caught by properly marking the deprecation. The earlier case I noted around the implementing was for return_type stuff, where the chain isn't setup correctly (as it cannot really be as the args changed in an incompatible way). > We also tried to document this in the (new for the first time) upgrade guide: https://datafusion.apache.org/library-user-guide/upgrading.html#use-invoke-with-args-instead-of-invoke-and-invoke-batch Yup, that's where I originally saw this and wanted to test it, thanks! :) Still even with migration guide, people may not see or understand it, so avoiding silent runtime breaks is preferrable when ever possible (I'm sure we all agree on that, just saying it for completeness 😄 ) > I think we may simply want to remove `invoke_with_batch` / other older methods ahead of deprecation schedule to avoid the confusion I'd +1 that, especially for the return_type things, but also for invokes. It'll be a compile break but that's better than any weirdness happening at runtime due to mismatching/wrong implementations. FWIW, after fixing all the invokes, all our tests seem to pass :) -- 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-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
