You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "jkosh44 (via GitHub)" <gi...@apache.org> on 2025/02/06 18:24:25 UTC
[PR] functions: Remove NullHandling from scalar funcs [datafusion]
jkosh44 opened a new pull request, #14531:
URL: https://github.com/apache/datafusion/pull/14531
3dfce7d33c19d6e7941b58cb7e83194c066347ca added an enum to all scalar functions called `NullHandling`, with two variants: `PassThrough` and Propagate`. `PassThrough` would pass through null inputs to the function implementation. `Propagate` would cause the function to return null if any of the inputs were scalar and null, it would not do anything if an input was an array and null. Function implementors were responsible for handling null array inputs and making sure the behavior was consistent with the scalar null behavior caused by `NullHandling`.
If the function signature correctly described the accepted types, then the null array input handling would also work for null scalar inputs. However, if the function signature was `VariadicAny`, then the null array input handling would not work for null scalar inputs. The reason is that when the signature is `VariadicAny`, null inputs are not properly typed (for example `ScalarValue::Null` instead of `ScalarValue::Int64(None)`. So it turns out that `NullHandling` was only useful for compensating for non-descriptive function signatures.
Furthermore, many array functions use a signature of `VariadicAny` and reject invalid types within the function implementation. This does not work with `NullHandling::Propagate`, because any null input would skip the function implementation, which would skip the type validation. So, if a function wanted to use `NullHandling::Propagate`, then they would need to use a descriptive function signature. However, using a descriptive function signature removes the usefulness of `NullHandling::Propagate`. So as it turns out `NullHandling::Propagate` is never useful.
For all the reasons stated above, this commit removes the `NullHandling` enum.
## Which issue does this PR close?
Related to #10548
## Are these changes tested?
Yes
## Are there any user-facing changes?
No
--
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] functions: Remove NullHandling from scalar funcs [datafusion]
Posted by "jkosh44 (via GitHub)" <gi...@apache.org>.
jkosh44 commented on PR #14531:
URL: https://github.com/apache/datafusion/pull/14531#issuecomment-2640875858
Sorry for the spam of PRs related to this issue. It turns out, IMO, that the fix to the null input issue was improving the function signature and the `NullHandling` enum did not help. Also I think going forward to fix similar issues we should improve the function signatures and `NullHandling` wouldn't be helpful. The commit message has more details.
--
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] functions: Remove NullHandling from scalar funcs [datafusion]
Posted by "jkosh44 (via GitHub)" <gi...@apache.org>.
jkosh44 commented on PR #14531:
URL: https://github.com/apache/datafusion/pull/14531#issuecomment-2640948421
@jayzhan211 I'm curious if you think this is a good idea or not.
--
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] functions: Remove NullHandling from scalar funcs [datafusion]
Posted by "jkosh44 (via GitHub)" <gi...@apache.org>.
jkosh44 commented on PR #14531:
URL: https://github.com/apache/datafusion/pull/14531#issuecomment-2641147504
I also have a PoC for how we'd make the array signatures more expressive and more easily fix the null input errors: https://github.com/apache/datafusion/pull/14532
--
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] functions: Remove NullHandling from scalar funcs [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #14531:
URL: https://github.com/apache/datafusion/pull/14531
--
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] functions: Remove NullHandling from scalar funcs [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #14531:
URL: https://github.com/apache/datafusion/pull/14531#issuecomment-2643368273
Thanks @jayzhan211 and @jkosh44
--
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