From cf3dc8c90f5d75214cfe5b2d4983f2d358163b41 Mon Sep 17 00:00:00 2001 From: SHIVANSH-ux-ys Date: Mon, 19 Oct 2026 15:49:48 +0530 Subject: [PATCH] GH-54534: [C++] Fix truncation of sub-minute UTC offsets when formatting and parsing zoned timestamps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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, 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 #54534. Signed-off-by: SHIVANSH-ux-ys --- .../arrow/compute/kernels/scalar_cast_test.cc | 10 +++ .../compute/kernels/scalar_temporal_unary.cc | 11 +-- cpp/src/arrow/util/value_parsing.cc | 7 +- cpp/src/arrow/util/value_parsing.h | 67 ++++++++++++++++--- cpp/src/arrow/util/value_parsing_test.cc | 8 +++ cpp/src/arrow/vendored/datetime/date.h | 15 ++++- 6 files changed, 99 insertions(+), 19 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc index 864ec4afce0c..347477293a47 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc @@ -2792,6 +2792,16 @@ TEST_F(CastTimezone, TimestampWithZoneToString) { ArrayFromJSON( string_type, R"(["1968-11-30 13:30:44.123456789-0700", "2016-02-29 10:42:23.456789246-0700"])")); + + CheckCast(ArrayFromJSON(timestamp(TimeUnit::MICRO, "Africa/Monrovia"), + "[-302443200000000]"), + ArrayFromJSON(string_type, + R"(["1960-06-01 11:15:30.000000-004430"])")); + + CheckCast(ArrayFromJSON(timestamp(TimeUnit::MICRO, "Europe/Paris"), + "[-2195899200000000]"), + ArrayFromJSON(string_type, + R"(["1900-06-01 12:09:21.000000+000921"])")); } } diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_unary.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_unary.cc index 74a29081ae86..d51f239af6e1 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_unary.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_unary.cc @@ -1262,16 +1262,19 @@ struct Strftime { // Convert string representations of timestamps in arbitrary format to timestamps const std::string GetZone(const std::string& format) { - // Check for use of %z or %Z + // Check for use of %z or %Ez size_t cur = 0; size_t count = 0; std::string zone = ""; while (cur < format.size() - 1) { if (format[cur] == '%') { count++; - if (format[cur + 1] == 'z' && count % 2 == 1) { - zone = "UTC"; - break; + if (count % 2 == 1) { + if (format[cur + 1] == 'z' || + (format[cur + 1] == 'E' && cur + 2 < format.size() && format[cur + 2] == 'z')) { + zone = "UTC"; + break; + } } } else { count = 0; diff --git a/cpp/src/arrow/util/value_parsing.cc b/cpp/src/arrow/util/value_parsing.cc index 0cc71f276df4..4161c958ce5a 100644 --- a/cpp/src/arrow/util/value_parsing.cc +++ b/cpp/src/arrow/util/value_parsing.cc @@ -78,11 +78,14 @@ class StrptimeTimestampParser : public TimestampParser { public: explicit StrptimeTimestampParser(std::string format) : format_(std::move(format)), have_zone_offset_(false) { - // Check for use of %z + // Check for use of %z or %Ez size_t cur = 0; while (cur < format_.size()) { if (format_[cur] == '%') { - if (cur + 1 < format_.size() && format_[cur + 1] == 'z') { + if (cur + 1 < format_.size() && + (format_[cur + 1] == 'z' || + (format_[cur + 1] == 'E' && cur + 2 < format_.size() && + format_[cur + 2] == 'z'))) { have_zone_offset_ = true; break; } diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index 195cdc843ac6..0097291593a3 100644 --- a/cpp/src/arrow/util/value_parsing.h +++ b/cpp/src/arrow/util/value_parsing.h @@ -553,6 +553,35 @@ static inline bool ParseHH_MM_SS(const char* s, Duration* out) { return true; } +template +static inline bool ParseHHMMSS(const char* s, Duration* out) { + uint8_t hours = 0; + uint8_t minutes = 0; + uint8_t seconds = 0; + if (ARROW_PREDICT_FALSE(!ParseUnsigned(s + 0, 2, &hours))) { + return false; + } + if (ARROW_PREDICT_FALSE(!ParseUnsigned(s + 2, 2, &minutes))) { + return false; + } + if (ARROW_PREDICT_FALSE(!ParseUnsigned(s + 4, 2, &seconds))) { + return false; + } + if (ARROW_PREDICT_FALSE(hours >= 24)) { + return false; + } + if (ARROW_PREDICT_FALSE(minutes >= 60)) { + return false; + } + if (ARROW_PREDICT_FALSE(seconds >= 60)) { + return false; + } + *out = std::chrono::duration_cast(std::chrono::hours(hours) + + std::chrono::minutes(minutes) + + std::chrono::seconds(seconds)); + return true; +} + static inline bool ParseSubSeconds(const char* s, size_t length, TimeUnit::type unit, uint32_t* out) { // The decimal point has been peeled off at this point @@ -669,7 +698,7 @@ static inline bool ParseTimestampISO8601(const char* s, size_t length, // We allow the following zone offset formats: // - (none) // - Z - // - [+-]HH(:?MM)? + // - [+-]HH(:?MM(:?SS)?)? // // We allow the following formats for all units: // - "YYYY-MM-DD" @@ -712,23 +741,25 @@ static inline bool ParseTimestampISO8601(const char* s, size_t length, if (s[length - 1] == 'Z') { --length; if (out_zone_offset_present) *out_zone_offset_present = true; - } else if (s[length - 3] == '+' || s[length - 3] == '-') { - // [+-]HH - length -= 3; - if (ARROW_PREDICT_FALSE(!detail::ParseHH(s + length + 1, &zone_offset))) { + } else if (length >= 19 && (s[length - 9] == '+' || s[length - 9] == '-') && + (s[length - 6] == ':') && (s[length - 3] == ':')) { + // [+-]HH:MM:SS + length -= 9; + if (ARROW_PREDICT_FALSE(!detail::ParseHH_MM_SS(s + length + 1, &zone_offset))) { return false; } if (s[length] == '+') zone_offset *= -1; if (out_zone_offset_present) *out_zone_offset_present = true; - } else if (s[length - 5] == '+' || s[length - 5] == '-') { - // [+-]HHMM - length -= 5; - if (ARROW_PREDICT_FALSE(!detail::ParseHHMM(s + length + 1, &zone_offset))) { + } else if (length >= 17 && (s[length - 7] == '+' || s[length - 7] == '-')) { + // [+-]HHMMSS + length -= 7; + if (ARROW_PREDICT_FALSE(!detail::ParseHHMMSS(s + length + 1, &zone_offset))) { return false; } if (s[length] == '+') zone_offset *= -1; if (out_zone_offset_present) *out_zone_offset_present = true; - } else if ((s[length - 6] == '+' || s[length - 6] == '-') && (s[length - 3] == ':')) { + } else if (length >= 16 && (s[length - 6] == '+' || s[length - 6] == '-') && + (s[length - 3] == ':')) { // [+-]HH:MM length -= 6; if (ARROW_PREDICT_FALSE(!detail::ParseHH_MM(s + length + 1, &zone_offset))) { @@ -736,6 +767,22 @@ static inline bool ParseTimestampISO8601(const char* s, size_t length, } if (s[length] == '+') zone_offset *= -1; if (out_zone_offset_present) *out_zone_offset_present = true; + } else if (length >= 15 && (s[length - 5] == '+' || s[length - 5] == '-')) { + // [+-]HHMM + length -= 5; + if (ARROW_PREDICT_FALSE(!detail::ParseHHMM(s + length + 1, &zone_offset))) { + return false; + } + if (s[length] == '+') zone_offset *= -1; + if (out_zone_offset_present) *out_zone_offset_present = true; + } else if (length >= 13 && (s[length - 3] == '+' || s[length - 3] == '-')) { + // [+-]HH + length -= 3; + if (ARROW_PREDICT_FALSE(!detail::ParseHH(s + length + 1, &zone_offset))) { + return false; + } + if (s[length] == '+') zone_offset *= -1; + if (out_zone_offset_present) *out_zone_offset_present = true; } seconds_type seconds_since_midnight; diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc index b61f777685b7..23a7901a8b61 100644 --- a/cpp/src/arrow/util/value_parsing_test.cc +++ b/cpp/src/arrow/util/value_parsing_test.cc @@ -722,6 +722,10 @@ TEST(StringConversion, ToTimestampDateTime_ISO8601) { AssertConversionFails(type, "1970-01-01 00:00:00-00000"); AssertConversionFails(type, "1970-01-01 00:00:00+2400"); AssertConversionFails(type, "1970-01-01 00:00:00+00:99"); + AssertConversionFails(type, "1970-01-01 00:00:00+00:00:60"); + AssertConversionFails(type, "1970-01-01 00:00:00+000060"); + AssertConversionFails(type, "1970-01-01 00:00:00+240000"); + AssertConversionFails(type, "1970-01-01 00:00:00+24:00:00"); } { TimestampType type{TimeUnit::MILLI}; @@ -742,6 +746,10 @@ TEST(StringConversion, ToTimestampDateTime_ISO8601) { AssertConversion(type, "2018-11-13 17:11:10.123-01", 1542129070123LL + 3600000LL); AssertConversion(type, "2018-11-13 17:11:10.123-0117", 1542129070123LL + 4620000LL); AssertConversion(type, "2018-11-13 17:11:10.123-01:17", 1542129070123LL + 4620000LL); + AssertConversion(type, "2018-11-13 17:11:10.123+004430", 1542129070123LL - 2670000LL); + AssertConversion(type, "2018-11-13 17:11:10.123+00:44:30", 1542129070123LL - 2670000LL); + AssertConversion(type, "2018-11-13 17:11:10.123-004430", 1542129070123LL + 2670000LL); + AssertConversion(type, "2018-11-13 17:11:10.123-00:44:30", 1542129070123LL + 2670000LL); // Invalid subseconds AssertConversionFails(type, "1900-02-28 12:34:56.1234"); diff --git a/cpp/src/arrow/vendored/datetime/date.h b/cpp/src/arrow/vendored/datetime/date.h index 5e0ff8524628..82a7988bb3f0 100644 --- a/cpp/src/arrow/vendored/datetime/date.h +++ b/cpp/src/arrow/vendored/datetime/date.h @@ -6018,11 +6018,12 @@ to_stream(std::basic_ostream& os, const CharT* fmt, os.setstate(ios::failbit); return os; } - auto m = duration_cast(*offset_sec); - auto neg = m < minutes{0}; - m = date::abs(m); + auto abs_sec = date::abs(*offset_sec); + auto m = duration_cast(abs_sec); + auto neg = *offset_sec < std::chrono::seconds{0}; auto h = duration_cast(m); m -= h; + auto s = abs_sec - duration_cast(duration_cast(abs_sec)); if (neg) os << CharT{'-'}; else @@ -6035,6 +6036,14 @@ to_stream(std::basic_ostream& os, const CharT* fmt, if (m < minutes{10}) os << CharT{'0'}; os << m.count(); + if (s != std::chrono::seconds{0}) + { + if (modified != CharT{}) + os << CharT{':'}; + if (s < std::chrono::seconds{10}) + os << CharT{'0'}; + os << s.count(); + } command = nullptr; modified = CharT{}; }