From 6998bb47ce0e6493d6e97b3fee14cf04a7c92da2 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Mon, 7 Sep 2026 16:34:30 +0200 Subject: [PATCH 01/12] GH-51215: [C++] Migrate datetime consumers to the chrono shim Route datetime consumers in Arrow and Gandiva through arrow::internal::chrono. Prefer std::chrono when the standard library has reliable C++20 timezone support, while retaining the vendored fallback and allowing an explicit backend override. Compile the vendored timezone sources, including the Apple helper, only when the fallback backend is selected. --- cpp/src/arrow/CMakeLists.txt | 3 +- cpp/src/arrow/array/diff.cc | 30 +++++----- .../compute/kernels/scalar_cast_temporal.cc | 3 +- cpp/src/arrow/config.cc | 15 ++--- cpp/src/arrow/config.h | 4 +- cpp/src/arrow/pretty_print.cc | 1 - cpp/src/arrow/public_api_test.cc | 14 +++++ cpp/src/arrow/testing/util.cc | 2 + cpp/src/arrow/util/CMakeLists.txt | 5 ++ cpp/src/arrow/util/chrono_config_internal.h | 56 +++++++++++++++++++ cpp/src/arrow/util/chrono_internal.h | 43 ++++---------- cpp/src/arrow/util/formatting.h | 37 ++++++------ cpp/src/arrow/util/logger_test.cc | 5 +- cpp/src/arrow/util/meson.build | 2 + cpp/src/arrow/util/value_parsing.h | 21 +++---- cpp/src/arrow/vendored/datetime.cpp | 11 +++- cpp/src/arrow/vendored/datetime_ios.mm | 25 +++++++++ cpp/src/gandiva/cast_time.cc | 16 +++--- cpp/src/gandiva/gdv_function_stubs.cc | 11 ++-- .../gandiva/precompiled/epoch_time_point.h | 43 +++++++------- cpp/src/gandiva/precompiled/time.cc | 20 +++---- .../precompiled/timestamp_arithmetic.cc | 2 +- cpp/src/gandiva/to_date_holder.cc | 1 - 23 files changed, 227 insertions(+), 143 deletions(-) create mode 100644 cpp/src/arrow/util/chrono_config_internal.h create mode 100644 cpp/src/arrow/vendored/datetime_ios.mm diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index 36edf50ba946..dae5f2c5d069 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -538,7 +538,8 @@ set(ARROW_VENDORED_SRCS vendored/uriparser/UriResolve.c vendored/uriparser/UriShorten.c) if(APPLE) - list(APPEND ARROW_VENDORED_SRCS vendored/datetime/ios.mm) + # This wrapper excludes the iOS timezone implementation from standard-backend builds. + list(APPEND ARROW_VENDORED_SRCS vendored/datetime_ios.mm) endif() set_source_files_properties(vendored/datetime.cpp PROPERTIES SKIP_UNITY_BUILD_INCLUSION ON) diff --git a/cpp/src/arrow/array/diff.cc b/cpp/src/arrow/array/diff.cc index fd907e3c7b29..9deeac949926 100644 --- a/cpp/src/arrow/array/diff.cc +++ b/cpp/src/arrow/array/diff.cc @@ -43,17 +43,19 @@ #include "arrow/type_traits.h" #include "arrow/util/bit_util.h" #include "arrow/util/checked_cast.h" +#include "arrow/util/chrono_internal.h" #include "arrow/util/float16.h" #include "arrow/util/logging_internal.h" #include "arrow/util/range.h" #include "arrow/util/ree_util.h" #include "arrow/util/string.h" #include "arrow/util/unreachable.h" -#include "arrow/vendored/datetime.h" #include "arrow/visit_type_inline.h" namespace arrow { +namespace chrono = internal::chrono; + using internal::checked_cast; using internal::checked_pointer_cast; using internal::MakeLazyRange; @@ -631,14 +633,13 @@ class MakeFormatterImpl { template enable_if_date Visit(const T&) { using unit = typename std::conditional::value, - arrow_vendored::date::days, - std::chrono::milliseconds>::type; + chrono::days, std::chrono::milliseconds>::type; - static arrow_vendored::date::sys_days epoch{arrow_vendored::date::jan / 1 / 1970}; + static chrono::sys_days epoch{chrono::jan / 1 / 1970}; impl_ = [](const Array& array, int64_t index, std::ostream* os) { unit value(checked_cast&>(array).Value(index)); - *os << arrow_vendored::date::format("%F", value + epoch); + *os << chrono::format("%F", value + epoch); }; return Status::OK(); } @@ -854,42 +855,41 @@ class MakeFormatterImpl { auto value = checked_cast&>(array).Value(index); // Using unqualified `format` directly would produce ambiguous // lookup because of `std::format` (ARROW-15520). - namespace avd = arrow_vendored::date; using std::chrono::nanoseconds; using std::chrono::microseconds; using std::chrono::milliseconds; using std::chrono::seconds; if (AddEpoch) { - static avd::sys_days epoch{avd::jan / 1 / 1970}; + static chrono::sys_days epoch{chrono::jan / 1 / 1970}; switch (unit) { case TimeUnit::NANO: - *os << avd::format(fmt, static_cast(value) + epoch); + *os << chrono::format(fmt, static_cast(value) + epoch); break; case TimeUnit::MICRO: - *os << avd::format(fmt, static_cast(value) + epoch); + *os << chrono::format(fmt, static_cast(value) + epoch); break; case TimeUnit::MILLI: - *os << avd::format(fmt, static_cast(value) + epoch); + *os << chrono::format(fmt, static_cast(value) + epoch); break; case TimeUnit::SECOND: - *os << avd::format(fmt, static_cast(value) + epoch); + *os << chrono::format(fmt, static_cast(value) + epoch); break; } return; } switch (unit) { case TimeUnit::NANO: - *os << avd::format(fmt, static_cast(value)); + *os << chrono::format(fmt, static_cast(value)); break; case TimeUnit::MICRO: - *os << avd::format(fmt, static_cast(value)); + *os << chrono::format(fmt, static_cast(value)); break; case TimeUnit::MILLI: - *os << avd::format(fmt, static_cast(value)); + *os << chrono::format(fmt, static_cast(value)); break; case TimeUnit::SECOND: - *os << avd::format(fmt, static_cast(value)); + *os << chrono::format(fmt, static_cast(value)); break; } }; diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc b/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc index d076186e5635..a35c8e3d42e2 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_temporal.cc @@ -462,8 +462,7 @@ struct ParseDate { using value_type = typename DateType::c_type; using duration_type = - typename std::conditional::value, - arrow_vendored::date::days, + typename std::conditional::value, chrono::days, std::chrono::milliseconds>::type; template diff --git a/cpp/src/arrow/config.cc b/cpp/src/arrow/config.cc index 41cc6decc6bf..290c0db2f447 100644 --- a/cpp/src/arrow/config.cc +++ b/cpp/src/arrow/config.cc @@ -19,14 +19,15 @@ #include +#include "arrow/util/chrono_internal.h" #include "arrow/util/config.h" #include "arrow/util/config_internal.h" #include "arrow/util/cpu_info.h" -#include "arrow/vendored/datetime.h" namespace arrow { using internal::CpuInfo; +namespace chrono = internal::chrono; namespace { @@ -77,8 +78,8 @@ RuntimeInfo GetRuntimeInfo() { MakeSimdLevelString([&](int64_t flags) { return cpu_info->IsSupported(flags); }); info.detected_simd_level = MakeSimdLevelString([&](int64_t flags) { return cpu_info->IsDetected(flags); }); - info.using_os_timezone_db = USE_OS_TZDB; -#if !USE_OS_TZDB + info.using_os_timezone_db = ARROW_CHRONO_USE_OS_TZDB; +#if !ARROW_CHRONO_USE_OS_TZDB info.timezone_db_path = timezone_db_path; #else info.timezone_db_path = std::optional(); @@ -91,10 +92,10 @@ RuntimeInfo GetRuntimeInfo() { Status Initialize(const GlobalOptions& options) noexcept { ARROW_SUPPRESS_DEPRECATION_WARNING if (options.timezone_db_path.has_value()) { -#if !USE_OS_TZDB +#if !ARROW_CHRONO_USE_OS_TZDB try { - arrow_vendored::date::set_install(options.timezone_db_path.value()); - arrow_vendored::date::reload_tzdb(); + chrono::set_install(options.timezone_db_path.value()); + chrono::reload_tzdb(); } catch (const std::runtime_error& e) { return Status::IOError(e.what()); } @@ -103,7 +104,7 @@ Status Initialize(const GlobalOptions& options) noexcept { return Status::Invalid( "Arrow was set to use OS timezone database at compile time, " "so a downloaded database cannot be provided at runtime."); -#endif // !USE_OS_TZDB +#endif // !ARROW_CHRONO_USE_OS_TZDB } ARROW_UNSUPPRESS_DEPRECATION_WARNING return Status::OK(); diff --git a/cpp/src/arrow/config.h b/cpp/src/arrow/config.h index cbb29c84ae77..fe5a7937dd6a 100644 --- a/cpp/src/arrow/config.h +++ b/cpp/src/arrow/config.h @@ -66,8 +66,8 @@ struct RuntimeInfo { /// The SIMD level available on the OS and CPU std::string detected_simd_level; - /// Whether using the OS-based timezone database - /// This is set at compile-time. + /// Whether the timezone database is managed by the OS or standard library, + /// rather than Arrow's configurable text database. This is set at compile-time. bool using_os_timezone_db; /// The path to the timezone database; by default None. diff --git a/cpp/src/arrow/pretty_print.cc b/cpp/src/arrow/pretty_print.cc index 723449928592..a311247e53c4 100644 --- a/cpp/src/arrow/pretty_print.cc +++ b/cpp/src/arrow/pretty_print.cc @@ -42,7 +42,6 @@ #include "arrow/util/int_util_overflow.h" #include "arrow/util/key_value_metadata.h" #include "arrow/util/string.h" -#include "arrow/vendored/datetime.h" #include "arrow/visit_array_inline.h" namespace arrow { diff --git a/cpp/src/arrow/public_api_test.cc b/cpp/src/arrow/public_api_test.cc index 12c703f120f6..30a148e239f7 100644 --- a/cpp/src/arrow/public_api_test.cc +++ b/cpp/src/arrow/public_api_test.cc @@ -125,6 +125,19 @@ TEST(Misc, BuildInfo) { // TODO(GH-48593): Remove when libc++ supports std::chrono timezones. ARROW_SUPPRESS_DEPRECATION_WARNING TEST(Misc, SetTimezoneConfig) { + ASSERT_OK(Initialize(GlobalOptions{})); + if (GetRuntimeInfo().using_os_timezone_db) { + ASSERT_FALSE(GetRuntimeInfo().timezone_db_path.has_value()); + // Standard-library backends must reject even an existing path rather than + // silently configuring an unused vendored database. + GlobalOptions options; + options.timezone_db_path = "."; + ASSERT_RAISES(Invalid, Initialize(options)); + ASSERT_FALSE(GetRuntimeInfo().timezone_db_path.has_value()); + EnvVarGuard tzdata("ARROW_TIMEZONE_DATABASE", "."); + ASSERT_OK(InitTestTimezoneDatabase()); + return; + } #ifndef _WIN32 GTEST_SKIP() << "Can only set the Timezone database on Windows"; #elif !defined(ARROW_FILESYSTEM) @@ -164,6 +177,7 @@ TEST(Misc, SetTimezoneConfig) { // Validate that tzdb is working ASSERT_OK(arrow::Initialize(options)); + ASSERT_EQ(GetRuntimeInfo().timezone_db_path, options.timezone_db_path); #endif } ARROW_UNSUPPRESS_DEPRECATION_WARNING diff --git a/cpp/src/arrow/testing/util.cc b/cpp/src/arrow/testing/util.cc index 5edec7cd21fe..2b2b130b2ee0 100644 --- a/cpp/src/arrow/testing/util.cc +++ b/cpp/src/arrow/testing/util.cc @@ -141,6 +141,8 @@ std::optional GetTestTimezoneDatabaseRoot() { // TODO(GH-48593): Remove when libc++ supports std::chrono timezones. ARROW_SUPPRESS_DEPRECATION_WARNING Status InitTestTimezoneDatabase() { + if (GetRuntimeInfo().using_os_timezone_db) return Status::OK(); + auto maybe_tzdata = GetTestTimezoneDatabaseRoot(); // If missing, timezone database will default to %USERPROFILE%\Downloads\tzdata if (!maybe_tzdata.has_value()) return Status::OK(); diff --git a/cpp/src/arrow/util/CMakeLists.txt b/cpp/src/arrow/util/CMakeLists.txt index c67abf55a251..806f9585c154 100644 --- a/cpp/src/arrow/util/CMakeLists.txt +++ b/cpp/src/arrow/util/CMakeLists.txt @@ -22,6 +22,11 @@ # Headers: top level arrow_install_all_headers("arrow/util") +# The automatic rule excludes internal headers, but these are dependencies of +# the installed formatting.h and value_parsing.h headers. +install(FILES chrono_config_internal.h chrono_internal.h + DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/arrow/util") + # # arrow_test_main # diff --git a/cpp/src/arrow/util/chrono_config_internal.h b/cpp/src/arrow/util/chrono_config_internal.h new file mode 100644 index 000000000000..731f21065003 --- /dev/null +++ b/cpp/src/arrow/util/chrono_config_internal.h @@ -0,0 +1,56 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include + +// Share backend selection with the vendored implementation without including its +// headers. datetime.h undefines macros needed when compiling the implementation. +// +// On Windows, MSVC's standard library uses the system timezone database, while +// libstdc++ reads tzdata files (using TZDIR). Libraries without the C++20 timezone +// APIs, including older libc++, still require the vendored date library. +// +// Use the standard backend by default. Builds may explicitly define +// ARROW_USE_STD_CHRONO to 0 or 1 when they need to select a backend. +// +// Automatically disable the default for libraries without the C++20 timezone APIs. +// On non-Windows, older libstdc++ versions also need the fallback because of +// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116110 (fully fixed in GCC 16.2). +// Check library macros, not __GNUC__, so Clang using libstdc++ agrees with GCC. +// The datestamp distinguishes 16.2 (2026-08-07) from 16.1 and early snapshots. +// Keep the existing Windows backend selection unchanged. +#ifndef ARROW_USE_STD_CHRONO +# define ARROW_USE_STD_CHRONO 1 +# if !defined(__cpp_lib_chrono) || __cpp_lib_chrono < 201907L +# undef ARROW_USE_STD_CHRONO +# define ARROW_USE_STD_CHRONO 0 +# elif !defined(_WIN32) && defined(__GLIBCXX__) && \ + (!defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < 16 || __GLIBCXX__ < 20260807) +# undef ARROW_USE_STD_CHRONO +# define ARROW_USE_STD_CHRONO 0 +# endif +#endif + +// Only the vendored Windows text database supports setting its path via Arrow. +// The non-Windows vendored backend uses USE_OS_TZDB (see datetime/visibility.h). +#if ARROW_USE_STD_CHRONO || !defined(_WIN32) +# define ARROW_CHRONO_USE_OS_TZDB 1 +#else +# define ARROW_CHRONO_USE_OS_TZDB 0 +#endif diff --git a/cpp/src/arrow/util/chrono_internal.h b/cpp/src/arrow/util/chrono_internal.h index ea4051bccf59..ad602b778ab5 100644 --- a/cpp/src/arrow/util/chrono_internal.h +++ b/cpp/src/arrow/util/chrono_internal.h @@ -21,45 +21,19 @@ /// \brief Abstraction layer for C++20 chrono calendar/timezone APIs /// /// This header provides a unified interface for chrono calendar and timezone -/// functionality. On compilers with full C++20 chrono support, it uses -/// std::chrono. On other compilers, it falls back to the vendored Howard Hinnant +/// functionality. It uses std::chrono with supported C++20 timezone +/// implementations, otherwise falling back to the vendored Howard Hinnant /// date library. +/// See chrono_config_internal.h for backend selection. /// -/// The main benefit is on Windows where std::chrono uses the system timezone -/// database, eliminating the need for users to install IANA tzdata separately. +/// On Windows with MSVC, std::chrono uses the system timezone database, +/// eliminating the need for users to install IANA tzdata separately. #include #include #include -// Feature detection for C++20 chrono timezone support -// https://en.cppreference.com/w/cpp/compiler_support/20.html#cpp_lib_chrono_201907L -// -// On Windows with MSVC: std::chrono uses Windows' internal timezone database, -// eliminating the need for users to install IANA tzdata separately. -// -// On Windows with MinGW/GCC: libstdc++ reads tzdata files via TZDIR env var. -// Set TZDIR=/usr/share/zoneinfo to use the system tzdata. -// -// On non-Windows: GCC libstdc++ has a bug where DST state is incorrectly reset when -// a timezone transitions between rule sets (e.g., Australia/Broken_Hill around -// 2000-02-29). Until this is fixed, we use the vendored date.h library. -// See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116110 - -// Use std::chrono on Windows when C++20 chrono timezone support is available. -// The __cpp_lib_chrono >= 201907L feature test macro indicates full support: -// - MSVC: Uses Windows' internal timezone database (no IANA tzdata needed) -// - GCC/libstdc++: Requires TZDIR environment variable to locate tzdata -// - Clang/libc++: Does not define 201907L (no timezone support), so falls back -// -// On non-Windows, we use the vendored date library due to a GCC libstdc++ bug -// where DST state is incorrectly reset during timezone rule transitions. -// See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116110 -#if defined(_WIN32) && defined(__cpp_lib_chrono) && __cpp_lib_chrono >= 201907L -# define ARROW_USE_STD_CHRONO 1 -#else -# define ARROW_USE_STD_CHRONO 0 -#endif +#include "arrow/util/chrono_config_internal.h" #if ARROW_USE_STD_CHRONO // Use C++20 standard library chrono @@ -248,6 +222,11 @@ inline const time_zone* locate_zone(std::string_view tz_name) { inline const time_zone* current_zone() { return vendored::current_zone(); } +# if !ARROW_CHRONO_USE_OS_TZDB +using vendored::reload_tzdb; +using vendored::set_install; +# endif + // Formatting support using vendored::format; diff --git a/cpp/src/arrow/util/formatting.h b/cpp/src/arrow/util/formatting.h index 844b6fb91a8d..3447513526e9 100644 --- a/cpp/src/arrow/util/formatting.h +++ b/cpp/src/arrow/util/formatting.h @@ -32,11 +32,11 @@ #include "arrow/status.h" #include "arrow/type_fwd.h" #include "arrow/type_traits.h" +#include "arrow/util/chrono_internal.h" #include "arrow/util/macros.h" #include "arrow/util/string.h" #include "arrow/util/time.h" #include "arrow/util/visibility.h" -#include "arrow/vendored/datetime.h" namespace arrow { namespace internal { @@ -344,7 +344,7 @@ constexpr size_t BufferSizeYYYY_MM_DD() { detail::Digits10(31); } -inline void FormatYYYY_MM_DD(arrow_vendored::date::year_month_day ymd, char** cursor) { +inline void FormatYYYY_MM_DD(chrono::year_month_day ymd, char** cursor) { FormatTwoDigits(static_cast(ymd.day()), cursor); FormatOneChar('-', cursor); FormatTwoDigits(static_cast(ymd.month()), cursor); @@ -372,7 +372,7 @@ constexpr size_t BufferSizeHH_MM_SS() { } template -void FormatHH_MM_SS(arrow_vendored::date::hh_mm_ss hms, char** cursor) { +void FormatHH_MM_SS(chrono::hh_mm_ss hms, char** cursor) { constexpr size_t subsecond_digits = Digits10(Duration::period::den) - 1; if (subsecond_digits != 0) { FormatAllDigitsLeftPadded(hms.subseconds().count(), subsecond_digits, '0', cursor); @@ -386,20 +386,18 @@ void FormatHH_MM_SS(arrow_vendored::date::hh_mm_ss hms, char** cursor) } // Some out-of-bound datetime values would result in erroneous printing -// because of silent integer wraparound in the `arrow_vendored::date` library. +// because calendar conversions outside the supported year range can wrap around. // // To avoid such misprinting, we must therefore check the bounds explicitly. // The bounds correspond to start of year -32767 and end of year 32767, -// respectively (-32768 is an invalid year value in `arrow_vendored::date`). +// respectively (-32768 is an invalid year value in both chrono backends). // // Note these values are the same as documented for C++20: // https://en.cppreference.com/w/cpp/chrono/year_month_day/operator_days template bool IsDateTimeInRange(Unit duration) { - constexpr Unit kMinIncl = - std::chrono::duration_cast(arrow_vendored::date::days{-12687428}); - constexpr Unit kMaxExcl = - std::chrono::duration_cast(arrow_vendored::date::days{11248738}); + constexpr Unit kMinIncl = std::chrono::duration_cast(chrono::days{-12687428}); + constexpr Unit kMaxExcl = std::chrono::duration_cast(chrono::days{11248738}); return duration >= kMinIncl && duration < kMaxExcl; } @@ -422,7 +420,7 @@ Return FormatOutOfRange(RawValue&& raw_value, Appender&& append) { return append(std::move(formatted)); } -const auto kEpoch = arrow_vendored::date::sys_days{arrow_vendored::date::jan / 1 / 1970}; +const auto kEpoch = chrono::sys_days{chrono::jan / 1 / 1970}; } // namespace detail @@ -437,16 +435,15 @@ class DateToStringFormatterMixin { protected: template - Return FormatDays(arrow_vendored::date::days since_epoch, Appender&& append) { - arrow_vendored::date::sys_days timepoint_days{since_epoch}; + Return FormatDays(chrono::days since_epoch, Appender&& append) { + chrono::sys_days timepoint_days{since_epoch}; constexpr size_t buffer_size = detail::BufferSizeYYYY_MM_DD(); std::array buffer; char* cursor = buffer.data() + buffer_size; - detail::FormatYYYY_MM_DD(arrow_vendored::date::year_month_day{timepoint_days}, - &cursor); + detail::FormatYYYY_MM_DD(chrono::year_month_day{timepoint_days}, &cursor); return append(detail::ViewDigitBuffer(buffer, cursor)); } }; @@ -460,7 +457,7 @@ class StringFormatter : public DateToStringFormatterMixin { template Return operator()(value_type value, Appender&& append) { - const auto since_epoch = arrow_vendored::date::days{value}; + const auto since_epoch = chrono::days{value}; if (!ARROW_PREDICT_TRUE(detail::IsDateTimeInRange(since_epoch))) { return detail::FormatOutOfRange(value, append); } @@ -481,7 +478,7 @@ class StringFormatter : public DateToStringFormatterMixin { if (!ARROW_PREDICT_TRUE(detail::IsDateTimeInRange(since_epoch))) { return detail::FormatOutOfRange(value, append); } - return FormatDays(std::chrono::duration_cast(since_epoch), + return FormatDays(std::chrono::duration_cast(since_epoch), std::forward(append)); } }; @@ -497,7 +494,7 @@ class StringFormatter { template Return operator()(Duration, value_type value, Appender&& append) { - using arrow_vendored::date::days; + using chrono::days; const Duration since_epoch{value}; if (!ARROW_PREDICT_TRUE(detail::IsDateTimeInRange(since_epoch))) { @@ -506,7 +503,7 @@ class StringFormatter { const auto timepoint = detail::kEpoch + since_epoch; // Round days towards zero - // (the naive approach of using arrow_vendored::date::floor() would + // (the naive approach of using chrono::floor() would // result in UB for very large negative timestamps, similarly as // https://github.com/HowardHinnant/date/issues/696) auto timepoint_days = std::chrono::time_point_cast(timepoint); @@ -530,7 +527,7 @@ class StringFormatter { if (timezone_.size() > 0) { detail::FormatOneChar('Z', &cursor); } - detail::FormatHH_MM_SS(arrow_vendored::date::make_time(since_midnight), &cursor); + detail::FormatHH_MM_SS(chrono::hh_mm_ss{since_midnight}, &cursor); detail::FormatOneChar(' ', &cursor); detail::FormatYYYY_MM_DD(timepoint_days, &cursor); return append(detail::ViewDigitBuffer(buffer, cursor)); @@ -566,7 +563,7 @@ class StringFormatter> { std::array buffer; char* cursor = buffer.data() + buffer_size; - detail::FormatHH_MM_SS(arrow_vendored::date::make_time(since_midnight), &cursor); + detail::FormatHH_MM_SS(chrono::hh_mm_ss{since_midnight}, &cursor); return append(detail::ViewDigitBuffer(buffer, cursor)); } diff --git a/cpp/src/arrow/util/logger_test.cc b/cpp/src/arrow/util/logger_test.cc index 0faea81a598f..786d99e157e4 100644 --- a/cpp/src/arrow/util/logger_test.cc +++ b/cpp/src/arrow/util/logger_test.cc @@ -23,8 +23,9 @@ #include "arrow/testing/gtest_util.h" #include "arrow/util/logger.h" -// Emit log via the default logger -#define DO_LOG(LEVEL, ...) ARROW_LOGGER_CALL("", LEVEL, __VA_ARGS__) +// Emit log via the default logger. Token-paste here to prevent Windows' ERROR +// macro from expanding before the logger macro is selected. +#define DO_LOG(LEVEL, ...) ARROW_LOGGER_##LEVEL("", __VA_ARGS__) namespace arrow { namespace util { diff --git a/cpp/src/arrow/util/meson.build b/cpp/src/arrow/util/meson.build index 729cfba47222..52c19fa60079 100644 --- a/cpp/src/arrow/util/meson.build +++ b/cpp/src/arrow/util/meson.build @@ -121,6 +121,8 @@ install_headers( 'byte_size.h', 'cancel.h', 'checked_cast.h', + 'chrono_config_internal.h', + 'chrono_internal.h', 'compare.h', 'compression.h', 'concurrent_map.h', diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index 195cdc843ac6..750970a33fee 100644 --- a/cpp/src/arrow/util/value_parsing.h +++ b/cpp/src/arrow/util/value_parsing.h @@ -31,13 +31,13 @@ #include "arrow/type.h" #include "arrow/type_traits.h" #include "arrow/util/checked_cast.h" +#include "arrow/util/chrono_internal.h" #include "arrow/util/config.h" #include "arrow/util/float16.h" #include "arrow/util/int_util_overflow.h" #include "arrow/util/macros.h" #include "arrow/util/time.h" #include "arrow/util/visibility.h" -#include "arrow/vendored/datetime.h" #include "arrow/vendored/strptime.h" namespace arrow { @@ -651,13 +651,11 @@ static inline bool ParseYYYY_MM_DD(const char* s, Duration* since_epoch) { if (ARROW_PREDICT_FALSE(!ParseUnsigned(s + 8, 2, &day))) { return false; } - arrow_vendored::date::year_month_day ymd{arrow_vendored::date::year{year}, - arrow_vendored::date::month{month}, - arrow_vendored::date::day{day}}; + chrono::year_month_day ymd{chrono::year{year}, chrono::month{month}, chrono::day{day}}; if (ARROW_PREDICT_FALSE(!ymd.ok())) return false; - *since_epoch = std::chrono::duration_cast( - arrow_vendored::date::sys_days{ymd}.time_since_epoch()); + *since_epoch = + std::chrono::duration_cast(chrono::sys_days{ymd}.time_since_epoch()); return true; } @@ -810,7 +808,7 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, const char* format, bool ignore_time_in_day, bool allow_trailing_chars, TimeUnit::type unit, int64_t* out) { - // NOTE: strptime() is more than 10x faster than arrow_vendored::date::parse(). + // Keep strptime(): benchmarks found it more than 10x faster than date::parse(). // The buffer may not be nul-terminated std::string clean_copy(buf, length); struct tm result; @@ -827,9 +825,9 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, return false; } // ignore the time part - arrow_vendored::date::sys_seconds secs = - arrow_vendored::date::sys_days(arrow_vendored::date::year(result.tm_year + 1900) / - (result.tm_mon + 1) / std::max(result.tm_mday, 1)); + chrono::sys_seconds secs = + chrono::sys_days(chrono::year(result.tm_year + 1900) / (result.tm_mon + 1) / + std::max(result.tm_mday, 1)); if (!ignore_time_in_day) { secs += (std::chrono::hours(result.tm_hour) + std::chrono::minutes(result.tm_min) + std::chrono::seconds(result.tm_sec)); @@ -860,8 +858,7 @@ struct StringConverter> { using value_type = typename DATE_TYPE::c_type; using duration_type = - typename std::conditional::value, - arrow_vendored::date::days, + typename std::conditional::value, chrono::days, std::chrono::milliseconds>::type; bool Convert(const DATE_TYPE& type, const char* s, size_t length, value_type* out) { diff --git a/cpp/src/arrow/vendored/datetime.cpp b/cpp/src/arrow/vendored/datetime.cpp index 0f0bd12c7e16..b4f9dd368300 100644 --- a/cpp/src/arrow/vendored/datetime.cpp +++ b/cpp/src/arrow/vendored/datetime.cpp @@ -15,5 +15,12 @@ // specific language governing permissions and limitations // under the License. -#include "datetime/visibility.h" -#include "datetime/tz.cpp" +#include "arrow/util/chrono_config_internal.h" + +// Keep backend selection identical to the callers, including in Gandiva tests. +// Standard-library builds must not compile a second timezone implementation. +#if !ARROW_USE_STD_CHRONO +# include "datetime/visibility.h" + +# include "datetime/tz.cpp" +#endif diff --git a/cpp/src/arrow/vendored/datetime_ios.mm b/cpp/src/arrow/vendored/datetime_ios.mm new file mode 100644 index 000000000000..35f0fe08d729 --- /dev/null +++ b/cpp/src/arrow/vendored/datetime_ios.mm @@ -0,0 +1,25 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Evaluate automatic backend selection only when the build has not selected one. +#ifndef ARROW_USE_STD_CHRONO +# include "arrow/util/chrono_config_internal.h" +#endif + +#if !ARROW_USE_STD_CHRONO +# include "datetime/ios.mm" +#endif diff --git a/cpp/src/gandiva/cast_time.cc b/cpp/src/gandiva/cast_time.cc index f170375298b5..3effeeab36ab 100644 --- a/cpp/src/gandiva/cast_time.cc +++ b/cpp/src/gandiva/cast_time.cc @@ -17,7 +17,7 @@ #include -#include "arrow/vendored/datetime.h" +#include "arrow/util/chrono_internal.h" #include "gandiva/precompiled/time_fields.h" @@ -48,17 +48,19 @@ arrow::Status ExportedTimeFunctions::AddMappings(Engine* engine) const { } // namespace gandiva #endif // !GANDIVA_UNIT_TEST +namespace chrono = arrow::internal::chrono; + extern "C" { // TODO : Do input validation or make sure the callers do that ? int gdv_fn_time_with_zone(int* time_fields, const char* zone, int zone_len, int64_t* ret_time) { - using arrow_vendored::date::day; - using arrow_vendored::date::local_days; - using arrow_vendored::date::locate_zone; - using arrow_vendored::date::month; - using arrow_vendored::date::time_zone; - using arrow_vendored::date::year; + using chrono::day; + using chrono::local_days; + using chrono::locate_zone; + using chrono::month; + using chrono::time_zone; + using chrono::year; using std::chrono::hours; using std::chrono::milliseconds; using std::chrono::minutes; diff --git a/cpp/src/gandiva/gdv_function_stubs.cc b/cpp/src/gandiva/gdv_function_stubs.cc index 6b3e9935b017..13cbcff69b5c 100644 --- a/cpp/src/gandiva/gdv_function_stubs.cc +++ b/cpp/src/gandiva/gdv_function_stubs.cc @@ -27,6 +27,7 @@ #include "arrow/util/base64.h" #include "arrow/util/bit_util.h" +#include "arrow/util/chrono_internal.h" #include "arrow/util/double_conversion_internal.h" #include "arrow/util/value_parsing.h" @@ -38,6 +39,8 @@ #include "gandiva/random_generator_holder.h" #include "gandiva/to_date_holder.h" +namespace chrono = arrow::internal::chrono; + /// Stub functions that can be accessed from LLVM or the pre-compiled library. extern "C" { @@ -835,8 +838,8 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_ GANDIVA_EXPORT gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_milliseconds, const char* timezone, gdv_int32 length) { - using arrow_vendored::date::locate_zone; - using arrow_vendored::date::sys_time; + using chrono::locate_zone; + using chrono::sys_time; using std::chrono::milliseconds; sys_time tp{milliseconds{time_milliseconds}}; @@ -855,8 +858,8 @@ GANDIVA_EXPORT gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context, gdv_timestamp time_milliseconds, const char* timezone, gdv_int32 length) { - using arrow_vendored::date::sys_time; - using arrow_vendored::date::zoned_time; + using chrono::sys_time; + using chrono::zoned_time; using std::chrono::milliseconds; const sys_time tp{milliseconds{time_milliseconds}}; diff --git a/cpp/src/gandiva/precompiled/epoch_time_point.h b/cpp/src/gandiva/precompiled/epoch_time_point.h index 45cfb28ca38c..781d588a51ac 100644 --- a/cpp/src/gandiva/precompiled/epoch_time_point.h +++ b/cpp/src/gandiva/precompiled/epoch_time_point.h @@ -17,11 +17,12 @@ #pragma once -// TODO(wesm): IR compilation does not have any include directories set -#include "../../arrow/vendored/datetime/date.h" +#include "arrow/util/chrono_internal.h" + +namespace chrono = arrow::internal::chrono; bool is_leap_year(int yy); -bool did_days_overflow(arrow_vendored::date::year_month_day ymd); +bool did_days_overflow(chrono::year_month_day ymd); int last_possible_day_in_month(int month, int year); // A point of time measured in millis since epoch. @@ -38,19 +39,16 @@ class EpochTimePoint { int TmMon() const { return static_cast(YearMonthDay().month()) - 1; } int TmYday() const { - auto to_days = arrow_vendored::date::floor(tp_); - auto first_day_in_year = arrow_vendored::date::sys_days{ - YearMonthDay().year() / arrow_vendored::date::jan / 1}; + auto to_days = chrono::floor(tp_); + auto first_day_in_year = chrono::sys_days{YearMonthDay().year() / chrono::jan / 1}; return (to_days - first_day_in_year).count(); } int TmMday() const { return static_cast(YearMonthDay().day()); } int TmWday() const { - auto to_days = arrow_vendored::date::floor(tp_); - return (arrow_vendored::date::weekday{to_days} - // NOLINT - arrow_vendored::date::Sunday) - .count(); + auto to_days = chrono::floor(tp_); + return (chrono::weekday{to_days} - chrono::Sunday).count(); } int TmHour() const { return static_cast(TimeOfDay().hours().count()); } @@ -63,16 +61,16 @@ class EpochTimePoint { } EpochTimePoint AddYears(int num_years) const { - auto ymd = YearMonthDay() + arrow_vendored::date::years(num_years); - return EpochTimePoint((arrow_vendored::date::sys_days{ymd} + // NOLINT + auto ymd = YearMonthDay() + chrono::years(num_years); + return EpochTimePoint((chrono::sys_days{ymd} + // NOLINT TimeOfDay().to_duration()) .time_since_epoch()); } EpochTimePoint AddMonths(int num_months) const { - auto ymd = YearMonthDay() + arrow_vendored::date::months(num_months); + auto ymd = YearMonthDay() + chrono::months(num_months); - EpochTimePoint tp = EpochTimePoint((arrow_vendored::date::sys_days{ymd} + // NOLINT + EpochTimePoint tp = EpochTimePoint((chrono::sys_days{ymd} + // NOLINT TimeOfDay().to_duration()) .time_since_epoch()); @@ -87,8 +85,8 @@ class EpochTimePoint { } EpochTimePoint AddDays(int num_days) const { - auto days_since_epoch = arrow_vendored::date::sys_days{YearMonthDay()} + // NOLINT - arrow_vendored::date::days(num_days); + auto days_since_epoch = chrono::sys_days{YearMonthDay()} + // NOLINT + chrono::days(num_days); return EpochTimePoint( (days_since_epoch + TimeOfDay().to_duration()).time_since_epoch()); } @@ -101,17 +99,14 @@ class EpochTimePoint { int64_t MillisSinceEpoch() const { return tp_.time_since_epoch().count(); } - arrow_vendored::date::time_of_day TimeOfDay() const { - auto millis_since_midnight = - tp_ - arrow_vendored::date::floor(tp_); - return arrow_vendored::date::time_of_day( - millis_since_midnight); + chrono::hh_mm_ss TimeOfDay() const { + auto millis_since_midnight = tp_ - chrono::floor(tp_); + return chrono::hh_mm_ss{millis_since_midnight}; } private: - arrow_vendored::date::year_month_day YearMonthDay() const { - return arrow_vendored::date::year_month_day{ - arrow_vendored::date::floor(tp_)}; // NOLINT + chrono::year_month_day YearMonthDay() const { + return chrono::year_month_day{chrono::floor(tp_)}; // NOLINT } std::chrono::time_point tp_; diff --git a/cpp/src/gandiva/precompiled/time.cc b/cpp/src/gandiva/precompiled/time.cc index 2b60f63651db..f1c3a189b0a3 100644 --- a/cpp/src/gandiva/precompiled/time.cc +++ b/cpp/src/gandiva/precompiled/time.cc @@ -637,11 +637,11 @@ void set_error_for_date(gdv_int32 length, const char* input, const char* msg, } gdv_date64 castDATE_utf8(int64_t context, const char* input, gdv_int32 length) { - using arrow_vendored::date::day; - using arrow_vendored::date::month; - using arrow_vendored::date::sys_days; - using arrow_vendored::date::year; - using arrow_vendored::date::year_month_day; + using chrono::day; + using chrono::month; + using chrono::sys_days; + using chrono::year; + using chrono::year_month_day; using gandiva::TimeFields; // format : 0 is year, 1 is month and 2 is day. int dateFields[3]; @@ -701,11 +701,11 @@ gdv_date64 castDATE_utf8(int64_t context, const char* input, gdv_int32 length) { * Format is [ hours:minutes:seconds][.millis][ displacement|zone] */ gdv_timestamp castTIMESTAMP_utf8(int64_t context, const char* input, gdv_int32 length) { - using arrow_vendored::date::day; - using arrow_vendored::date::month; - using arrow_vendored::date::sys_days; - using arrow_vendored::date::year; - using arrow_vendored::date::year_month_day; + using chrono::day; + using chrono::month; + using chrono::sys_days; + using chrono::year; + using chrono::year_month_day; using gandiva::TimeFields; using std::chrono::hours; using std::chrono::milliseconds; diff --git a/cpp/src/gandiva/precompiled/timestamp_arithmetic.cc b/cpp/src/gandiva/precompiled/timestamp_arithmetic.cc index 695605b3cc77..018af1d14af4 100644 --- a/cpp/src/gandiva/precompiled/timestamp_arithmetic.cc +++ b/cpp/src/gandiva/precompiled/timestamp_arithmetic.cc @@ -41,7 +41,7 @@ bool is_last_day_of_month(const EpochTimePoint& tp) { return (tp.TmMday() == days_in_a_month[matrix_index][tp.TmMon()]); } -bool did_days_overflow(arrow_vendored::date::year_month_day ymd) { +bool did_days_overflow(chrono::year_month_day ymd) { int year = static_cast(ymd.year()); int month = static_cast(ymd.month()); int days = static_cast(ymd.day()); diff --git a/cpp/src/gandiva/to_date_holder.cc b/cpp/src/gandiva/to_date_holder.cc index 76f16f0cb1b7..8b7220bb8107 100644 --- a/cpp/src/gandiva/to_date_holder.cc +++ b/cpp/src/gandiva/to_date_holder.cc @@ -21,7 +21,6 @@ #include #include "arrow/util/value_parsing.h" -#include "arrow/vendored/datetime.h" #include "gandiva/date_utils.h" #include "gandiva/execution_context.h" #include "gandiva/node.h" From 7e450a641dd480a3e8b699f21cf2021b90e46831 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Tue, 8 Sep 2026 13:24:28 +0200 Subject: [PATCH 02/12] GH-51215: [C++] Preserve strftime behavior with standard chrono Translate Arrow's strftime syntax to C++20 chrono replacement fields when using the standard backend. Preserve empty formats, literals and braces, locale-sensitive output, duration directives, timezone offsets and abbreviations, and %E/%O modifiers. Keep the vendored formatter available for fallback builds. --- .../compute/kernels/scalar_temporal_test.cc | 22 +++ cpp/src/arrow/util/chrono_internal.h | 172 +++++++++++++++++- 2 files changed, 184 insertions(+), 10 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index 86a81ffdd384..aaa4aefa6c54 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -2003,6 +2003,28 @@ TEST_F(ScalarTemporalTest, TestAssumeTimezoneNonexistent) { &options_earliest); } +TEST_F(ScalarTemporalTest, StrftimeFormatSyntax) { + const auto type = timestamp(TimeUnit::MILLI, "UTC"); + const char* input = R"(["1970-01-01T00:00:00.123", null])"; + for (const auto& [format, expected] : + {std::pair{"", R"(["", null])"}, + std::pair{"literal {%Y}", R"(["literal {1970}", null])"}, + std::pair{"unmatched }%Y{", R"(["unmatched }1970{", null])"}, + std::pair{"%Y}", R"(["1970}", null])"}, + std::pair{"%Q %q %J %z %Z", R"(["123 ms %J +0000 UTC", null])"}, + std::pair{"%% %n%t %Ez %Oz %OV %EJ end%", + R"(["% \n\t +00:00 +00:00 01 %EJ end%", null])"}}) { + SCOPED_TRACE(format); + const auto options = StrftimeOptions(format); + CheckScalarUnary("strftime", type, input, utf8(), expected, &options); + } + + const auto options = StrftimeOptions("%Q %q"); + CheckScalarUnary("strftime", timestamp(TimeUnit::MICRO, "UTC"), + R"(["1970-01-01T00:00:00.000001", null])", utf8(), + R"(["1 \u00b5s", null])", &options); +} + TEST_F(ScalarTemporalTest, StrftimeOffsetTimezone) { auto options_ymdhms = StrftimeOptions("%Y-%m-%dT%H:%M:%S"); diff --git a/cpp/src/arrow/util/chrono_internal.h b/cpp/src/arrow/util/chrono_internal.h index ad602b778ab5..fbb086028e08 100644 --- a/cpp/src/arrow/util/chrono_internal.h +++ b/cpp/src/arrow/util/chrono_internal.h @@ -38,8 +38,10 @@ #if ARROW_USE_STD_CHRONO // Use C++20 standard library chrono # include -# include +# include # include +# include +# include #else // Use vendored Howard Hinnant date library # include "arrow/vendored/datetime.h" @@ -125,22 +127,172 @@ inline const time_zone* locate_zone(std::string_view tz_name) { inline const time_zone* current_zone() { return std::chrono::current_zone(); } -// Formatting support - streams directly using C++20 std::vformat_to -// Provides: direct streaming, stream state preservation, chaining, rich format specifiers +namespace detail { + +// Argument positions passed to std::vformat by to_stream below. +enum class FormatArgument : char { + ZonedTime = '0', + TimeOfDay = '1', + TimeOfDayCount = '2', +}; + +template +void AppendEscapedLiteral(std::basic_string* out, CharT value) { + out->push_back(value); + if (value == CharT{'{'} || value == CharT{'}'}) { + out->push_back(value); + } +} + +// These are the directives accepted by Arrow's existing strftime syntax. Treat +// all others as literals to preserve compatibility. +template +bool IsSupportedStrftimeSpecifier(CharT modifier, CharT specifier) { + const auto contains = [specifier](const char* candidates) { + for (; *candidates != '\0'; ++candidates) { + if (specifier == static_cast(*candidates)) return true; + } + return false; + }; + if (modifier == CharT{}) { + return contains("aAbBhcCxdeDFgGHIjmMprRSTuUVWwXyYzZ"); + } + if (modifier == CharT{'E'}) { + return contains("cCxXyYz"); + } + if (modifier == CharT{'O'}) { + return contains("deHImMSuUVwWyz"); + } + return false; +} + +template +void AppendChronoField(std::basic_string* out, FormatArgument argument, + CharT specifier, CharT modifier = CharT{}) { + *out += {CharT{'{'}, static_cast(argument), CharT{':'}, CharT{'L'}, CharT{'%'}}; + if (modifier != CharT{}) out->push_back(modifier); + *out += {specifier, CharT{'}'}}; +} + +template +void AppendLocalizedField(std::basic_string* out, FormatArgument argument) { + *out += {CharT{'{'}, static_cast(argument), CharT{':'}, CharT{'L'}, CharT{'}'}}; +} + +template +std::basic_string ToChronoFormat(const CharT* fmt, bool use_microseconds_suffix) { + std::basic_string out; + while (*fmt != CharT{}) { + if (*fmt != CharT{'%'}) { + AppendEscapedLiteral(&out, *fmt++); + continue; + } + + ++fmt; + if (*fmt == CharT{}) { + AppendEscapedLiteral(&out, CharT{'%'}); + break; + } + + CharT modifier{}; + if (*fmt == CharT{'E'} || *fmt == CharT{'O'}) { + modifier = *fmt++; + if (*fmt == CharT{}) { + AppendEscapedLiteral(&out, CharT{'%'}); + AppendEscapedLiteral(&out, modifier); + break; + } + } + const CharT specifier = *fmt++; + + if (modifier == CharT{}) { + switch (specifier) { + case CharT{'%'}: + AppendEscapedLiteral(&out, CharT{'%'}); + continue; + case CharT{'n'}: + AppendEscapedLiteral(&out, CharT{'\n'}); + continue; + case CharT{'t'}: + AppendEscapedLiteral(&out, CharT{'\t'}); + continue; + case CharT{'Q'}: + // Formatting a duration's %Q does not consistently apply the numeric locale. + AppendLocalizedField(&out, FormatArgument::TimeOfDayCount); + continue; + case CharT{'q'}: + if (use_microseconds_suffix) { + // Some standard libraries use "us"; Arrow uses the micro sign. + if constexpr (std::is_same_v) { + AppendEscapedLiteral(&out, CharT{'\xC2'}); + AppendEscapedLiteral(&out, CharT{'\xB5'}); + } else { + AppendEscapedLiteral(&out, static_cast(0xB5)); + } + AppendEscapedLiteral(&out, CharT{'s'}); + } else { + AppendChronoField(&out, FormatArgument::TimeOfDay, specifier); + } + continue; + default: + break; + } + } + +# if defined(__GLIBCXX__) + if (modifier == CharT{'O'} && specifier == CharT{'V'}) { + // libstdc++ does not yet accept %OV; use its equivalent base representation. + AppendChronoField(&out, FormatArgument::ZonedTime, specifier); + continue; + } +# endif + + if (IsSupportedStrftimeSpecifier(modifier, specifier)) { + AppendChronoField(&out, FormatArgument::ZonedTime, specifier, modifier); + } else { + AppendEscapedLiteral(&out, CharT{'%'}); + if (modifier != CharT{}) AppendEscapedLiteral(&out, modifier); + AppendEscapedLiteral(&out, specifier); + } + } + return out; +} + +} // namespace detail + +// Convert Arrow's strftime syntax to C++20 replacement fields. Literal braces and +// unsupported directives remain literal, and %Q/%q use local time of day. template std::basic_ostream& to_stream( std::basic_ostream& os, const CharT* fmt, const std::chrono::zoned_time& zt) { - std::vformat_to(std::ostreambuf_iterator(os), std::string("{:") + fmt + "}", - std::make_format_args(zt)); + static_assert(std::is_same_v || std::is_same_v); + using Precision = typename std::chrono::zoned_time::duration; + const auto standard_format = detail::ToChronoFormat( + fmt, std::ratio_equal_v); + const auto local_time = zt.get_local_time(); + const auto local_day = std::chrono::floor(local_time); + const auto time_of_day = local_time - local_day; + const auto time_of_day_count = time_of_day.count(); + + std::basic_string formatted; + if constexpr (std::is_same_v) { + formatted = std::vformat(os.getloc(), standard_format, + std::make_format_args(zt, time_of_day, time_of_day_count)); + } else { + formatted = std::vformat(os.getloc(), standard_format, + std::make_wformat_args(zt, time_of_day, time_of_day_count)); + } + os.write(formatted.data(), static_cast(formatted.size())); return os; } -// Format a duration using strftime-like format specifiers -// Converts "%H%M" style to C++20's "{:%H%M}" style and uses std::vformat -template -std::string format(const char* fmt, const Duration& d) { - return std::vformat(std::string("{:") + fmt + "}", std::make_format_args(d)); +// Format a duration or time point using strftime-like format specifiers. +// Converts "%H%M" style to C++20's "{:L%H%M}" style and uses std::vformat. +template +std::string format(const char* fmt, const Temporal& value) { + return std::vformat(std::locale{}, std::string("{:L") + fmt + "}", + std::make_format_args(value)); } inline constexpr std::chrono::month jan = std::chrono::January; From cc92cd345844906e69953b53629192d062e70631 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Tue, 8 Sep 2026 23:33:48 +0200 Subject: [PATCH 03/12] GH-51215: [CI] Add GCC 16 experimental C++ task --- dev/tasks/tasks.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/dev/tasks/tasks.yml b/dev/tasks/tasks.yml index 0e7d7bd296b9..b3b55c318994 100644 --- a/dev/tasks/tasks.yml +++ b/dev/tasks/tasks.yml @@ -421,6 +421,17 @@ tasks: LLVM: "22" image: debian-cpp + test-debian-experimental-cpp-gcc-16: + ci: github + template: docker-tests/github.linux.yml + params: + env: + ARCH: "amd64" + DEBIAN: "experimental" + GCC: "16" + LLVM: "22" + image: debian-cpp + test-fedora-42-cpp: ci: github template: docker-tests/github.linux.yml From c97bdd57780565bb63a103c1361506d5438a2789 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Thu, 17 Sep 2026 17:02:57 +0200 Subject: [PATCH 04/12] GH-51215: [C++] Address chrono shim review feedback --- .../arrow/compute/kernels/temporal_internal.h | 4 +- cpp/src/arrow/util/chrono_config_internal.h | 21 ++- cpp/src/arrow/util/chrono_internal.h | 166 +++++++++--------- cpp/src/arrow/util/logger_test.cc | 15 +- cpp/src/arrow/util/time_test.cc | 62 +++++++ cpp/src/arrow/vendored/datetime.cpp | 1 - cpp/src/arrow/vendored/datetime_ios.mm | 5 +- 7 files changed, 174 insertions(+), 100 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/temporal_internal.h b/cpp/src/arrow/compute/kernels/temporal_internal.h index 02f5965522ca..5dddf6b00244 100644 --- a/cpp/src/arrow/compute/kernels/temporal_internal.h +++ b/cpp/src/arrow/compute/kernels/temporal_internal.h @@ -165,7 +165,7 @@ struct ZonedLocalizer { template struct TimestampFormatter { - const std::string format; + const chrono::ZonedFormat format; const ArrowTimeZone tz; std::ostringstream bufstream; @@ -182,7 +182,7 @@ struct TimestampFormatter { const auto timepoint = sys_time(Duration{arg}); auto format_zoned_time = [&](auto&& zt) { try { - chrono::to_stream(bufstream, format.c_str(), zt); + chrono::to_stream(bufstream, format, zt); return Status::OK(); } catch (const std::runtime_error& ex) { bufstream.clear(); diff --git a/cpp/src/arrow/util/chrono_config_internal.h b/cpp/src/arrow/util/chrono_config_internal.h index 731f21065003..3ebe27247c12 100644 --- a/cpp/src/arrow/util/chrono_config_internal.h +++ b/cpp/src/arrow/util/chrono_config_internal.h @@ -19,22 +19,21 @@ #include -// Share backend selection with the vendored implementation without including its -// headers. datetime.h undefines macros needed when compiling the implementation. +// Prefer std::chrono when the standard library provides the C++20 timezone APIs. +// Builds may explicitly set ARROW_USE_STD_CHRONO to 0 (vendored date.h) or 1 +// (std::chrono) to override automatic selection. // -// On Windows, MSVC's standard library uses the system timezone database, while -// libstdc++ reads tzdata files (using TZDIR). Libraries without the C++20 timezone -// APIs, including older libc++, still require the vendored date library. +// Share this selection between chrono_internal.h and the vendored timezone sources +// datetime/tz.cpp and datetime/ios.mm. Do not include arrow/vendored/datetime.h here: +// it undefines NOEXCEPT, which is needed to compile datetime/tz.cpp. // -// Use the standard backend by default. Builds may explicitly define -// ARROW_USE_STD_CHRONO to 0 or 1 when they need to select a backend. +// On Windows, MSVC's standard library uses the system timezone database, while +// libstdc++ reads tzdata files (using TZDIR). // -// Automatically disable the default for libraries without the C++20 timezone APIs. -// On non-Windows, older libstdc++ versions also need the fallback because of +// Select the vendored date backend when the C++20 timezone APIs are unavailable. +// On non-Windows, also select it for older libstdc++ versions because of // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116110 (fully fixed in GCC 16.2). -// Check library macros, not __GNUC__, so Clang using libstdc++ agrees with GCC. // The datestamp distinguishes 16.2 (2026-08-07) from 16.1 and early snapshots. -// Keep the existing Windows backend selection unchanged. #ifndef ARROW_USE_STD_CHRONO # define ARROW_USE_STD_CHRONO 1 # if !defined(__cpp_lib_chrono) || __cpp_lib_chrono < 201907L diff --git a/cpp/src/arrow/util/chrono_internal.h b/cpp/src/arrow/util/chrono_internal.h index fbb086028e08..bd59bbe3e434 100644 --- a/cpp/src/arrow/util/chrono_internal.h +++ b/cpp/src/arrow/util/chrono_internal.h @@ -38,10 +38,10 @@ #if ARROW_USE_STD_CHRONO // Use C++20 standard library chrono # include +# include # include # include # include -# include #else // Use vendored Howard Hinnant date library # include "arrow/vendored/datetime.h" @@ -129,107 +129,93 @@ inline const time_zone* current_zone() { return std::chrono::current_zone(); } namespace detail { -// Argument positions passed to std::vformat by to_stream below. +// Argument positions passed to std::vformat_to by to_stream below. enum class FormatArgument : char { ZonedTime = '0', TimeOfDay = '1', TimeOfDayCount = '2', }; -template -void AppendEscapedLiteral(std::basic_string* out, CharT value) { +inline void AppendEscapedLiteral(std::string* out, char value) { out->push_back(value); - if (value == CharT{'{'} || value == CharT{'}'}) { + if (value == '{' || value == '}') { out->push_back(value); } } // These are the directives accepted by Arrow's existing strftime syntax. Treat // all others as literals to preserve compatibility. -template -bool IsSupportedStrftimeSpecifier(CharT modifier, CharT specifier) { - const auto contains = [specifier](const char* candidates) { - for (; *candidates != '\0'; ++candidates) { - if (specifier == static_cast(*candidates)) return true; - } - return false; +inline bool IsSupportedStrftimeSpecifier(char modifier, char specifier) { + const auto contains = [specifier](std::string_view candidates) { + return candidates.find(specifier) != std::string_view::npos; }; - if (modifier == CharT{}) { + if (modifier == '\0') { return contains("aAbBhcCxdeDFgGHIjmMprRSTuUVWwXyYzZ"); } - if (modifier == CharT{'E'}) { + if (modifier == 'E') { return contains("cCxXyYz"); } - if (modifier == CharT{'O'}) { + if (modifier == 'O') { return contains("deHImMSuUVwWyz"); } return false; } -template -void AppendChronoField(std::basic_string* out, FormatArgument argument, - CharT specifier, CharT modifier = CharT{}) { - *out += {CharT{'{'}, static_cast(argument), CharT{':'}, CharT{'L'}, CharT{'%'}}; - if (modifier != CharT{}) out->push_back(modifier); - *out += {specifier, CharT{'}'}}; +inline void AppendChronoField(std::string* out, FormatArgument argument, char specifier, + char modifier = '\0') { + *out += {'{', static_cast(argument), ':', 'L', '%'}; + if (modifier != '\0') out->push_back(modifier); + *out += {specifier, '}'}; } -template -void AppendLocalizedField(std::basic_string* out, FormatArgument argument) { - *out += {CharT{'{'}, static_cast(argument), CharT{':'}, CharT{'L'}, CharT{'}'}}; +inline void AppendLocalizedField(std::string* out, FormatArgument argument) { + *out += {'{', static_cast(argument), ':', 'L', '}'}; } -template -std::basic_string ToChronoFormat(const CharT* fmt, bool use_microseconds_suffix) { - std::basic_string out; - while (*fmt != CharT{}) { - if (*fmt != CharT{'%'}) { +inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) { + std::string out; + while (*fmt != '\0') { + if (*fmt != '%') { AppendEscapedLiteral(&out, *fmt++); continue; } ++fmt; - if (*fmt == CharT{}) { - AppendEscapedLiteral(&out, CharT{'%'}); + if (*fmt == '\0') { + AppendEscapedLiteral(&out, '%'); break; } - CharT modifier{}; - if (*fmt == CharT{'E'} || *fmt == CharT{'O'}) { + char modifier = '\0'; + if (*fmt == 'E' || *fmt == 'O') { modifier = *fmt++; - if (*fmt == CharT{}) { - AppendEscapedLiteral(&out, CharT{'%'}); + if (*fmt == '\0') { + AppendEscapedLiteral(&out, '%'); AppendEscapedLiteral(&out, modifier); break; } } - const CharT specifier = *fmt++; + const char specifier = *fmt++; - if (modifier == CharT{}) { + if (modifier == '\0') { switch (specifier) { - case CharT{'%'}: - AppendEscapedLiteral(&out, CharT{'%'}); + case '%': + AppendEscapedLiteral(&out, '%'); continue; - case CharT{'n'}: - AppendEscapedLiteral(&out, CharT{'\n'}); + case 'n': + AppendEscapedLiteral(&out, '\n'); continue; - case CharT{'t'}: - AppendEscapedLiteral(&out, CharT{'\t'}); + case 't': + AppendEscapedLiteral(&out, '\t'); continue; - case CharT{'Q'}: + case 'Q': // Formatting a duration's %Q does not consistently apply the numeric locale. AppendLocalizedField(&out, FormatArgument::TimeOfDayCount); continue; - case CharT{'q'}: + case 'q': if (use_microseconds_suffix) { // Some standard libraries use "us"; Arrow uses the micro sign. - if constexpr (std::is_same_v) { - AppendEscapedLiteral(&out, CharT{'\xC2'}); - AppendEscapedLiteral(&out, CharT{'\xB5'}); - } else { - AppendEscapedLiteral(&out, static_cast(0xB5)); - } - AppendEscapedLiteral(&out, CharT{'s'}); + out += "\xC2\xB5s"; } else { AppendChronoField(&out, FormatArgument::TimeOfDay, specifier); } @@ -240,7 +226,7 @@ std::basic_string ToChronoFormat(const CharT* fmt, bool use_microseconds_ } # if defined(__GLIBCXX__) - if (modifier == CharT{'O'} && specifier == CharT{'V'}) { + if (modifier == 'O' && specifier == 'V') { // libstdc++ does not yet accept %OV; use its equivalent base representation. AppendChronoField(&out, FormatArgument::ZonedTime, specifier); continue; @@ -250,8 +236,8 @@ std::basic_string ToChronoFormat(const CharT* fmt, bool use_microseconds_ if (IsSupportedStrftimeSpecifier(modifier, specifier)) { AppendChronoField(&out, FormatArgument::ZonedTime, specifier, modifier); } else { - AppendEscapedLiteral(&out, CharT{'%'}); - if (modifier != CharT{}) AppendEscapedLiteral(&out, modifier); + AppendEscapedLiteral(&out, '%'); + if (modifier != '\0') AppendEscapedLiteral(&out, modifier); AppendEscapedLiteral(&out, specifier); } } @@ -260,35 +246,45 @@ std::basic_string ToChronoFormat(const CharT* fmt, bool use_microseconds_ } // namespace detail -// Convert Arrow's strftime syntax to C++20 replacement fields. Literal braces and -// unsupported directives remain literal, and %Q/%q use local time of day. -template -std::basic_ostream& to_stream( - std::basic_ostream& os, const CharT* fmt, - const std::chrono::zoned_time& zt) { - static_assert(std::is_same_v || std::is_same_v); - using Precision = typename std::chrono::zoned_time::duration; - const auto standard_format = detail::ToChronoFormat( - fmt, std::ratio_equal_v); +// Prepare once and reuse for a sequence of zoned times with the same precision. +template +class ZonedFormat { + public: + explicit ZonedFormat(const std::string& format) + : value_(detail::ToChronoFormat( + format.c_str(), std::ratio_equal_v)) { + } + + const std::string& value() const { return value_; } + + private: + using Precision = typename zoned_time::duration; + std::string value_; +}; + +// Literal braces and unsupported directives remain literal; %Q/%q use local time +// of day rather than elapsed time since the epoch. +template +std::ostream& to_stream(std::ostream& os, const ZonedFormat& format, + const std::chrono::zoned_time& zt) { const auto local_time = zt.get_local_time(); const auto local_day = std::chrono::floor(local_time); const auto time_of_day = local_time - local_day; const auto time_of_day_count = time_of_day.count(); - std::basic_string formatted; - if constexpr (std::is_same_v) { - formatted = std::vformat(os.getloc(), standard_format, - std::make_format_args(zt, time_of_day, time_of_day_count)); - } else { - formatted = std::vformat(os.getloc(), standard_format, - std::make_wformat_args(zt, time_of_day, time_of_day_count)); + const std::ostream::sentry sentry(os); + if (sentry) { + const auto end = + std::vformat_to(std::ostreambuf_iterator(os), os.getloc(), format.value(), + std::make_format_args(zt, time_of_day, time_of_day_count)); + if (end.failed()) os.setstate(std::ios::badbit); } - os.write(formatted.data(), static_cast(formatted.size())); return os; } -// Format a duration or time point using strftime-like format specifiers. -// Converts "%H%M" style to C++20's "{:L%H%M}" style and uses std::vformat. +// Format durations and unzoned time points using internal format strings shared +// by both backends. User-supplied strftime syntax uses ZonedFormat and to_stream. +// Keep each format in one replacement field to avoid repeating duration signs. template std::string format(const char* fmt, const Temporal& value) { return std::vformat(std::locale{}, std::string("{:L") + fmt + "}", @@ -382,11 +378,21 @@ using vendored::set_install; // Formatting support using vendored::format; -template -std::basic_ostream& to_stream( - std::basic_ostream& os, const CharT* fmt, - const vendored::zoned_time& zt) { - return vendored::to_stream(os, fmt, zt); +template +class ZonedFormat { + public: + explicit ZonedFormat(const std::string& format) : value_(format) {} + + const std::string& value() const { return value_; } + + private: + std::string value_; +}; + +template +std::ostream& to_stream(std::ostream& os, const ZonedFormat& format, + const vendored::zoned_time& zt) { + return vendored::to_stream(os, format.value().c_str(), zt); } inline constexpr vendored::month jan = vendored::jan; diff --git a/cpp/src/arrow/util/logger_test.cc b/cpp/src/arrow/util/logger_test.cc index 786d99e157e4..a8de9a7611da 100644 --- a/cpp/src/arrow/util/logger_test.cc +++ b/cpp/src/arrow/util/logger_test.cc @@ -23,8 +23,9 @@ #include "arrow/testing/gtest_util.h" #include "arrow/util/logger.h" -// Emit log via the default logger. Token-paste here to prevent Windows' ERROR -// macro from expanding before the logger macro is selected. +// Emit log via the default logger. In unity builds, io_util_test.cc can include +// Windows headers before this file. Paste LEVEL here so ERROR is not expanded to +// 0 before selecting ARROW_LOGGER_ERROR. #define DO_LOG(LEVEL, ...) ARROW_LOGGER_##LEVEL("", __VA_ARGS__) namespace arrow { @@ -59,6 +60,16 @@ struct OstreamableTracer { } // namespace +// Reproduce the Windows macro collision on every platform, without leaking the +// test macro into other sources in a unity build. +#pragma push_macro("ERROR") +#undef ERROR +#define ERROR 0 +TEST(LoggerTest, ErrorMacroCollision) { + DO_LOG(ERROR, "Logging with the Windows ERROR macro defined"); +} +#pragma pop_macro("ERROR") + TEST(LoggerTest, Basics) { // Basic tests using the default logger DO_LOG(ERROR); diff --git a/cpp/src/arrow/util/time_test.cc b/cpp/src/arrow/util/time_test.cc index 0224cca49aac..33cd92bf2f2a 100644 --- a/cpp/src/arrow/util/time_test.cc +++ b/cpp/src/arrow/util/time_test.cc @@ -17,12 +17,74 @@ #include +#include + #include "arrow/testing/gtest_util.h" +#include "arrow/util/date_internal.h" #include "arrow/util/time.h" namespace arrow { namespace util { +TEST(TimeTest, ChronoFormats) { + namespace chrono = arrow::internal::chrono; + using std::chrono::milliseconds; + using std::chrono::minutes; + EXPECT_EQ(chrono::format("%F", chrono::sys_days{}), "1970-01-01"); + EXPECT_EQ(chrono::format("%F %T", chrono::sys_time{milliseconds{-1}}), + "1969-12-31 23:59:59.999"); + EXPECT_EQ(chrono::format("%T", milliseconds{5400123}), "01:30:00.123"); + EXPECT_EQ(chrono::format("%T", milliseconds{-5400123}), "-01:30:00.123"); + EXPECT_EQ(chrono::format("%H%M", minutes{90}), "0130"); + EXPECT_EQ(chrono::format("%H%M", minutes{-90}), "-0130"); +} + +TEST(TimeTest, ReuseZonedFormat) { + namespace chrono = arrow::internal::chrono; + using std::chrono::microseconds; + const arrow::internal::OffsetZone zone{std::chrono::minutes{60}}; + const chrono::ZonedFormat format{"{%F %T} %Q %q %J"}; + for (const auto& [count, expected] : + {std::pair{1, "{1970-01-01 01:00:00.000001} 3600000001 \xC2\xB5s %J"}, + std::pair{-1, "{1970-01-01 00:59:59.999999} 3599999999 \xC2\xB5s %J"}}) { + std::ostringstream out; + out.imbue(std::locale::classic()); + const chrono::zoned_time value{ + zone, chrono::sys_time{microseconds{count}}}; + chrono::to_stream(out, format, value); + EXPECT_EQ(out.str(), expected); + } +} + +TEST(TimeTest, ZonedFormatStreamState) { + namespace chrono = arrow::internal::chrono; + using std::chrono::seconds; + const chrono::ZonedFormat format{"%F %T"}; + const chrono::zoned_time value{ + arrow::internal::OffsetZone{std::chrono::minutes{0}}, + chrono::sys_time{seconds{0}}}; + std::ostringstream out; + out.imbue(std::locale::classic()); + out << std::hex << std::showbase; + out.precision(3); + out.width(30); + out.fill('*'); + const auto flags = out.flags(); + EXPECT_EQ(&chrono::to_stream(out, format, value), &out); + EXPECT_EQ(out.str(), "1970-01-01 00:00:00"); + EXPECT_EQ(out.flags(), flags); + EXPECT_EQ(out.precision(), 3); + EXPECT_EQ(out.width(), 30); + EXPECT_EQ(out.fill(), '*'); + + // A streambuf with no put area rejects every write. + class FailingBuffer : public std::streambuf { + } buffer; + std::ostream failing(&buffer); + failing.exceptions(std::ios::badbit | std::ios::failbit); + EXPECT_THROW(chrono::to_stream(failing, format, value), std::ios_base::failure); +} + TEST(TimeTest, ConvertTimestampValue) { auto convert = [](TimeUnit::type in, TimeUnit::type out, int64_t value) { return ConvertTimestampValue(timestamp(in), timestamp(out), value).ValueOrDie(); diff --git a/cpp/src/arrow/vendored/datetime.cpp b/cpp/src/arrow/vendored/datetime.cpp index b4f9dd368300..ffc3b14e2fa3 100644 --- a/cpp/src/arrow/vendored/datetime.cpp +++ b/cpp/src/arrow/vendored/datetime.cpp @@ -21,6 +21,5 @@ // Standard-library builds must not compile a second timezone implementation. #if !ARROW_USE_STD_CHRONO # include "datetime/visibility.h" - # include "datetime/tz.cpp" #endif diff --git a/cpp/src/arrow/vendored/datetime_ios.mm b/cpp/src/arrow/vendored/datetime_ios.mm index 35f0fe08d729..c68ffc19bac8 100644 --- a/cpp/src/arrow/vendored/datetime_ios.mm +++ b/cpp/src/arrow/vendored/datetime_ios.mm @@ -15,10 +15,7 @@ // specific language governing permissions and limitations // under the License. -// Evaluate automatic backend selection only when the build has not selected one. -#ifndef ARROW_USE_STD_CHRONO -# include "arrow/util/chrono_config_internal.h" -#endif +#include "arrow/util/chrono_config_internal.h" #if !ARROW_USE_STD_CHRONO # include "datetime/ios.mm" From 2ab86d6aabf1575a4bc16e5cd60fac50b73b2108 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Thu, 17 Sep 2026 17:15:13 +0200 Subject: [PATCH 05/12] GH-51215: [C++] Move strftime formatting out of the chrono shim --- .../compute/kernels/scalar_temporal_test.cc | 48 +++++ .../arrow/compute/kernels/temporal_internal.h | 181 +++++++++++++++++- cpp/src/arrow/util/chrono_internal.h | 178 +---------------- cpp/src/arrow/util/time_test.cc | 50 +---- 4 files changed, 229 insertions(+), 228 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index aaa4aefa6c54..e7ecf0349988 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -15,12 +15,14 @@ // specific language governing permissions and limitations // under the License. +#include #include #include #include "arrow/compute/api_scalar.h" #include "arrow/compute/cast.h" +#include "arrow/compute/kernels/temporal_internal.h" #include "arrow/compute/kernels/test_util_internal.h" #include "arrow/testing/gtest_util.h" #include "arrow/testing/matchers.h" @@ -2003,6 +2005,52 @@ TEST_F(ScalarTemporalTest, TestAssumeTimezoneNonexistent) { &options_earliest); } +TEST(StrftimeFormatterTest, ReuseFormatter) { + namespace chrono = arrow::internal::chrono; + using std::chrono::microseconds; + const arrow::internal::OffsetZone zone{std::chrono::minutes{60}}; + const internal::StrftimeFormatter formatter{"{%F %T} %Q %q %J"}; + for (const auto& [count, expected] : + {std::pair{1, "{1970-01-01 01:00:00.000001} 3600000001 \xC2\xB5s %J"}, + std::pair{-1, "{1970-01-01 00:59:59.999999} 3599999999 \xC2\xB5s %J"}}) { + std::ostringstream out; + out.imbue(std::locale::classic()); + const chrono::zoned_time value{ + zone, chrono::sys_time{microseconds{count}}}; + formatter.Format(out, value); + EXPECT_EQ(out.str(), expected); + } +} + +TEST(StrftimeFormatterTest, StreamState) { + namespace chrono = arrow::internal::chrono; + using std::chrono::seconds; + const internal::StrftimeFormatter formatter{"%F %T"}; + const chrono::zoned_time value{ + arrow::internal::OffsetZone{std::chrono::minutes{0}}, + chrono::sys_time{seconds{0}}}; + std::ostringstream out; + out.imbue(std::locale::classic()); + out << std::hex << std::showbase; + out.precision(3); + out.width(30); + out.fill('*'); + const auto flags = out.flags(); + EXPECT_EQ(&formatter.Format(out, value), &out); + EXPECT_EQ(out.str(), "1970-01-01 00:00:00"); + EXPECT_EQ(out.flags(), flags); + EXPECT_EQ(out.precision(), 3); + EXPECT_EQ(out.width(), 30); + EXPECT_EQ(out.fill(), '*'); + + // A streambuf with no put area rejects every write. + class FailingBuffer : public std::streambuf { + } buffer; + std::ostream failing(&buffer); + failing.exceptions(std::ios::badbit | std::ios::failbit); + EXPECT_THROW(formatter.Format(failing, value), std::ios_base::failure); +} + TEST_F(ScalarTemporalTest, StrftimeFormatSyntax) { const auto type = timestamp(TimeUnit::MILLI, "UTC"); const char* input = R"(["1970-01-01T00:00:00.123", null])"; diff --git a/cpp/src/arrow/compute/kernels/temporal_internal.h b/cpp/src/arrow/compute/kernels/temporal_internal.h index 5dddf6b00244..1e9c7c79bc88 100644 --- a/cpp/src/arrow/compute/kernels/temporal_internal.h +++ b/cpp/src/arrow/compute/kernels/temporal_internal.h @@ -19,12 +19,23 @@ #include #include +#include +#include +#include +#include +#include #include "arrow/compute/api_scalar.h" #include "arrow/compute/kernels/codegen_internal.h" #include "arrow/util/date_internal.h" #include "arrow/util/value_parsing.h" +#if ARROW_USE_STD_CHRONO +# include +# include +# include +#endif + namespace arrow::compute::internal { namespace chrono = arrow::internal::chrono; @@ -163,15 +174,179 @@ struct ZonedLocalizer { local_days ConvertDays(sys_days d) const { return local_days(year_month_day(d)); } }; +#if ARROW_USE_STD_CHRONO +namespace detail { + +// Argument positions passed to std::vformat_to by StrftimeFormatter below. +enum class FormatArgument : char { + ZonedTime = '0', + TimeOfDay = '1', + TimeOfDayCount = '2', +}; + +inline void AppendEscapedLiteral(std::string* out, char value) { + out->push_back(value); + if (value == '{' || value == '}') { + out->push_back(value); + } +} + +// These are the directives accepted by Arrow's existing strftime syntax. Treat +// all others as literals to preserve compatibility. +inline bool IsSupportedStrftimeSpecifier(char modifier, char specifier) { + const auto contains = [specifier](std::string_view candidates) { + return candidates.find(specifier) != std::string_view::npos; + }; + if (modifier == '\0') { + return contains("aAbBhcCxdeDFgGHIjmMprRSTuUVWwXyYzZ"); + } + if (modifier == 'E') { + return contains("cCxXyYz"); + } + if (modifier == 'O') { + return contains("deHImMSuUVwWyz"); + } + return false; +} + +inline void AppendChronoField(std::string* out, FormatArgument argument, char specifier, + char modifier = '\0') { + *out += {'{', static_cast(argument), ':', 'L', '%'}; + if (modifier != '\0') out->push_back(modifier); + *out += {specifier, '}'}; +} + +inline void AppendLocalizedField(std::string* out, FormatArgument argument) { + *out += {'{', static_cast(argument), ':', 'L', '}'}; +} + +inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) { + std::string out; + while (*fmt != '\0') { + if (*fmt != '%') { + AppendEscapedLiteral(&out, *fmt++); + continue; + } + + ++fmt; + if (*fmt == '\0') { + AppendEscapedLiteral(&out, '%'); + break; + } + + char modifier = '\0'; + if (*fmt == 'E' || *fmt == 'O') { + modifier = *fmt++; + if (*fmt == '\0') { + AppendEscapedLiteral(&out, '%'); + AppendEscapedLiteral(&out, modifier); + break; + } + } + const char specifier = *fmt++; + + if (modifier == '\0') { + switch (specifier) { + case '%': + AppendEscapedLiteral(&out, '%'); + continue; + case 'n': + AppendEscapedLiteral(&out, '\n'); + continue; + case 't': + AppendEscapedLiteral(&out, '\t'); + continue; + case 'Q': + // Formatting a duration's %Q does not consistently apply the numeric locale. + AppendLocalizedField(&out, FormatArgument::TimeOfDayCount); + continue; + case 'q': + if (use_microseconds_suffix) { + // Some standard libraries use "us"; Arrow uses the micro sign. + out += "\xC2\xB5s"; + } else { + AppendChronoField(&out, FormatArgument::TimeOfDay, specifier); + } + continue; + default: + break; + } + } + +# if defined(__GLIBCXX__) + if (modifier == 'O' && specifier == 'V') { + // libstdc++ does not yet accept %OV; use its equivalent base representation. + AppendChronoField(&out, FormatArgument::ZonedTime, specifier); + continue; + } +# endif + + if (IsSupportedStrftimeSpecifier(modifier, specifier)) { + AppendChronoField(&out, FormatArgument::ZonedTime, specifier, modifier); + } else { + AppendEscapedLiteral(&out, '%'); + if (modifier != '\0') AppendEscapedLiteral(&out, modifier); + AppendEscapedLiteral(&out, specifier); + } + } + return out; +} + +} // namespace detail +#endif + +// Prepare Arrow's strftime syntax once and reuse it for values with the same +// precision. The timezone belongs to each value, not to this formatter. +template +class StrftimeFormatter { + public: + explicit StrftimeFormatter(const std::string& format) { +#if ARROW_USE_STD_CHRONO + using Precision = typename chrono::zoned_time::duration; + format_ = detail::ToChronoFormat( + format.c_str(), std::ratio_equal_v); +#else + format_ = format; +#endif + } + + // Literal braces and unsupported directives remain literal; %Q/%q use local + // time of day rather than elapsed time since the epoch. + template + std::ostream& Format(std::ostream& os, + const chrono::zoned_time& value) const { +#if ARROW_USE_STD_CHRONO + const auto local_time = value.get_local_time(); + const auto local_day = std::chrono::floor(local_time); + const auto time_of_day = local_time - local_day; + const auto time_of_day_count = time_of_day.count(); + + const std::ostream::sentry sentry(os); + if (sentry) { + const auto end = + std::vformat_to(std::ostreambuf_iterator(os), os.getloc(), format_, + std::make_format_args(value, time_of_day, time_of_day_count)); + if (end.failed()) os.setstate(std::ios::badbit); + } + return os; +#else + return arrow_vendored::date::to_stream(os, format_.c_str(), value); +#endif + } + + private: + std::string format_; +}; + template struct TimestampFormatter { - const chrono::ZonedFormat format; + const StrftimeFormatter formatter; const ArrowTimeZone tz; std::ostringstream bufstream; explicit TimestampFormatter(const std::string& format, const ArrowTimeZone time_zone, const std::locale& locale) - : format(format), tz(time_zone) { + : formatter(format), tz(time_zone) { bufstream.imbue(locale); // Propagate errors as C++ exceptions (to get an actual error message) bufstream.exceptions(std::ios::failbit | std::ios::badbit); @@ -182,7 +357,7 @@ struct TimestampFormatter { const auto timepoint = sys_time(Duration{arg}); auto format_zoned_time = [&](auto&& zt) { try { - chrono::to_stream(bufstream, format, zt); + formatter.Format(bufstream, zt); return Status::OK(); } catch (const std::runtime_error& ex) { bufstream.clear(); diff --git a/cpp/src/arrow/util/chrono_internal.h b/cpp/src/arrow/util/chrono_internal.h index bd59bbe3e434..1b4ca1e7ef20 100644 --- a/cpp/src/arrow/util/chrono_internal.h +++ b/cpp/src/arrow/util/chrono_internal.h @@ -38,10 +38,7 @@ #if ARROW_USE_STD_CHRONO // Use C++20 standard library chrono # include -# include # include -# include -# include #else // Use vendored Howard Hinnant date library # include "arrow/vendored/datetime.h" @@ -127,163 +124,9 @@ inline const time_zone* locate_zone(std::string_view tz_name) { inline const time_zone* current_zone() { return std::chrono::current_zone(); } -namespace detail { - -// Argument positions passed to std::vformat_to by to_stream below. -enum class FormatArgument : char { - ZonedTime = '0', - TimeOfDay = '1', - TimeOfDayCount = '2', -}; - -inline void AppendEscapedLiteral(std::string* out, char value) { - out->push_back(value); - if (value == '{' || value == '}') { - out->push_back(value); - } -} - -// These are the directives accepted by Arrow's existing strftime syntax. Treat -// all others as literals to preserve compatibility. -inline bool IsSupportedStrftimeSpecifier(char modifier, char specifier) { - const auto contains = [specifier](std::string_view candidates) { - return candidates.find(specifier) != std::string_view::npos; - }; - if (modifier == '\0') { - return contains("aAbBhcCxdeDFgGHIjmMprRSTuUVWwXyYzZ"); - } - if (modifier == 'E') { - return contains("cCxXyYz"); - } - if (modifier == 'O') { - return contains("deHImMSuUVwWyz"); - } - return false; -} - -inline void AppendChronoField(std::string* out, FormatArgument argument, char specifier, - char modifier = '\0') { - *out += {'{', static_cast(argument), ':', 'L', '%'}; - if (modifier != '\0') out->push_back(modifier); - *out += {specifier, '}'}; -} - -inline void AppendLocalizedField(std::string* out, FormatArgument argument) { - *out += {'{', static_cast(argument), ':', 'L', '}'}; -} - -inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) { - std::string out; - while (*fmt != '\0') { - if (*fmt != '%') { - AppendEscapedLiteral(&out, *fmt++); - continue; - } - - ++fmt; - if (*fmt == '\0') { - AppendEscapedLiteral(&out, '%'); - break; - } - - char modifier = '\0'; - if (*fmt == 'E' || *fmt == 'O') { - modifier = *fmt++; - if (*fmt == '\0') { - AppendEscapedLiteral(&out, '%'); - AppendEscapedLiteral(&out, modifier); - break; - } - } - const char specifier = *fmt++; - - if (modifier == '\0') { - switch (specifier) { - case '%': - AppendEscapedLiteral(&out, '%'); - continue; - case 'n': - AppendEscapedLiteral(&out, '\n'); - continue; - case 't': - AppendEscapedLiteral(&out, '\t'); - continue; - case 'Q': - // Formatting a duration's %Q does not consistently apply the numeric locale. - AppendLocalizedField(&out, FormatArgument::TimeOfDayCount); - continue; - case 'q': - if (use_microseconds_suffix) { - // Some standard libraries use "us"; Arrow uses the micro sign. - out += "\xC2\xB5s"; - } else { - AppendChronoField(&out, FormatArgument::TimeOfDay, specifier); - } - continue; - default: - break; - } - } - -# if defined(__GLIBCXX__) - if (modifier == 'O' && specifier == 'V') { - // libstdc++ does not yet accept %OV; use its equivalent base representation. - AppendChronoField(&out, FormatArgument::ZonedTime, specifier); - continue; - } -# endif - - if (IsSupportedStrftimeSpecifier(modifier, specifier)) { - AppendChronoField(&out, FormatArgument::ZonedTime, specifier, modifier); - } else { - AppendEscapedLiteral(&out, '%'); - if (modifier != '\0') AppendEscapedLiteral(&out, modifier); - AppendEscapedLiteral(&out, specifier); - } - } - return out; -} - -} // namespace detail - -// Prepare once and reuse for a sequence of zoned times with the same precision. -template -class ZonedFormat { - public: - explicit ZonedFormat(const std::string& format) - : value_(detail::ToChronoFormat( - format.c_str(), std::ratio_equal_v)) { - } - - const std::string& value() const { return value_; } - - private: - using Precision = typename zoned_time::duration; - std::string value_; -}; - -// Literal braces and unsupported directives remain literal; %Q/%q use local time -// of day rather than elapsed time since the epoch. -template -std::ostream& to_stream(std::ostream& os, const ZonedFormat& format, - const std::chrono::zoned_time& zt) { - const auto local_time = zt.get_local_time(); - const auto local_day = std::chrono::floor(local_time); - const auto time_of_day = local_time - local_day; - const auto time_of_day_count = time_of_day.count(); - - const std::ostream::sentry sentry(os); - if (sentry) { - const auto end = - std::vformat_to(std::ostreambuf_iterator(os), os.getloc(), format.value(), - std::make_format_args(zt, time_of_day, time_of_day_count)); - if (end.failed()) os.setstate(std::ios::badbit); - } - return os; -} - // Format durations and unzoned time points using internal format strings shared -// by both backends. User-supplied strftime syntax uses ZonedFormat and to_stream. +// by both backends. User-supplied strftime syntax uses StrftimeFormatter in +// arrow/compute/kernels/temporal_internal.h. // Keep each format in one replacement field to avoid repeating duration signs. template std::string format(const char* fmt, const Temporal& value) { @@ -378,23 +221,6 @@ using vendored::set_install; // Formatting support using vendored::format; -template -class ZonedFormat { - public: - explicit ZonedFormat(const std::string& format) : value_(format) {} - - const std::string& value() const { return value_; } - - private: - std::string value_; -}; - -template -std::ostream& to_stream(std::ostream& os, const ZonedFormat& format, - const vendored::zoned_time& zt) { - return vendored::to_stream(os, format.value().c_str(), zt); -} - inline constexpr vendored::month jan = vendored::jan; inline constexpr vendored::month dec = vendored::dec; diff --git a/cpp/src/arrow/util/time_test.cc b/cpp/src/arrow/util/time_test.cc index 33cd92bf2f2a..a79aff9db2b4 100644 --- a/cpp/src/arrow/util/time_test.cc +++ b/cpp/src/arrow/util/time_test.cc @@ -17,10 +17,8 @@ #include -#include - #include "arrow/testing/gtest_util.h" -#include "arrow/util/date_internal.h" +#include "arrow/util/chrono_internal.h" #include "arrow/util/time.h" namespace arrow { @@ -39,52 +37,6 @@ TEST(TimeTest, ChronoFormats) { EXPECT_EQ(chrono::format("%H%M", minutes{-90}), "-0130"); } -TEST(TimeTest, ReuseZonedFormat) { - namespace chrono = arrow::internal::chrono; - using std::chrono::microseconds; - const arrow::internal::OffsetZone zone{std::chrono::minutes{60}}; - const chrono::ZonedFormat format{"{%F %T} %Q %q %J"}; - for (const auto& [count, expected] : - {std::pair{1, "{1970-01-01 01:00:00.000001} 3600000001 \xC2\xB5s %J"}, - std::pair{-1, "{1970-01-01 00:59:59.999999} 3599999999 \xC2\xB5s %J"}}) { - std::ostringstream out; - out.imbue(std::locale::classic()); - const chrono::zoned_time value{ - zone, chrono::sys_time{microseconds{count}}}; - chrono::to_stream(out, format, value); - EXPECT_EQ(out.str(), expected); - } -} - -TEST(TimeTest, ZonedFormatStreamState) { - namespace chrono = arrow::internal::chrono; - using std::chrono::seconds; - const chrono::ZonedFormat format{"%F %T"}; - const chrono::zoned_time value{ - arrow::internal::OffsetZone{std::chrono::minutes{0}}, - chrono::sys_time{seconds{0}}}; - std::ostringstream out; - out.imbue(std::locale::classic()); - out << std::hex << std::showbase; - out.precision(3); - out.width(30); - out.fill('*'); - const auto flags = out.flags(); - EXPECT_EQ(&chrono::to_stream(out, format, value), &out); - EXPECT_EQ(out.str(), "1970-01-01 00:00:00"); - EXPECT_EQ(out.flags(), flags); - EXPECT_EQ(out.precision(), 3); - EXPECT_EQ(out.width(), 30); - EXPECT_EQ(out.fill(), '*'); - - // A streambuf with no put area rejects every write. - class FailingBuffer : public std::streambuf { - } buffer; - std::ostream failing(&buffer); - failing.exceptions(std::ios::badbit | std::ios::failbit); - EXPECT_THROW(chrono::to_stream(failing, format, value), std::ios_base::failure); -} - TEST(TimeTest, ConvertTimestampValue) { auto convert = [](TimeUnit::type in, TimeUnit::type out, int64_t value) { return ConvertTimestampValue(timestamp(in), timestamp(out), value).ValueOrDie(); From 5781ee3cabadaa3a5f032623cb218f9a4a87fbf1 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Thu, 17 Sep 2026 19:36:03 +0200 Subject: [PATCH 06/12] Fold StrftimeFormatter into TimestampFormatter --- .../compute/kernels/scalar_temporal_test.cc | 43 +++++------ .../arrow/compute/kernels/temporal_internal.h | 72 +++++++------------ cpp/src/arrow/util/chrono_internal.h | 2 +- cpp/src/arrow/util/logger_test.cc | 16 +---- 4 files changed, 47 insertions(+), 86 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index e7ecf0349988..9e9ad9895f3a 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -2005,39 +2005,30 @@ TEST_F(ScalarTemporalTest, TestAssumeTimezoneNonexistent) { &options_earliest); } -TEST(StrftimeFormatterTest, ReuseFormatter) { - namespace chrono = arrow::internal::chrono; - using std::chrono::microseconds; +TEST(TimestampFormatterTest, ReuseFormatter) { const arrow::internal::OffsetZone zone{std::chrono::minutes{60}}; - const internal::StrftimeFormatter formatter{"{%F %T} %Q %q %J"}; + internal::TimestampFormatter formatter{ + "{%F %T} %Q %q %J", zone, std::locale::classic()}; for (const auto& [count, expected] : {std::pair{1, "{1970-01-01 01:00:00.000001} 3600000001 \xC2\xB5s %J"}, std::pair{-1, "{1970-01-01 00:59:59.999999} 3599999999 \xC2\xB5s %J"}}) { - std::ostringstream out; - out.imbue(std::locale::classic()); - const chrono::zoned_time value{ - zone, chrono::sys_time{microseconds{count}}}; - formatter.Format(out, value); - EXPECT_EQ(out.str(), expected); + ASSERT_OK_AND_ASSIGN(auto result, formatter(count)); + EXPECT_EQ(result, expected); } } -TEST(StrftimeFormatterTest, StreamState) { - namespace chrono = arrow::internal::chrono; - using std::chrono::seconds; - const internal::StrftimeFormatter formatter{"%F %T"}; - const chrono::zoned_time value{ - arrow::internal::OffsetZone{std::chrono::minutes{0}}, - chrono::sys_time{seconds{0}}}; - std::ostringstream out; - out.imbue(std::locale::classic()); +TEST(TimestampFormatterTest, StreamState) { + internal::TimestampFormatter formatter{ + "%F %T", arrow::internal::OffsetZone{std::chrono::minutes{0}}, + std::locale::classic()}; + auto& out = formatter.bufstream; out << std::hex << std::showbase; out.precision(3); out.width(30); out.fill('*'); const auto flags = out.flags(); - EXPECT_EQ(&formatter.Format(out, value), &out); - EXPECT_EQ(out.str(), "1970-01-01 00:00:00"); + ASSERT_OK_AND_ASSIGN(auto result, formatter(0)); + EXPECT_EQ(result, "1970-01-01 00:00:00"); EXPECT_EQ(out.flags(), flags); EXPECT_EQ(out.precision(), 3); EXPECT_EQ(out.width(), 30); @@ -2046,9 +2037,13 @@ TEST(StrftimeFormatterTest, StreamState) { // A streambuf with no put area rejects every write. class FailingBuffer : public std::streambuf { } buffer; - std::ostream failing(&buffer); - failing.exceptions(std::ios::badbit | std::ios::failbit); - EXPECT_THROW(formatter.Format(failing, value), std::ios_base::failure); + auto& stream = static_cast(out); + auto* original = stream.rdbuf(&buffer); + EXPECT_RAISES_WITH_MESSAGE_THAT( + Invalid, testing::HasSubstr("Failed formatting timestamp"), formatter(0)); + stream.rdbuf(original); + ASSERT_OK_AND_ASSIGN(result, formatter(0)); + EXPECT_EQ(result, "1970-01-01 00:00:00"); } TEST_F(ScalarTemporalTest, StrftimeFormatSyntax) { diff --git a/cpp/src/arrow/compute/kernels/temporal_internal.h b/cpp/src/arrow/compute/kernels/temporal_internal.h index 1e9c7c79bc88..ff0bc7ba7a4e 100644 --- a/cpp/src/arrow/compute/kernels/temporal_internal.h +++ b/cpp/src/arrow/compute/kernels/temporal_internal.h @@ -177,7 +177,7 @@ struct ZonedLocalizer { #if ARROW_USE_STD_CHRONO namespace detail { -// Argument positions passed to std::vformat_to by StrftimeFormatter below. +// Argument positions passed to std::vformat_to by TimestampFormatter below. enum class FormatArgument : char { ZonedTime = '0', TimeOfDay = '1', @@ -295,58 +295,21 @@ inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) } // namespace detail #endif -// Prepare Arrow's strftime syntax once and reuse it for values with the same -// precision. The timezone belongs to each value, not to this formatter. -template -class StrftimeFormatter { - public: - explicit StrftimeFormatter(const std::string& format) { -#if ARROW_USE_STD_CHRONO - using Precision = typename chrono::zoned_time::duration; - format_ = detail::ToChronoFormat( - format.c_str(), std::ratio_equal_v); -#else - format_ = format; -#endif - } - - // Literal braces and unsupported directives remain literal; %Q/%q use local - // time of day rather than elapsed time since the epoch. - template - std::ostream& Format(std::ostream& os, - const chrono::zoned_time& value) const { -#if ARROW_USE_STD_CHRONO - const auto local_time = value.get_local_time(); - const auto local_day = std::chrono::floor(local_time); - const auto time_of_day = local_time - local_day; - const auto time_of_day_count = time_of_day.count(); - - const std::ostream::sentry sentry(os); - if (sentry) { - const auto end = - std::vformat_to(std::ostreambuf_iterator(os), os.getloc(), format_, - std::make_format_args(value, time_of_day, time_of_day_count)); - if (end.failed()) os.setstate(std::ios::badbit); - } - return os; -#else - return arrow_vendored::date::to_stream(os, format_.c_str(), value); -#endif - } - - private: - std::string format_; -}; - template struct TimestampFormatter { - const StrftimeFormatter formatter; + std::string format; const ArrowTimeZone tz; std::ostringstream bufstream; explicit TimestampFormatter(const std::string& format, const ArrowTimeZone time_zone, const std::locale& locale) - : formatter(format), tz(time_zone) { + : format(format), tz(time_zone) { +#if ARROW_USE_STD_CHRONO + // Translate strftime syntax once, not for every timestamp. + using Precision = typename chrono::zoned_time::duration; + this->format = detail::ToChronoFormat( + format.c_str(), std::ratio_equal_v); +#endif bufstream.imbue(locale); // Propagate errors as C++ exceptions (to get an actual error message) bufstream.exceptions(std::ios::failbit | std::ios::badbit); @@ -357,7 +320,22 @@ struct TimestampFormatter { const auto timepoint = sys_time(Duration{arg}); auto format_zoned_time = [&](auto&& zt) { try { - formatter.Format(bufstream, zt); +#if ARROW_USE_STD_CHRONO + // %Q/%q refer to local time of day rather than elapsed time since the epoch. + const auto local_time = zt.get_local_time(); + const auto local_day = std::chrono::floor(local_time); + const auto time_of_day = local_time - local_day; + const auto time_of_day_count = time_of_day.count(); + const std::ostream::sentry sentry(bufstream); + if (sentry) { + const auto end = std::vformat_to( + std::ostreambuf_iterator(bufstream), bufstream.getloc(), format, + std::make_format_args(zt, time_of_day, time_of_day_count)); + if (end.failed()) bufstream.setstate(std::ios::badbit); + } +#else + arrow_vendored::date::to_stream(bufstream, format.c_str(), zt); +#endif return Status::OK(); } catch (const std::runtime_error& ex) { bufstream.clear(); diff --git a/cpp/src/arrow/util/chrono_internal.h b/cpp/src/arrow/util/chrono_internal.h index 1b4ca1e7ef20..c4a1d1ad3760 100644 --- a/cpp/src/arrow/util/chrono_internal.h +++ b/cpp/src/arrow/util/chrono_internal.h @@ -125,7 +125,7 @@ inline const time_zone* locate_zone(std::string_view tz_name) { inline const time_zone* current_zone() { return std::chrono::current_zone(); } // Format durations and unzoned time points using internal format strings shared -// by both backends. User-supplied strftime syntax uses StrftimeFormatter in +// by both backends. User-supplied strftime syntax uses TimestampFormatter in // arrow/compute/kernels/temporal_internal.h. // Keep each format in one replacement field to avoid repeating duration signs. template diff --git a/cpp/src/arrow/util/logger_test.cc b/cpp/src/arrow/util/logger_test.cc index a8de9a7611da..0faea81a598f 100644 --- a/cpp/src/arrow/util/logger_test.cc +++ b/cpp/src/arrow/util/logger_test.cc @@ -23,10 +23,8 @@ #include "arrow/testing/gtest_util.h" #include "arrow/util/logger.h" -// Emit log via the default logger. In unity builds, io_util_test.cc can include -// Windows headers before this file. Paste LEVEL here so ERROR is not expanded to -// 0 before selecting ARROW_LOGGER_ERROR. -#define DO_LOG(LEVEL, ...) ARROW_LOGGER_##LEVEL("", __VA_ARGS__) +// Emit log via the default logger +#define DO_LOG(LEVEL, ...) ARROW_LOGGER_CALL("", LEVEL, __VA_ARGS__) namespace arrow { namespace util { @@ -60,16 +58,6 @@ struct OstreamableTracer { } // namespace -// Reproduce the Windows macro collision on every platform, without leaking the -// test macro into other sources in a unity build. -#pragma push_macro("ERROR") -#undef ERROR -#define ERROR 0 -TEST(LoggerTest, ErrorMacroCollision) { - DO_LOG(ERROR, "Logging with the Windows ERROR macro defined"); -} -#pragma pop_macro("ERROR") - TEST(LoggerTest, Basics) { // Basic tests using the default logger DO_LOG(ERROR); From e8de86cab477d8795204b426c7af5f3713c52518 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Thu, 17 Sep 2026 20:01:22 +0200 Subject: [PATCH 07/12] compatible directives should share chrono field --- .../compute/kernels/scalar_temporal_test.cc | 33 ++++++++++-- .../arrow/compute/kernels/temporal_internal.h | 50 ++++++++++++++----- 2 files changed, 67 insertions(+), 16 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index 9e9ad9895f3a..008f65cf1a22 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -2005,6 +2005,22 @@ TEST_F(ScalarTemporalTest, TestAssumeTimezoneNonexistent) { &options_earliest); } +#if ARROW_USE_STD_CHRONO +TEST(TimestampFormatterTest, CoalesceChronoFields) { + using internal::detail::ToChronoFormat; + EXPECT_EQ(ToChronoFormat(StrftimeOptions::kDefaultFormat, false), + "{0:L%Y-%m-%dT%H:%M:%S}"); + EXPECT_EQ(ToChronoFormat("%Y%m%d %H%M%S %Ez %Z", false), "{0:L%Y%m%d %H%M%S %Ez %Z}"); + EXPECT_EQ(ToChronoFormat("%Y%n%t%m", false), "{0:L%Y\n\t%m}"); + EXPECT_EQ(ToChronoFormat("%Y{%m}%d", false), "{0:L%Y}{{{0:L%m}}}{0:L%d}"); + EXPECT_EQ(ToChronoFormat("%Y%%%m%J%d%E", false), "{0:L%Y}%{0:L%m}%J{0:L%d}%E"); + EXPECT_EQ(ToChronoFormat("%Y %Q %m %q %d", false), + "{0:L%Y }{2:L} {0:L%m }{1:L%q} {0:L%d}"); + EXPECT_EQ(ToChronoFormat("%Y %Q %m %q %d", true), + "{0:L%Y }{2:L} {0:L%m }\xC2\xB5s {0:L%d}"); +} +#endif + TEST(TimestampFormatterTest, ReuseFormatter) { const arrow::internal::OffsetZone zone{std::chrono::minutes{60}}; internal::TimestampFormatter formatter{ @@ -2054,6 +2070,10 @@ TEST_F(ScalarTemporalTest, StrftimeFormatSyntax) { std::pair{"literal {%Y}", R"(["literal {1970}", null])"}, std::pair{"unmatched }%Y{", R"(["unmatched }1970{", null])"}, std::pair{"%Y}", R"(["1970}", null])"}, + std::pair{"%Y{%m}%d", R"(["1970{01}01", null])"}, + std::pair{"%Y%%%m%J%d%E", R"(["1970%01%J01%E", null])"}, + std::pair{"%Y%n%t%m", R"(["1970\n\t01", null])"}, + std::pair{"%Y %Q %m %q %d", R"(["1970 123 01 ms 01", null])"}, std::pair{"%Q %q %J %z %Z", R"(["123 ms %J +0000 UTC", null])"}, std::pair{"%% %n%t %Ez %Oz %OV %EJ end%", R"(["% \n\t +00:00 +00:00 01 %EJ end%", null])"}}) { @@ -2062,10 +2082,15 @@ TEST_F(ScalarTemporalTest, StrftimeFormatSyntax) { CheckScalarUnary("strftime", type, input, utf8(), expected, &options); } - const auto options = StrftimeOptions("%Q %q"); - CheckScalarUnary("strftime", timestamp(TimeUnit::MICRO, "UTC"), - R"(["1970-01-01T00:00:00.000001", null])", utf8(), - R"(["1 \u00b5s", null])", &options); + for (const auto& [format, expected] : + {std::pair{"%Q %q", R"(["1 \u00b5s", null])"}, + std::pair{"%Y %Q %m %q %d", R"(["1970 1 01 \u00b5s 01", null])"}}) { + SCOPED_TRACE(format); + const auto options = StrftimeOptions(format); + CheckScalarUnary("strftime", timestamp(TimeUnit::MICRO, "UTC"), + R"(["1970-01-01T00:00:00.000001", null])", utf8(), expected, + &options); + } } TEST_F(ScalarTemporalTest, StrftimeOffsetTimezone) { diff --git a/cpp/src/arrow/compute/kernels/temporal_internal.h b/cpp/src/arrow/compute/kernels/temporal_internal.h index ff0bc7ba7a4e..539224a747a4 100644 --- a/cpp/src/arrow/compute/kernels/temporal_internal.h +++ b/cpp/src/arrow/compute/kernels/temporal_internal.h @@ -222,15 +222,38 @@ inline void AppendLocalizedField(std::string* out, FormatArgument argument) { inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) { std::string out; + bool zoned_field_open = false; + const auto close_zoned_field = [&] { + if (zoned_field_open) { + out.push_back('}'); + zoned_field_open = false; + } + }; + const auto append_literal = [&](char value) { + // Braces and literal percent signs must stay outside chrono replacement fields. + if (value == '{' || value == '}' || value == '%') close_zoned_field(); + AppendEscapedLiteral(&out, value); + }; + const auto append_zoned_directive = [&](char specifier, char modifier = '\0') { + // Keep compatible directives and intervening literals in one field to avoid + // repeating timezone lookup and calendar decomposition for every directive. + if (!zoned_field_open) { + out += {'{', static_cast(FormatArgument::ZonedTime), ':', 'L'}; + zoned_field_open = true; + } + out.push_back('%'); + if (modifier != '\0') out.push_back(modifier); + out.push_back(specifier); + }; while (*fmt != '\0') { if (*fmt != '%') { - AppendEscapedLiteral(&out, *fmt++); + append_literal(*fmt++); continue; } ++fmt; if (*fmt == '\0') { - AppendEscapedLiteral(&out, '%'); + append_literal('%'); break; } @@ -238,8 +261,8 @@ inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) if (*fmt == 'E' || *fmt == 'O') { modifier = *fmt++; if (*fmt == '\0') { - AppendEscapedLiteral(&out, '%'); - AppendEscapedLiteral(&out, modifier); + append_literal('%'); + append_literal(modifier); break; } } @@ -248,19 +271,21 @@ inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) if (modifier == '\0') { switch (specifier) { case '%': - AppendEscapedLiteral(&out, '%'); + append_literal('%'); continue; case 'n': - AppendEscapedLiteral(&out, '\n'); + append_literal('\n'); continue; case 't': - AppendEscapedLiteral(&out, '\t'); + append_literal('\t'); continue; case 'Q': // Formatting a duration's %Q does not consistently apply the numeric locale. + close_zoned_field(); AppendLocalizedField(&out, FormatArgument::TimeOfDayCount); continue; case 'q': + close_zoned_field(); if (use_microseconds_suffix) { // Some standard libraries use "us"; Arrow uses the micro sign. out += "\xC2\xB5s"; @@ -276,19 +301,20 @@ inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) # if defined(__GLIBCXX__) if (modifier == 'O' && specifier == 'V') { // libstdc++ does not yet accept %OV; use its equivalent base representation. - AppendChronoField(&out, FormatArgument::ZonedTime, specifier); + append_zoned_directive(specifier); continue; } # endif if (IsSupportedStrftimeSpecifier(modifier, specifier)) { - AppendChronoField(&out, FormatArgument::ZonedTime, specifier, modifier); + append_zoned_directive(specifier, modifier); } else { - AppendEscapedLiteral(&out, '%'); - if (modifier != '\0') AppendEscapedLiteral(&out, modifier); - AppendEscapedLiteral(&out, specifier); + append_literal('%'); + if (modifier != '\0') append_literal(modifier); + append_literal(specifier); } } + close_zoned_field(); return out; } From dfb9ed7822958d87b6df17e60130e9a2249d3a39 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Thu, 17 Sep 2026 20:38:28 +0200 Subject: [PATCH 08/12] restore format as const --- .../arrow/compute/kernels/temporal_internal.h | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/temporal_internal.h b/cpp/src/arrow/compute/kernels/temporal_internal.h index 539224a747a4..6ac6eb812b78 100644 --- a/cpp/src/arrow/compute/kernels/temporal_internal.h +++ b/cpp/src/arrow/compute/kernels/temporal_internal.h @@ -323,19 +323,24 @@ inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) template struct TimestampFormatter { - std::string format; - const ArrowTimeZone tz; - std::ostringstream bufstream; - - explicit TimestampFormatter(const std::string& format, const ArrowTimeZone time_zone, - const std::locale& locale) - : format(format), tz(time_zone) { + static std::string PrepareFormat(const std::string& format) { #if ARROW_USE_STD_CHRONO // Translate strftime syntax once, not for every timestamp. using Precision = typename chrono::zoned_time::duration; - this->format = detail::ToChronoFormat( + return detail::ToChronoFormat( format.c_str(), std::ratio_equal_v); +#else + return format; #endif + } + + const std::string format; + const ArrowTimeZone tz; + std::ostringstream bufstream; + + explicit TimestampFormatter(const std::string& format, const ArrowTimeZone time_zone, + const std::locale& locale) + : format(PrepareFormat(format)), tz(time_zone) { bufstream.imbue(locale); // Propagate errors as C++ exceptions (to get an actual error message) bufstream.exceptions(std::ios::failbit | std::ios::badbit); From 48be9cc5b875ff706acdedd8c172e7ec23e08cc5 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Wed, 30 Sep 2026 09:27:24 +0200 Subject: [PATCH 09/12] GH-51215: [C++][R][Python] Address chrono backend review concerns - R: skip set_timezone_database when the timezone database is managed by the OS or standard library, avoiding a startup error on MinGW builds - Clarify the Initialize() error message for standard-library backends - Document backend selection caveats (runtime libstdc++, MinGW, installed headers, vendored timezone symbols not exported) - Avoid a global namespace alias in gandiva's epoch_time_point.h - Report an error when TimestampFormatter's stream sentry fails --- .../arrow/compute/kernels/temporal_internal.h | 2 + cpp/src/arrow/config.cc | 5 ++- cpp/src/arrow/util/chrono_config_internal.h | 11 +++++ .../gandiva/precompiled/epoch_time_point.h | 45 ++++++++++--------- cpp/src/gandiva/precompiled/time.cc | 2 + .../precompiled/timestamp_arithmetic.cc | 2 + python/pyarrow/tests/test_misc.py | 4 +- r/src/config.cpp | 4 ++ 8 files changed, 51 insertions(+), 24 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/temporal_internal.h b/cpp/src/arrow/compute/kernels/temporal_internal.h index 6ac6eb812b78..54626117fd3e 100644 --- a/cpp/src/arrow/compute/kernels/temporal_internal.h +++ b/cpp/src/arrow/compute/kernels/temporal_internal.h @@ -363,6 +363,8 @@ struct TimestampFormatter { std::ostreambuf_iterator(bufstream), bufstream.getloc(), format, std::make_format_args(zt, time_of_day, time_of_day_count)); if (end.failed()) bufstream.setstate(std::ios::badbit); + } else { + bufstream.setstate(std::ios::badbit); } #else arrow_vendored::date::to_stream(bufstream, format.c_str(), zt); diff --git a/cpp/src/arrow/config.cc b/cpp/src/arrow/config.cc index 290c0db2f447..96d6b7856661 100644 --- a/cpp/src/arrow/config.cc +++ b/cpp/src/arrow/config.cc @@ -102,8 +102,9 @@ Status Initialize(const GlobalOptions& options) noexcept { timezone_db_path = options.timezone_db_path.value(); #else return Status::Invalid( - "Arrow was set to use OS timezone database at compile time, " - "so a downloaded database cannot be provided at runtime."); + "Arrow was set to use OS timezone database at compile time " + "(OS or the C++ standard library), so a downloaded database " + "cannot be provided at runtime."); #endif // !ARROW_CHRONO_USE_OS_TZDB } ARROW_UNSUPPRESS_DEPRECATION_WARNING diff --git a/cpp/src/arrow/util/chrono_config_internal.h b/cpp/src/arrow/util/chrono_config_internal.h index 3ebe27247c12..37d0b5ce7261 100644 --- a/cpp/src/arrow/util/chrono_config_internal.h +++ b/cpp/src/arrow/util/chrono_config_internal.h @@ -34,6 +34,17 @@ // On non-Windows, also select it for older libstdc++ versions because of // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116110 (fully fixed in GCC 16.2). // The datestamp distinguishes 16.2 (2026-08-07) from 16.1 and early snapshots. +// The fix lives in the libstdc++ shared library, so this header check assumes the +// runtime libstdc++ is at least as new as the one Arrow was compiled against. +// MinGW intentionally keeps using std::chrono with older libstdc++ versions, +// matching previous releases. +// +// This header is installed because public headers (formatting.h, value_parsing.h) +// include chrono_internal.h. The selection is then evaluated with the consumer's +// toolchain, which may differ from Arrow's, so installed headers must only use the +// header-only calendar APIs and never the timezone database functions. +// Likewise, standard-library builds do not compile the vendored timezone sources, +// so libarrow does not export arrow_vendored::date timezone functions. #ifndef ARROW_USE_STD_CHRONO # define ARROW_USE_STD_CHRONO 1 # if !defined(__cpp_lib_chrono) || __cpp_lib_chrono < 201907L diff --git a/cpp/src/gandiva/precompiled/epoch_time_point.h b/cpp/src/gandiva/precompiled/epoch_time_point.h index 781d588a51ac..9494d9d6b7b3 100644 --- a/cpp/src/gandiva/precompiled/epoch_time_point.h +++ b/cpp/src/gandiva/precompiled/epoch_time_point.h @@ -19,10 +19,8 @@ #include "arrow/util/chrono_internal.h" -namespace chrono = arrow::internal::chrono; - bool is_leap_year(int yy); -bool did_days_overflow(chrono::year_month_day ymd); +bool did_days_overflow(arrow::internal::chrono::year_month_day ymd); int last_possible_day_in_month(int month, int year); // A point of time measured in millis since epoch. @@ -39,16 +37,19 @@ class EpochTimePoint { int TmMon() const { return static_cast(YearMonthDay().month()) - 1; } int TmYday() const { - auto to_days = chrono::floor(tp_); - auto first_day_in_year = chrono::sys_days{YearMonthDay().year() / chrono::jan / 1}; + auto to_days = arrow::internal::chrono::floor(tp_); + auto first_day_in_year = arrow::internal::chrono::sys_days{ + YearMonthDay().year() / arrow::internal::chrono::jan / 1}; return (to_days - first_day_in_year).count(); } int TmMday() const { return static_cast(YearMonthDay().day()); } int TmWday() const { - auto to_days = chrono::floor(tp_); - return (chrono::weekday{to_days} - chrono::Sunday).count(); + auto to_days = arrow::internal::chrono::floor(tp_); + return (arrow::internal::chrono::weekday{to_days} - // NOLINT + arrow::internal::chrono::Sunday) + .count(); } int TmHour() const { return static_cast(TimeOfDay().hours().count()); } @@ -61,18 +62,19 @@ class EpochTimePoint { } EpochTimePoint AddYears(int num_years) const { - auto ymd = YearMonthDay() + chrono::years(num_years); - return EpochTimePoint((chrono::sys_days{ymd} + // NOLINT + auto ymd = YearMonthDay() + arrow::internal::chrono::years(num_years); + return EpochTimePoint((arrow::internal::chrono::sys_days{ymd} + // NOLINT TimeOfDay().to_duration()) .time_since_epoch()); } EpochTimePoint AddMonths(int num_months) const { - auto ymd = YearMonthDay() + chrono::months(num_months); + auto ymd = YearMonthDay() + arrow::internal::chrono::months(num_months); - EpochTimePoint tp = EpochTimePoint((chrono::sys_days{ymd} + // NOLINT - TimeOfDay().to_duration()) - .time_since_epoch()); + EpochTimePoint tp = + EpochTimePoint((arrow::internal::chrono::sys_days{ymd} + // NOLINT + TimeOfDay().to_duration()) + .time_since_epoch()); if (did_days_overflow(ymd)) { int days_to_offset = @@ -85,8 +87,8 @@ class EpochTimePoint { } EpochTimePoint AddDays(int num_days) const { - auto days_since_epoch = chrono::sys_days{YearMonthDay()} + // NOLINT - chrono::days(num_days); + auto days_since_epoch = arrow::internal::chrono::sys_days{YearMonthDay()} + // NOLINT + arrow::internal::chrono::days(num_days); return EpochTimePoint( (days_since_epoch + TimeOfDay().to_duration()).time_since_epoch()); } @@ -99,14 +101,17 @@ class EpochTimePoint { int64_t MillisSinceEpoch() const { return tp_.time_since_epoch().count(); } - chrono::hh_mm_ss TimeOfDay() const { - auto millis_since_midnight = tp_ - chrono::floor(tp_); - return chrono::hh_mm_ss{millis_since_midnight}; + arrow::internal::chrono::hh_mm_ss TimeOfDay() const { + auto millis_since_midnight = + tp_ - arrow::internal::chrono::floor(tp_); + return arrow::internal::chrono::hh_mm_ss{ + millis_since_midnight}; } private: - chrono::year_month_day YearMonthDay() const { - return chrono::year_month_day{chrono::floor(tp_)}; // NOLINT + arrow::internal::chrono::year_month_day YearMonthDay() const { + return arrow::internal::chrono::year_month_day{ + arrow::internal::chrono::floor(tp_)}; // NOLINT } std::chrono::time_point tp_; diff --git a/cpp/src/gandiva/precompiled/time.cc b/cpp/src/gandiva/precompiled/time.cc index f1c3a189b0a3..bf292ae8689a 100644 --- a/cpp/src/gandiva/precompiled/time.cc +++ b/cpp/src/gandiva/precompiled/time.cc @@ -19,6 +19,8 @@ #include +namespace chrono = arrow::internal::chrono; + extern "C" { #define __STDC_FORMAT_MACROS diff --git a/cpp/src/gandiva/precompiled/timestamp_arithmetic.cc b/cpp/src/gandiva/precompiled/timestamp_arithmetic.cc index 018af1d14af4..d513b960fa94 100644 --- a/cpp/src/gandiva/precompiled/timestamp_arithmetic.cc +++ b/cpp/src/gandiva/precompiled/timestamp_arithmetic.cc @@ -17,6 +17,8 @@ #include "./epoch_time_point.h" +namespace chrono = arrow::internal::chrono; + // The first row is for non-leap years static int days_in_a_month[2][12] = {{31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}, {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}}; diff --git a/python/pyarrow/tests/test_misc.py b/python/pyarrow/tests/test_misc.py index 856873f441b4..f9a0f6b01fbd 100644 --- a/python/pyarrow/tests/test_misc.py +++ b/python/pyarrow/tests/test_misc.py @@ -138,8 +138,8 @@ def import_arrow(): @pytest.mark.skipif(sys.platform == "win32", - reason="Path to timezone database is not configurable " - "on non-Windows platforms") + reason="Whether the timezone database path is configurable " + "on Windows depends on the C++ chrono backend") def test_set_timezone_db_path_non_windows(): # set_timezone_db_path raises an error on non-Windows platforms with pytest.warns(FutureWarning, match="deprecated"): diff --git a/r/src/config.cpp b/r/src/config.cpp index 950e29a168a1..a010c9896ef5 100644 --- a/r/src/config.cpp +++ b/r/src/config.cpp @@ -41,6 +41,10 @@ void set_timezone_database(cpp11::strings path) { cpp11::stop("Must provide a single path to the timezone database."); } + // Builds using the OS or standard library timezone database (e.g. MinGW with + // std::chrono) do not accept a path, so there is nothing to configure. + if (arrow::GetRuntimeInfo().using_os_timezone_db) return; + ARROW_SUPPRESS_DEPRECATION_WARNING arrow::GlobalOptions options; options.timezone_db_path = std::make_optional(paths[0]); From 14c50d855836c1eb6571d53f464c658ad996d4f4 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Wed, 30 Sep 2026 09:34:18 +0200 Subject: [PATCH 10/12] update R comment --- r/R/arrow-package.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/r/R/arrow-package.R b/r/R/arrow-package.R index 2706faee5cb1..5b955b360f0d 100644 --- a/r/R/arrow-package.R +++ b/r/R/arrow-package.R @@ -158,7 +158,9 @@ s3_finalizer <- new.env(parent = emptyenv()) # Use the tzdata package to configure the tzdata database on non-MSVC (i.e. # MinGW) systems. This fix was put in specifically for Winbuilder (See # GH-49866) but is needed for all non-MSVC systems. This code assumes the - # tzdata package is in Suggests. + # tzdata package is in Suggests. It has no effect when Arrow C++ uses the + # C++ standard library's timezone database (std::chrono), which does not + # accept a database path. if (!identical(build_info()[[2]], "MSVC")) { configure_tzdb() } From 6e9782e0569470dc4ed0c2bb405de7974e07e37b Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Wed, 30 Sep 2026 10:40:42 +0200 Subject: [PATCH 11/12] GH-51215: [C++] Avoid overflow in timestamp time-of-day formatting --- .../compute/kernels/scalar_temporal_test.cc | 27 +++++++++++++++++++ .../arrow/compute/kernels/temporal_internal.h | 9 +++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc index 008f65cf1a22..4ec6d9baa1f4 100644 --- a/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_temporal_test.cc @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include @@ -2033,6 +2034,32 @@ TEST(TimestampFormatterTest, ReuseFormatter) { } } +TEST(TimestampFormatterTest, NanosecondRangeLimits) { + const arrow::internal::OffsetZone zone{std::chrono::minutes{0}}; + internal::TimestampFormatter formatter{ + "%F %T %Q %q", zone, std::locale::classic()}; + for (const auto& [count, expected] : + {std::pair{std::numeric_limits::min() + 1000000000, + "1677-09-21 00:12:44.145224192 764145224192 ns"}, + std::pair{int64_t{-86400000000000}, "1969-12-31 00:00:00.000000000 0 ns"}, + std::pair{int64_t{-1}, "1969-12-31 23:59:59.999999999 86399999999999 ns"}, + std::pair{int64_t{0}, "1970-01-01 00:00:00.000000000 0 ns"}, + std::pair{int64_t{1}, "1970-01-01 00:00:00.000000001 1 ns"}, + std::pair{std::numeric_limits::max(), + "2262-04-11 23:47:16.854775807 85636854775807 ns"}}) { + SCOPED_TRACE(count); + ASSERT_OK_AND_ASSIGN(auto result, formatter(count)); + EXPECT_EQ(result, expected); + } + + // Formatting calendar fields at the exact lower bound can overflow within + // standard-library formatters. Test the time-of-day directives independently. + internal::TimestampFormatter count_formatter{ + "%Q %q", zone, std::locale::classic()}; + ASSERT_OK_AND_ASSIGN(auto result, count_formatter(std::numeric_limits::min())); + EXPECT_EQ(result, "763145224192 ns"); +} + TEST(TimestampFormatterTest, StreamState) { internal::TimestampFormatter formatter{ "%F %T", arrow::internal::OffsetZone{std::chrono::minutes{0}}, diff --git a/cpp/src/arrow/compute/kernels/temporal_internal.h b/cpp/src/arrow/compute/kernels/temporal_internal.h index 54626117fd3e..93f91ff18834 100644 --- a/cpp/src/arrow/compute/kernels/temporal_internal.h +++ b/cpp/src/arrow/compute/kernels/temporal_internal.h @@ -354,8 +354,13 @@ struct TimestampFormatter { #if ARROW_USE_STD_CHRONO // %Q/%q refer to local time of day rather than elapsed time since the epoch. const auto local_time = zt.get_local_time(); - const auto local_day = std::chrono::floor(local_time); - const auto time_of_day = local_time - local_day; + // Truncate toward zero, as in StringFormatter, so midnight + // remains representable even near the lower bound of nanosecond timestamps. + const auto local_day = + std::chrono::time_point_cast(local_time); + const auto time_of_day = local_day <= local_time + ? local_time - local_day + : std::chrono::days{1} - (local_day - local_time); const auto time_of_day_count = time_of_day.count(); const std::ostream::sentry sentry(bufstream); if (sentry) { From fd2bb12fb852830450056b234bf54c2fde67c183 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Wed, 30 Sep 2026 11:22:30 +0200 Subject: [PATCH 12/12] comment --- cpp/src/arrow/compute/kernels/temporal_internal.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/compute/kernels/temporal_internal.h b/cpp/src/arrow/compute/kernels/temporal_internal.h index 93f91ff18834..e9ee628ba730 100644 --- a/cpp/src/arrow/compute/kernels/temporal_internal.h +++ b/cpp/src/arrow/compute/kernels/temporal_internal.h @@ -300,7 +300,8 @@ inline std::string ToChronoFormat(const char* fmt, bool use_microseconds_suffix) # if defined(__GLIBCXX__) if (modifier == 'O' && specifier == 'V') { - // libstdc++ does not yet accept %OV; use its equivalent base representation. + // libstdc++ does not yet accept %OV; fall back to %V, losing any + // locale-specific alternative digits. append_zoned_directive(specifier); continue; }