You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "findepi (via GitHub)" <gi...@apache.org> on 2025/08/07 15:25:16 UTC
[PR] Deprecate ScalarUDF::is_nullable [datafusion]
findepi opened a new pull request, #17074:
URL: https://github.com/apache/datafusion/pull/17074
`ScalarUDFImpl::is_nullable` is deprecated since d3f1c9ad026e181e8cc6d1121f3223be5c26c3ae. The `ScalarUDF`'s wrapper also needs deprecation to allow eventually removing the deprecated `ScalarUDFImpl::is_nullable`.
cc @Blizzara
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Deprecate ScalarUDF::is_nullable [datafusion]
Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi commented on PR #17074:
URL: https://github.com/apache/datafusion/pull/17074#issuecomment-3166666368
> the new function does not by default delegate to the previously-internally-used function
Typically the new function delegates to the old one and the old one gets deprecated.
If that's not possible, adding such new function is a breaking API change. In such case, graceful deprecation of the old function doesn't help, and actually makes situation worse, as you said -- no compiler's help.
Given exactly this was the case with `ScalarUDFImpl::is_nullable` in https://github.com/apache/datafusion/commit/d3f1c9ad026e181e8cc6d1121f3223be5c26c3ae, I suggest removal of the `is_nullable` function. However, I don't have full context and I am not the author of that change.
@Blizzara do you want to make a follow-on PR with the removal?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Deprecate ScalarUDF::is_nullable [datafusion]
Posted by "Blizzara (via GitHub)" <gi...@apache.org>.
Blizzara commented on PR #17074:
URL: https://github.com/apache/datafusion/pull/17074#issuecomment-3166003379
Thanks! The deprecation is sure an improvement, but I wonder if overall this is_nullable -> return type from args change suffers from the same issue as the earlier ones, namely that if a user implements a custom UDF overriding the is_nullable, they don’t see the deprecation warning and so their compiler won’t likely warn them to switch to the new one. If I remember correctly, the ”best” way I could figure out for these cases where
- a new function is introduced in the (Scalar)UdfImpl interface and switched to internally, and
- the new function does not by default delegate to the previously-internally-used function,
would be to just remove the old function completely from the interface. Otherwise downstream implementors won’t get compile-time warnings, which is unfortunate. Obviously full removal has its own downsides too…
(I may be also a bit rusty in my thinking, haven’t been working on this for a while anymore now.)
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Deprecate ScalarUDF::is_nullable [datafusion]
Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi merged PR #17074:
URL: https://github.com/apache/datafusion/pull/17074
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Deprecate ScalarUDF::is_nullable [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #17074:
URL: https://github.com/apache/datafusion/pull/17074#issuecomment-3167509885
I agree that simply removing the `is_nullable` method, even though it is a breaking API, might be the least confusing course of action
now that we have an Upgrade Guide, we have a place to provide more context for these changes too so I think thier impact is less jarring / seemingly arbitrary
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org