Skip to content

[SPARK-52351][SQL] Fix T prefix detection after whitespace trimming in stringToTimestamp#55049

Open
xiaoxuandev wants to merge 1 commit intoapache:masterfrom
xiaoxuandev:fix-52351
Open

[SPARK-52351][SQL] Fix T prefix detection after whitespace trimming in stringToTimestamp#55049
xiaoxuandev wants to merge 1 commit intoapache:masterfrom
xiaoxuandev:fix-52351

Conversation

@xiaoxuandev
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Fix T prefix detection in parseTimestampString after whitespace trimming. The condition used hardcoded j == 0 instead of comparing against the trimmed start index, causing T-prefixed time strings with leading whitespace (e.g. " T23:17:50") to fail parsing.

Why are the changes needed?

Without this fix, timestamp strings like " T23:17:50" with leading whitespace before the T prefix are not parsed correctly, returning None instead of the expected timestamp.

Does this PR introduce any user-facing change?

Yes. T-prefixed time strings with leading whitespace are now parsed correctly.

How was this patch tested?

Added test cases for:

  • Leading whitespace with T prefix
  • Leading whitespace with T prefix and time zone
  • Leading whitespace with year sign (+/-)

Was this patch authored or co-authored using generative AI tooling?

Yes, co-authored with Kiro.

val parsedValue = b - '0'.toByte
if (parsedValue < 0 || parsedValue > 9) {
if (j == 0 && b == 'T') {
if (j == trimmedStart && b == 'T') {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also fix the Scaladoc above?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, thanks!

…n stringToTimestamp

### What changes were proposed in this pull request?
Fix T prefix detection in parseTimestampString after whitespace trimming. The condition used hardcoded j == 0 instead of comparing against the trimmed start index, causing T-prefixed time strings with leading whitespace (e.g. " T23:17:50") to fail parsing.

### Why are the changes needed?
Without this fix, timestamp strings like " T23:17:50" with leading whitespace before the T prefix are not parsed correctly, returning None instead of the expected timestamp.

### Does this PR introduce _any_ user-facing change?
Yes. T-prefixed time strings with leading whitespace are now parsed correctly.

### How was this patch tested?
Added test cases for:
- Leading whitespace with T prefix
- Leading whitespace with T prefix and time zone
- Leading whitespace with year sign (+/-)

### Was this patch authored or co-authored using generative AI tooling?
Yes, co-authored with Kiro.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants