[SPARK-58296][SQL] Fix to_time returning STRING type when the format is a foldable NULL#57486
Open
LuciferYang wants to merge 1 commit into
Open
[SPARK-58296][SQL] Fix to_time returning STRING type when the format is a foldable NULL#57486LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
…is a foldable NULL `ToTime` is a `RuntimeReplaceable`, so its `dataType` comes from `replacement.dataType`. When the format argument is foldable and evaluates to NULL, the replacement fell back to `Literal(null, expr.dataType)` where `expr` is the format argument, making `to_time` report STRING instead of TIME. Change the fallback to `Literal(null, TimeType())` so it matches every other branch.
Contributor
Author
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What changes were proposed in this pull request?
ToTime(theto_timefunction) is aRuntimeReplaceable, so itsdataTypecomes fromreplacement.dataType. When the format argument is foldable and evaluates to NULL, the replacement fell back toLiteral(null, expr.dataType), whereexpris the format argument, so the whole expression reported the format's type (STRING) instead of TIME. This changes the fallback toLiteral(null, TimeType()), soto_timereportsTimeType()like all of its other branches.Why are the changes needed?
to_time(str, fmt)with a foldable NULLfmtreports the wrong type.SELECT typeof(to_time('00:12:00', null))returnsstringinstead oftime(6), and the analyzed plan carries STRING for that column, so schema inference and UNION type coercion (against a real TIME column) use the wrong type. The value is NULL either way, but the type is user-visible and wrong. The inconsistency is also internal: a non-foldable NULL format takes theinvokeParser()branch and correctly yieldstime(6), so the result type depended on whether the format happened to be foldable.Does this PR introduce any user-facing change?
Yes.
to_time(str, fmt)with a foldable NULLfmtnow has typetime(6)instead of STRING, consistent with all other forms ofto_time. The returned value (NULL) is unchanged. This is a bug fix: the wrong type has been present sinceto_timewas introduced in 4.1.0.How was this patch tested?
Added a
TimeExpressionsSuitecase assertingToTime(...).dataType === TimeType()for both an untyped and a STRING-typed foldable NULL format; it fails on the unfixed tree withStringType did not equal TimeType(6). The same case also asserts the value is still NULL viacheckEvaluation. Also added two golden queries intime.sqlwhose schema now locks intime(6).Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)