Repository navigation
[SPARK-60484][SQL] Keep the sign of CAST(string AS INTERVAL SECOND) for values between -1 and 0 - #59725
Open
ykisana wants to merge 1 commit into
Open
[SPARK-60484][SQL] Keep the sign of CAST(string AS INTERVAL SECOND) for values between -1 and 0#59725ykisana wants to merge 1 commit into
ykisana wants to merge 1 commit into
Conversation
…or values between -1 and 0
IntervalUtils.parseSecondNano took the sign of the fractional part from
the parsed whole seconds (`seconds < 0`). For an input such as "-0.5"
the whole seconds are 0, so the fraction was added as positive and
CAST('-0.5' AS INTERVAL SECOND) returned 0.5 seconds. Because Spark
formats such a value as INTERVAL '-00.5' SECOND and the JSON/CSV
readers use the same parser, round trips also flipped the sign.
Take the sign from the seconds string instead.
This branch has not been deployed
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
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?
IntervalUtils.parseSecondNanotook the sign of the fractional part from the parsed whole seconds:When casting a string to
INTERVAL SECOND, a negative input such as-0.5reaches this method as"-0.5". Its whole seconds parse to0, soseconds < 0is false and the fraction is added as a positive value. This PR takes the sign from the seconds string instead (secondsStr.startsWith("-")). For any input whose whole seconds are non-zero, the result is unchanged.The other callers of
parseSecondNano(toDTIntervalfor... TO SECONDtypes) pass unsigned seconds and apply the sign outside, so they are not affected.Why are the changes needed?
Every value between -1 and 0 seconds loses its sign when cast from a string:
Spark formats such an interval as
INTERVAL '-00.5' SECOND, and the JSON and CSV readers parseINTERVAL SECONDwith the same code. As a result, writing-0.5seconds to JSON or CSV and reading it back returns+0.5seconds.Does this PR introduce any user-facing change?
Yes, it fixes a bug. Casting a string to
INTERVAL SECONDnow keeps the sign of values between -1 and 0 seconds, as theINTERVAL '-0.5' SECONDliteral already does. JSON and CSV round trips of such values are fixed as well. The bug reproduces on 3.5.8, 4.0.0, 4.1.1 and 4.2.0.How was this patch tested?
Added
SPARK-60484: Cast string to INTERVAL SECOND keeps the sign of values in (-1, 0)toCastSuiteBase. It covers:-0.5,-00.5,-0.000001,-0.999999999, and0.5as a positive control);INTERVAL '-0.5' SECOND,INTERVAL -'0.5' SECOND,INTERVAL -'-0.5' SECOND);Without the fix, the new test fails in all three suites with
cast(-0.5 as interval second), actual: 500000, expected: -500000.All 458 tests in these four suites pass with the fix. Scalastyle (
build/sbt catalyst/scalastyle catalyst/Test/scalastyle) reports 0 errors.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
Fix, tests, PR description, and comments by Claude.
All code manually reviewed by @ykisana.