Skip to content

#93 : failures on parsing timestamp with timezone#95

Merged
MatloaItumeleng merged 3 commits into
masterfrom
bugfix/93-failures-parsing-timestamp-with-timezone
May 11, 2026
Merged

#93 : failures on parsing timestamp with timezone#95
MatloaItumeleng merged 3 commits into
masterfrom
bugfix/93-failures-parsing-timestamp-with-timezone

Conversation

@MatloaItumeleng
Copy link
Copy Markdown
Contributor

Release notes:

  • Bugfix parsing timestamp with timezone addressed by removing the number of single-quote characters that appear before each section's start in the pattern

closes #93

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

JaCoCo code coverage report - scala 2.12.20

Metric (instruction) Coverage Threshold Status
Overall 81.65% 80.0%
Changed Files 84.72% 80.0%
Report Coverage (O/Ch) Threshold (O/Ch) Status (O/Ch)
spark-data-standardization Jacoco Report - scala:2.12.20 81.65% / 84.72% 80.0% / 80.0% ✅/✅
File Path Coverage Threshold Status
DateTimePattern.scala 84.72% 0.0%

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes timestamp parsing for patterns that include quoted literals (e.g., ISO-8601 yyyy-MM-dd'T'HH:mm:ss.SSSXXX) by correcting how second-fraction section indexes are mapped from the pattern to the input value string, and adds regression tests for the reported failure.

Changes:

  • Update DateTimePattern second-fraction scanning to compute patternWithoutSecondFractions from pattern indexes while adjusting extracted/removal section indexes for value strings with quoted literals.
  • Add a regression test covering parsing of a timezone + milliseconds timestamp with a quoted literal ('T') in the pattern.
  • Add a DateTimePattern unit test validating fraction detection/stripping for the same quoted-literal + timezone pattern.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/main/scala/za/co/absa/standardization/time/DateTimePattern.scala Adjusts second-fraction section index handling for patterns containing quoted literals; recomputes patternWithoutSecondFractions from pattern-based sections.
src/test/scala/za/co/absa/standardization/types/parsers/DateTimeParserSuite.scala Adds regression coverage for parsing ISO-like timestamps with timezone + milliseconds and quoted literals in the pattern.
src/test/scala/za/co/absa/standardization/time/DateTimePatternSuite.scala Adds a unit test for second-fraction detection/stripping in quoted-literal + timezone patterns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@ABLL526 ABLL526 left a comment

Choose a reason for hiding this comment

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

LGTM, although the Co-Pilot made some valid suggestions that need addressing IMO.

Copy link
Copy Markdown

@salamonpavel salamonpavel left a comment

Choose a reason for hiding this comment

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

LGTM (read the code), do address Copilot's suggestions otherwise ok.

ABLL526
ABLL526 previously approved these changes May 8, 2026
Copy link
Copy Markdown
Contributor

@ABLL526 ABLL526 left a comment

Choose a reason for hiding this comment

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

LGTM, Those comments were addressed well. Good work

salamonpavel
salamonpavel previously approved these changes May 11, 2026
dk1844
dk1844 previously approved these changes May 11, 2026
Copy link
Copy Markdown
Contributor

@dk1844 dk1844 left a comment

Choose a reason for hiding this comment

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

I am generally happy with it, please just address the comments

@MatloaItumeleng MatloaItumeleng dismissed stale reviews from dk1844, salamonpavel, and ABLL526 via 0263e3d May 11, 2026 10:38
@MatloaItumeleng MatloaItumeleng merged commit eab0423 into master May 11, 2026
5 checks passed
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.

Standardization fails on parsing timestamp with timezone

5 participants