Skip to content

GH-54534: [C++] Fix truncation of sub-minute UTC offsets when formatting and parsing zoned timestamps - #54684

Open
ShivanshhhG wants to merge 1 commit into
apache:mainfrom
ShivanshhhG:fix-54534-subminute-offsets
Open

ShivanshhhG wants to merge 1 commit into
apache:mainfrom
ShivanshhhG:fix-54534-subminute-offsets

Conversation

@ShivanshhhG

Copy link
Copy Markdown

Rationale for this change

When formatting zoned timestamps whose UTC offset includes sub-minute seconds (e.g. historical IANA zones such as Africa/Monrovia prior to 1972 at UTC-00:44:30 or Europe/Paris prior to 1911 at UTC+00:09:21), 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 (via Cast or CSV reader), the timestamp instant was shifted by up to 59 seconds.

What changes are included in this PR?

  1. cpp/src/arrow/vendored/datetime/date.h: In to_stream for %z and %Ez, check whether *offset_sec has non-zero seconds (s != 0). If present, format the seconds component (±hhmmss for %z, ±hh:mm:ss for %Ez). If whole minutes, preserve the standard ±hhmm / ±hh:mm representation.
  2. cpp/src/arrow/util/value_parsing.h: Add detail::ParseHHMMSS to parse 6-digit offsets and update ParseTimestampISO8601 to recognize and parse [+-]HHMMSS and [+-]HH:MM:SS zone offsets.
  3. cpp/src/arrow/util/value_parsing.cc & cpp/src/arrow/compute/kernels/scalar_temporal_unary.cc: Recognize %Ez as providing a zone offset in StrptimeTimestampParser and GetZone.
  4. 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).
  5. cpp/src/arrow/compute/kernels/scalar_cast_test.cc: Add regression tests casting zoned timestamps in Africa/Monrovia (-00:44:30) and Europe/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:SS or ±HHMMSS) rather than truncating them to minutes, preserving instant fidelity across round-trips.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #54534 has been automatically assigned in GitHub to PR creator.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #54534 has no components, please add labels for components.

Comment thread cpp/src/arrow/vendored/datetime/date.h Outdated
if (m < minutes{10})
os << CharT{'0'};
os << m.count();
if (s != std::chrono::seconds{0})

@dmitry-chirkov-dremio dmitry-chirkov-dremio Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

%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.

…p string casting and strftime %Ez

When casting 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), format in UTC (...Z) to preserve the stored instant without violating the %z format contract of whole minutes.

This patch:
1. Updates ConvertZoned in scalar_cast_string.cc to format in UTC when the instant's offset contains non-zero seconds (sub-minute offset).
2. Unifies timezone format validation across Strftime and Strptime (GetZone) using FormatHasZone to recognize %Ez alongside %z and %Z, rejecting %Ez when input has no timezone.
3. Updates ParseTimestampISO8601 in value_parsing.h to parse [+-]HHMMSS and [+-]HH:MM:SS zone offsets.
4. Adds tests for %Ez with and without a timezone in scalar_temporal_test.cc, and adds regression tests in scalar_cast_test.cc and value_parsing_test.cc.

Closes apache#54534.

Signed-off-by: SHIVANSH-ux-ys <singaser78@gmail.com>
@ShivanshhhG
ShivanshhhG force-pushed the fix-54534-subminute-offsets branch from cf3dc8c to 48e0b42 Compare October 26, 2026 12:24
@ShivanshhhG

Copy link
Copy Markdown
Author

Thank you for the detailed feedback @dmitry-chirkov-dremio!

I have updated the PR accordingly:

  1. Preserved %z / %Ez formatting contract in date.h: Reverted all changes to vendored date.h so that %z and %Ez continue adhering to their standard contracts (±hhmm and ±hh:mm).
  2. Sub-minute offset fallback to UTC in ConvertZoned: When casting a zoned timestamp to string, if the timestamp instant has a sub-minute offset (offset % 60 != 0), it is formatted in UTC (...Z) as suggested in [C++] Casting a zoned timestamp to string (and the CSV writer) truncates UTC offsets that have seconds, so the written instant is wrong #54534 to preserve the exact stored instant without emitting an inaccurate local wall offset. Updated expected values in scalar_cast_test.cc.
  3. Unified timezone format validation in Strftime::Make and GetZone: Added FormatHasZone helper function recognizing %Ez alongside %z and %Z. If an unzoned timestamp is passed to Strftime with %Ez, it is properly rejected with "Invalid: Timezone not present, cannot convert to string with timezone: ...".
  4. Added Unit Tests: Added test cases for %Ez both with a timezone (e.g. Asia/Kolkata -> +05:30) and without a timezone (verifying rejection) in scalar_temporal_test.cc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants