Repository navigation
GH-54534: [C++] Fix truncation of sub-minute UTC offsets when formatting and parsing zoned timestamps - #54684
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>
|
|
|
|
| if (m < minutes{10}) | ||
| os << CharT{'0'}; | ||
| os << m.count(); | ||
| if (s != std::chrono::seconds{0}) |
There was a problem hiding this comment.
What if a user calls Arrow’s strftime function with %z? The vendored date library defines %z as +/-hhmm and %Ez as +/-hh:mm, truncating offsets that include seconds. This change instead returns +000921 or +00:09:21. Please avoid changing the format contract and handle sub-minute offsets in the timestamp-to-string cast, for example by formatting these values in UTC as suggested in #54534.
|
|
||
| const std::string GetZone(const std::string& format) { | ||
| // Check for use of %z or %Z | ||
| // Check for use of %z or %Ez |
There was a problem hiding this comment.
%Ez is now recognized here and in StrptimeTimestampParser, but Strftime::Make still checks only %z and %Z when the input has no timezone. This means %Ez is treated as UTC while %z is rejected. Please use the same format check in both paths and add tests for %Ez with and without a timezone.
2 participants
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>, dropping 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: Into_streamfor%zand%Ez, check whether*offset_sechas non-zero seconds (s != 0). If present, format the seconds component (±hhmmssfor%z,±hh:mm:ssfor%Ez). If whole minutes, preserve the standard±hhmm/±hh:mmrepresentation.cpp/src/arrow/util/value_parsing.h: Adddetail::ParseHHMMSSto parse 6-digit offsets and updateParseTimestampISO8601to recognize and parse[+-]HHMMSSand[+-]HH:MM:SSzone offsets.cpp/src/arrow/util/value_parsing.cc&cpp/src/arrow/compute/kernels/scalar_temporal_unary.cc: Recognize%Ezas providing a zone offset inStrptimeTimestampParserandGetZone.cpp/src/arrow/util/value_parsing_test.cc: Add unit tests for sub-minute offsets (+004430,+00:44:30,-004430,-00:44:30) and invalid offset validations (+00:00:60,+000060,+240000,+24:00:00).cpp/src/arrow/compute/kernels/scalar_cast_test.cc: Add regression tests casting zoned timestamps inAfrica/Monrovia(-00:44:30) andEurope/Paris(+00:09:21) to string arrays.Are these changes tested?
Yes, verified with standalone compilation and unit tests covering parsing, formatting, validation bounds, and cast kernels.
Are there any user-facing changes?
Formatting zoned timestamps with historical sub-minute UTC offsets now accurately emits the seconds component (
±HH:MM:SSor±HHMMSS) rather than truncating them to minutes, preserving instant fidelity across round-trips.