Blizzara opened a new issue, #14729:
URL: https://github.com/apache/datafusion/issues/14729

   ### Describe the bug
   
   https://github.com/apache/datafusion/pull/14094 was a bit problematic for 
users with custom UDFs in that it may cause breaks at runtime.
   
   The #[deprecated()] apparently doesn't affect for people **implementing** 
this trait, only those who **call** it. 
   
   As such, if someone (like us) had implemented `return_type_from_exprs` and 
had `return_type` just panic, that'd compile fine, but at runtime rather than 
calling the `return_type_from_exprs` DF would call this new 
`return_type_from_args` with its default impl delegating to `return_type` which 
would then panic.
   
   Luckily, we had tests that caught it, but I think in this case it would have 
been better to fully remove the `return_type_from_exprs` so that users who 
override it would get compile failures, rather than runtime ones. Compile 
errors are annoying, sure, but still always better than compiling and doing 
wrong thing. And might still be good to remove it.
   
   The "correct" way would have been to have a default `return_type_from_args` 
delegate to `return_type_from_exprs`, but I'm not sure if that's possible? 
   
   A worse alternative would have been to write the chain as 
`return_type_from_exprs` -> `return_type_from_args`  -> `return_type`, and keep 
calling `return_type_from_exprs`. Not sure if that would have been possible 
either, or if the switch from `return_type_from_exprs` to 
`return_type_from_args` was necessary, and even still this wouldn't have helped 
if at some point in future someone would depend on `return_type_from_args`.
   
   ### To Reproduce
   
   _No response_
   
   ### Expected behavior
   
   _No response_
   
   ### Additional context
   
   As an aside, there are still couple places referring 
`return_type_from_exprs` as comments when they really mean the "from_args" 
variant: 
https://github.com/search?q=repo%3Aapache%2Fdatafusion+return_type_from_exprs+language%3ARust&type=code&l=Rust


-- 
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]

Reply via email to