Repository navigation
GH-54534: [C++] Fix truncation of sub-minute UTC offsets when formatting and parsing zoned timestamps - #54627
ShivanshhhG wants to merge 1 commit into
Conversation
…ormatting and parsing zoned timestamps When formatting zoned timestamps whose UTC offset includes sub-minute seconds (e.g. historical IANA zones like Africa/Monrovia prior to 1972 at UTC-00:44:30 or Europe/Paris prior to 1911), the vendored date.h library truncated the offset to whole minutes via duration_cast<minutes>, dropping the remaining seconds and emitting an inaccurate offset (e.g. -0044 instead of -004430). When parsed back, the timestamp was shifted by up to 59 seconds. This patch: 1. Updates date.h %z and %Ez formatters to format seconds when the UTC offset contains non-zero seconds (±hhmmss or ±hh:mm:ss). 2. Updates ParseTimestampISO8601 in value_parsing.h to recognize and parse [+-]HHMMSS and [+-]HH:MM:SS zone offsets. 3. Recognizes %Ez as providing a zone offset in StrptimeTimestampParser and GetZone. 4. Adds regression unit tests in value_parsing_test.cc and scalar_cast_test.cc. Closes apache#54534. Signed-off-by: SHIVANSH-ux-ys <singaser78@gmail.com>
|
Thanks for opening a pull request! This pull request has been automatically closed because you currently have 4 open pull requests, which is more than the limit of 3. Due to the increase in pull requests opened by AI bots, and in order to keep the review queue manageable, Apache Arrow limits contributors without repository access to at most 3 concurrently open pull requests. This helps make sure each pull request gets the attention it needs and that work in progress does not go stale. Once one of your other open pull requests has been merged or closed, you are welcome to reopen this one. See also: |
|
|
|
|
|
Closed stale PR #51133 so concurrent open PR count is now <= 3. Reopening this PR for review. Thank you! |
Rationale for this change
When formatting zoned timestamps whose UTC offset includes sub-minute seconds (e.g. historical IANA zones such as
Africa/Monroviaprior to 1972 at UTC-00:44:30 orEurope/Parisprior to 1911 at UTC+00:09:21), the vendoreddate.hlibrary truncated the offset to whole minutes viaduration_cast<minutes>, omitting the remaining seconds and emitting an inaccurate offset (e.g.-0044instead of-004430). When parsed back (viaCastor CSV reader), the timestamp instant was shifted by up to 59 seconds.What changes are included in this PR?
cpp/src/arrow/vendored/datetime/date.h:to_streamfor format specifiers%zand%Ez, check whether*offset_sechas non-zero seconds (s != 0).±hhmmssfor%zor±hh:mm:ssfor%Ez). If whole minutes, retain the standard±hhmm/±hh:mmrepresentation.cpp/src/arrow/util/value_parsing.h:detail::ParseHHMMSSto parse 6-digit offsets with hours, minutes, and seconds.ParseTimestampISO8601to recognize and parse[+-]HHMMSSand[+-]HH:MM:SSzone offsets.cpp/src/arrow/util/value_parsing.cc&cpp/src/arrow/compute/kernels/scalar_temporal_unary.cc:StrptimeTimestampParserandGetZoneto recognize%Ezas providing a zone offset.Tests:
cpp/src/arrow/util/value_parsing_test.cc.Africa/MonroviaandEurope/Paristo strings incpp/src/arrow/compute/kernels/scalar_cast_test.cc.Are these changes tested?
Yes, new unit tests were added to
value_parsing_test.ccandscalar_cast_test.cc.Are there any user-facing changes?
Yes, casting zoned timestamps with sub-minute UTC offsets to string (and exporting them to CSV) now accurately formats the offset seconds rather than truncating them, and
ParseTimestampISO8601can parse timestamps with seconds in the UTC offset.