Fix/spark next day nullability #19340
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
harshchawra
force-pushed
the
fix/spark-next_day-nullability
branch
from
89d6070 to
e3f2b2a
Compare
Author
|
Cleaned up commit history to a single logical change. |
martin-g
reviewed
Dec 16, 2025
| }) | ||
| .unwrap(); | ||
|
|
||
| assert!(field.is_nullable()); |
Member
There was a problem hiding this comment.
Please also add tests for non-None scalar arguments and for invalid scalar arguments.
Author
There was a problem hiding this comment.
I've added tests for non-None scalar arguments as well as invalid scalar arguments.
harshchawra
force-pushed
the
fix/spark-next_day-nullability
branch
2 times, most recently
from
858cd40 to
cf214a4
Compare
martin-g
approved these changes
Dec 17, 2025
| // returns NULL instead of an error for a malformed dayOfWeek. | ||
| None | ||
| } | ||
| let day_of_week = match day_of_week.trim().to_uppercase().as_str() { |
Member
There was a problem hiding this comment.
This could also be optimized to use str::eq_ignore_ascii_case() as below
Author
There was a problem hiding this comment.
Updated implementation to use eq_ignore_ascii_case().
harshchawra
force-pushed
the
fix/spark-next_day-nullability
branch
from
cf214a4 to
a001f77
Compare
Jefffrey
reviewed
Dec 24, 2025
Contributor
Jefffrey
left a comment
Jefffrey
left a comment
There was a problem hiding this comment.
Spark seems to define NextDay as always nullable:
Contributor
|
Looks like we already have |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
next_dayneed to have custom nullability #19156Rationale for this change
The Spark
next_dayUDF can return NULL for malformed day_of_week values even when all input arguments are non-null.However, the existing implementation inferred a non-nullable return type. This happened because the UDF implemented only
return_type(), which returns aDataTypebut does not propagate nullability information.DataFusion requires UDFs to implement
return_field_from_args()when return nullability depends on input fields or runtime validation.As a result:
next_day(non_nullable_date, non_nullable_string)could still produce NULL at runtimeday_of_weekvalues yield NULLThis PR corrects the nullability inference to accurately model Spark behavior.
What changes are included in this PR?
Implemented return_field_from_args() for the Spark next_day UDF
Date32Updated
return_type()to return an error, per DataFusion API guidelines when overriding nullabilityAdded unit tests verifying:
Are these changes tested?
Yes.
This PR includes new unit tests that validate:
Correct nullability inference
return_field_from_args()Are there any user-facing changes?
Yes, but they are correctness fixes, not breaking changes.
next_dayUDF now correctly reports nullable output schemas