From 09ac291500b53252c94c817cd9ce48336e81773e Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Tue, 9 Jun 2026 23:30:54 +0300 Subject: [PATCH 01/14] GH-41488: [C++][Python] Apply timestamp_parsers as fallback when parsing CSV date and time columns CSV columns explicitly typed as date32, date64, time32 or time64 could only be parsed from strict ISO-8601 strings; ConvertOptions::timestamp_parsers was consulted only for timestamp columns. Make the user-defined timestamp parsers act as a fallback for these column types: the built-in ISO-8601 parser is tried first (preserving existing behavior), then each configured parser in order. A timestamp produced by a fallback parser is floored to the day boundary for dates and reduced to the time of day for times, consistent with casting a timestamp to a date or time type. Type inference of date and time columns is deliberately unaffected: inference keeps using strict ISO-8601 parsing, otherwise a value with a time-of-day part could be inferred as a date and silently truncated. Also provide C-locale name tables to the vendored musl strptime used on Windows, where nl_langinfo() is unavailable: this makes %a/%A/%b/%B/%h/ %p/%c/%r/%x/%X work on Windows (matching musl's C locale), so that the month-name formats from the original issue reports parse on all platforms. Closes GH-28303. Co-Authored-By: Claude Fable 5 --- cpp/src/arrow/csv/converter.cc | 139 ++++++++++++++++++++++--- cpp/src/arrow/csv/converter_test.cc | 108 +++++++++++++++++++ cpp/src/arrow/csv/inference_internal.h | 23 +++- cpp/src/arrow/csv/options.h | 7 ++ cpp/src/arrow/vendored/musl/strptime.c | 19 +++- docs/source/cpp/csv.rst | 15 +++ python/pyarrow/_csv.pyx | 8 ++ python/pyarrow/tests/test_csv.py | 59 +++++++++++ 8 files changed, 361 insertions(+), 17 deletions(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index 0b7b9f5c8af5..b164e61786a4 100644 --- a/cpp/src/arrow/csv/converter.cc +++ b/cpp/src/arrow/csv/converter.cc @@ -440,6 +440,14 @@ struct SingleParserTimestampValueDecoder : public ValueDecoder { const TimestampParser& parser_; }; +std::vector GetTimestampParsers(const ConvertOptions& options) { + std::vector parsers(options.timestamp_parsers.size()); + for (size_t i = 0; i < options.timestamp_parsers.size(); ++i) { + parsers[i] = options.timestamp_parsers[i].get(); + } + return parsers; +} + struct MultipleParsersTimestampValueDecoder : public ValueDecoder { using value_type = int64_t; @@ -449,7 +457,7 @@ struct MultipleParsersTimestampValueDecoder : public ValueDecoder { : ValueDecoder(type, options, trie_cache), unit_(checked_cast(*type_).unit()), expect_timezone_(!checked_cast(*type_).timezone().empty()), - parsers_(GetParsers(options_)) {} + parsers_(GetTimestampParsers(options_)) {} Status Decode(const uint8_t* data, uint32_t size, bool quoted, value_type* out) { bool zone_offset_present = false; @@ -464,18 +472,93 @@ struct MultipleParsersTimestampValueDecoder : public ValueDecoder { } protected: - using ParserVector = std::vector; + TimeUnit::type unit_; + bool expect_timezone_; + std::vector parsers_; +}; - static ParserVector GetParsers(const ConvertOptions& options) { - ParserVector parsers(options.timestamp_parsers.size()); - for (size_t i = 0; i < options.timestamp_parsers.size(); ++i) { - parsers[i] = options.timestamp_parsers[i].get(); +// +// Value decoder for dates and times, with fallback to user-defined +// timestamp parsers +// + +// Tries the ISO-8601 format first, then the user-defined timestamp parsers. +// A timestamp produced by a user-defined parser is floored to the day +// boundary for dates, and reduced to the time of day for times (consistent +// with casting a timestamp to date32/date64/time32/time64). +template +struct DateTimeWithParsersValueDecoder : public ValueDecoder { + using value_type = typename T::c_type; + + DateTimeWithParsersValueDecoder(const std::shared_ptr& type, + const ConvertOptions& options, + const TrieCache* trie_cache) + : ValueDecoder(type, options, trie_cache), + concrete_type_(checked_cast(*type)), + parse_unit_(GetParseUnit(concrete_type_)), + ticks_per_day_(TicksPerDay(parse_unit_)), + parsers_(GetTimestampParsers(options_)) {} + + Status Decode(const uint8_t* data, uint32_t size, bool quoted, value_type* out) { + TrimWhiteSpace(&data, &size); + if (ARROW_PREDICT_TRUE(string_converter_.Convert( + concrete_type_, reinterpret_cast(data), size, out))) { + return Status::OK(); + } + for (const auto& parser : parsers_) { + int64_t timestamp = 0; + bool zone_offset_present = false; + if (parser->operator()(reinterpret_cast(data), size, parse_unit_, + ×tamp, &zone_offset_present) && + !zone_offset_present) { + // Floor division, to handle values before the epoch + int64_t days = timestamp / ticks_per_day_; + days -= (timestamp % ticks_per_day_) < 0; + if constexpr (std::is_same_v) { + *out = static_cast(days); + } else if constexpr (std::is_same_v) { + *out = days * kMillisPerDay; + } else { + static_assert(is_time_type::value); + *out = static_cast(timestamp - days * ticks_per_day_); + } + return Status::OK(); + } } - return parsers; + return GenericConversionError(type_, data, size); } - TimeUnit::type unit_; - bool expect_timezone_; + protected: + static constexpr int64_t kMillisPerDay = 86400000; + + static TimeUnit::type GetParseUnit(const T& type) { + if constexpr (is_time_type::value) { + // Parse in the time type's own unit, so that the time of day can be + // extracted without further conversion + return type.unit(); + } else { + return TimeUnit::SECOND; + } + } + + static int64_t TicksPerDay(TimeUnit::type unit) { + switch (unit) { + case TimeUnit::SECOND: + return 86400LL; + case TimeUnit::MILLI: + return 86400000LL; + case TimeUnit::MICRO: + return 86400000000LL; + case TimeUnit::NANO: + return 86400000000000LL; + } + return -1; // unreachable + } + + const T& concrete_type_; + arrow::internal::StringConverter string_converter_; + const TimeUnit::type parse_unit_; + const int64_t ticks_per_day_; std::vector parsers_; }; @@ -675,6 +758,24 @@ std::shared_ptr MakeTimestampConverter(const std::shared_ptr class ConverterType, typename T> +std::shared_ptr MakeDateTimeConverter(const std::shared_ptr& type, + const ConvertOptions& options, + MemoryPool* pool) { + if (options.timestamp_parsers.empty()) { + // Default to ISO-8601 + return std::make_shared>>(type, options, + pool); + } + // Try ISO-8601 first, then the user-defined timestamp parsers + return std::make_shared>>( + type, options, pool); +} + // // Concrete Converter factory for reals // @@ -746,10 +847,6 @@ Result> Converter::Make(const std::shared_ptr)) CONVERTER_CASE(Type::BINARY, @@ -763,6 +860,22 @@ Result> Converter::Make(const std::shared_ptr(type, options, pool); break; + case Type::DATE32: + ptr = MakeDateTimeConverter(type, options, pool); + break; + + case Type::DATE64: + ptr = MakeDateTimeConverter(type, options, pool); + break; + + case Type::TIME32: + ptr = MakeDateTimeConverter(type, options, pool); + break; + + case Type::TIME64: + ptr = MakeDateTimeConverter(type, options, pool); + break; + case Type::STRING: if (options.check_utf8) { ptr = std::make_shared>>( diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 5dc078e7fd80..462d0868da2f 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -472,6 +472,40 @@ TEST(Date32Conversion, Errors) { AssertConversionError(date32(), {"2020-13-01\n"}, {0}); } +TEST(Date32Conversion, UserDefinedParsers) { + auto options = ConvertOptions::Defaults(); + const auto type = date32(); + + // Test a single parser + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d/%m/%y")}; + AssertConversion(type, {"15/10/15,18/06/90\n"}, {{16723}, {7473}}, + options); + + // ISO-8601 values are still accepted when parsers are given + AssertConversion(type, {"2020-03-15,15/10/15\n"}, + {{18336}, {16723}}, options); + + // Test multiple parsers, with a pre-epoch value + options.timestamp_parsers.push_back(TimestampParser::MakeStrptime("%d-%m-%Y")); + AssertConversion(type, {"15/10/15,08-05-1945\n"}, + {{16723}, {-9004}}, options); + + // Test month names, parsed case-insensitively + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y")}; + AssertConversion(type, {"15-OCT-15,18-Jun-90\n"}, + {{16723}, {7473}}, options); + + // Parsed timestamps are floored to the day boundary, also before the epoch + options.timestamp_parsers = {TimestampParser::MakeStrptime("%m/%d/%Y %H:%M")}; + AssertConversion(type, {"03/15/2020 14:30,05/08/1945 14:30\n"}, + {{18336}, {-9004}}, options); + + // Test errors + AssertConversionError(type, {"24-12-2020\n"}, {0}, options); + options.timestamp_parsers = {TimestampParser::MakeStrptime("%m/%d/%Y %z")}; + AssertConversionError(type, {"01/02/1970 +0000\n"}, {0}, options); +} + TEST(Date64Conversion, Basics) { AssertConversion(date64(), {"1945-05-08\n", "2020-03-15\n"}, {{-777945600000LL, 1584230400000LL}}); @@ -487,6 +521,38 @@ TEST(Date64Conversion, Errors) { AssertConversionError(date64(), {"2020-13-01\n"}, {0}); } +TEST(Date64Conversion, UserDefinedParsers) { + auto options = ConvertOptions::Defaults(); + const auto type = date64(); + + // Test a single parser + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d/%m/%y")}; + AssertConversion(type, {"15/10/15,18/06/90\n"}, + {{1444867200000LL}, {645667200000LL}}, options); + + // ISO-8601 values are still accepted when parsers are given + AssertConversion(type, {"2020-03-15,15/10/15\n"}, + {{1584230400000LL}, {1444867200000LL}}, options); + + // Test multiple parsers, with a pre-epoch value + options.timestamp_parsers.push_back(TimestampParser::MakeStrptime("%d-%m-%Y")); + AssertConversion(type, {"15/10/15,08-05-1945\n"}, + {{1444867200000LL}, {-777945600000LL}}, options); + + // Test month names, parsed case-insensitively + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y")}; + AssertConversion(type, {"15-OCT-15,18-Jun-90\n"}, + {{1444867200000LL}, {645667200000LL}}, options); + + // Parsed timestamps are floored to the day boundary, also before the epoch + options.timestamp_parsers = {TimestampParser::MakeStrptime("%m/%d/%Y %H:%M")}; + AssertConversion(type, {"03/15/2020 14:30,05/08/1945 14:30\n"}, + {{1584230400000LL}, {-777945600000LL}}, options); + + // Test errors + AssertConversionError(type, {"24-12-2020\n"}, {0}, options); +} + TEST(Time32Conversion, Seconds) { const auto type = time32(TimeUnit::SECOND); @@ -513,6 +579,30 @@ TEST(Time32Conversion, Millis) { AssertConversionError(type, {"23:59:60\n"}, {0}); } +TEST(Time32Conversion, UserDefinedParsers) { + auto options = ConvertOptions::Defaults(); + + // Test a single parser, with non-zero-padded hours + options.timestamp_parsers = {TimestampParser::MakeStrptime("%H:%M:%S")}; + AssertConversion(time32(TimeUnit::SECOND), {"7:55:00,12:01:02\n"}, + {{28500}, {43262}}, options); + AssertConversion(time32(TimeUnit::MILLI), {"7:55:00\n"}, + {{28500000}}, options); + + // ISO-8601 values are still accepted when parsers are given + AssertConversion(time32(TimeUnit::SECOND), {"07:55:00,7:55:00\n"}, + {{28500}, {28500}}, options); + + // The time of day is extracted from parsed timestamps, also before the epoch + options.timestamp_parsers.push_back(TimestampParser::MakeStrptime("%Y-%m-%d %H:%M")); + AssertConversion(time32(TimeUnit::SECOND), + {"2020-03-15 07:55,1945-05-08 07:55\n"}, + {{28500}, {28500}}, options); + + // Test errors + AssertConversionError(time32(TimeUnit::SECOND), {"24:00:00\n"}, {0}, options); +} + TEST(Time64Conversion, Micros) { const auto type = time64(TimeUnit::MICRO); @@ -539,6 +629,24 @@ TEST(Time64Conversion, Nanos) { AssertConversionError(type, {"23:59:60\n"}, {0}); } +TEST(Time64Conversion, UserDefinedParsers) { + auto options = ConvertOptions::Defaults(); + + // Test a single parser, with non-zero-padded hours + options.timestamp_parsers = {TimestampParser::MakeStrptime("%H:%M:%S")}; + AssertConversion(time64(TimeUnit::MICRO), {"7:55:00\n"}, + {{28500000000LL}}, options); + AssertConversion(time64(TimeUnit::NANO), {"7:55:00\n"}, + {{28500000000000LL}}, options); + + // ISO-8601 values are still accepted when parsers are given + AssertConversion(time64(TimeUnit::MICRO), {"07:55:00.123456\n"}, + {{28500123456LL}}, options); + + // Test errors + AssertConversionError(time64(TimeUnit::MICRO), {"24:00:00\n"}, {0}, options); +} + TEST(TimestampConversion, Basics) { auto type = timestamp(TimeUnit::SECOND); diff --git a/cpp/src/arrow/csv/inference_internal.h b/cpp/src/arrow/csv/inference_internal.h index c08407ff7f6f..94b60e647cb5 100644 --- a/cpp/src/arrow/csv/inference_internal.h +++ b/cpp/src/arrow/csv/inference_internal.h @@ -46,7 +46,15 @@ enum class InferKind { class InferStatus { public: explicit InferStatus(const ConvertOptions& options) - : kind_(InferKind::Null), can_loosen_type_(true), options_(options) {} + : kind_(InferKind::Null), can_loosen_type_(true), options_(options) { + if (!options.timestamp_parsers.empty()) { + // Date and time inference must not use the user-defined timestamp parsers, + // otherwise a value with a time-of-day (resp. date) part could be inferred + // as a date (resp. time) and be silently truncated. + date_time_options_ = std::make_unique(options); + date_time_options_->timestamp_parsers.clear(); + } + } InferKind kind() const { return kind_; } @@ -106,6 +114,12 @@ class InferStatus { return Converter::Make(type, options_, pool); }; + auto make_date_time_converter = + [&](std::shared_ptr type) -> Result> { + return Converter::Make(type, date_time_options_ ? *date_time_options_ : options_, + pool); + }; + auto make_dict_converter = [&](std::shared_ptr type) -> Result> { ARROW_ASSIGN_OR_RAISE(auto dict_converter, @@ -122,9 +136,9 @@ class InferStatus { case InferKind::Boolean: return make_converter(boolean()); case InferKind::Date: - return make_converter(date32()); + return make_date_time_converter(date32()); case InferKind::Time: - return make_converter(time32(TimeUnit::SECOND)); + return make_date_time_converter(time32(TimeUnit::SECOND)); case InferKind::Timestamp: return make_converter(timestamp(TimeUnit::SECOND)); case InferKind::TimestampNS: @@ -159,6 +173,9 @@ class InferStatus { InferKind kind_; bool can_loosen_type_; const ConvertOptions& options_; + // Copy of options_ with timestamp_parsers cleared, for date and time inference. + // Only allocated when custom timestamp parsers are configured. + std::unique_ptr date_time_options_; }; } // namespace csv diff --git a/cpp/src/arrow/csv/options.h b/cpp/src/arrow/csv/options.h index 41d0b63dacbb..18a203bc0833 100644 --- a/cpp/src/arrow/csv/options.h +++ b/cpp/src/arrow/csv/options.h @@ -133,6 +133,13 @@ struct ARROW_EXPORT ConvertOptions { /// the CSV conversion logic will try parsing values starting from the /// beginning of this vector. If no parsers are specified, we use the default /// built-in ISO-8601 parser. + /// + /// These parsers are also used as a fallback for columns explicitly typed + /// as date32, date64, time32 or time64, after the built-in ISO-8601 parser + /// failed on a value. A timestamp produced by a fallback parser is floored + /// to the day boundary for dates, and reduced to the time of day for times + /// (like casting a timestamp to a date or time type). Type inference of + /// date and time columns is not affected. std::vector> timestamp_parsers; /// Create conversion options with default values, including conventional diff --git a/cpp/src/arrow/vendored/musl/strptime.c b/cpp/src/arrow/vendored/musl/strptime.c index 41912fd1bbf4..7982dde2dc54 100644 --- a/cpp/src/arrow/vendored/musl/strptime.c +++ b/cpp/src/arrow/vendored/musl/strptime.c @@ -17,12 +17,29 @@ #undef HAVE_LANGINFO -#ifndef _WIN32 +#if !defined(_WIN32) && !defined(ARROW_TEST_FALLBACK_LANGINFO) #define HAVE_LANGINFO 1 #endif #ifdef HAVE_LANGINFO #include +#else +// C-locale replacement for nl_langinfo(), restricted to the items used below. +// The constants follow the glibc/musl layout that the symbolic_range loop +// relies on: full names directly follow the abbreviated ones. +enum { ABDAY_1 = 0, DAY_1 = 7, ABMON_1 = 14, MON_1 = 26, AM_STR = 38, PM_STR = 39, + D_T_FMT = 40, D_FMT = 41, T_FMT = 42, T_FMT_AMPM = 43 }; +static const char *arrow_c_langinfo[] = { + "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat", + "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", + "Jan", "Feb", "Mar", "Apr", "May", "Jun", + "Jul", "Aug", "Sep", "Oct", "Nov", "Dec", + "January", "February", "March", "April", "May", "June", + "July", "August", "September", "October", "November", "December", + "AM", "PM", + "%a %b %e %T %Y", "%m/%d/%y", "%H:%M:%S", "%I:%M:%S %p"}; +#define nl_langinfo(x) arrow_c_langinfo[x] +#define HAVE_LANGINFO 1 #endif #define strptime arrow_strptime diff --git a/docs/source/cpp/csv.rst b/docs/source/cpp/csv.rst index 74ee0bb4fbbf..cffa7f992c8d 100644 --- a/docs/source/cpp/csv.rst +++ b/docs/source/cpp/csv.rst @@ -348,6 +348,21 @@ values afterwards using the ``assume_timezone`` compute function. | | 2021-01-01T00:00:00Z | | +-------------------+------------------------------+-------------------+ +Date and time parsing +--------------------- + +Columns explicitly typed as date32, date64, time32 or time64 are parsed +from the ISO-8601 formats (e.g. ``2021-01-01`` and ``01:23:45``, +respectively). If a value fails to parse as such, the user-defined parsers +in :member:`ConvertOptions::timestamp_parsers` are then tried in order. +A timestamp produced by such a parser is floored to the day boundary for +dates, and reduced to the time of day for times, like casting a timestamp +to a date or time type. For example, with the parser ``"%m/%d/%Y %H:%M"``, +the value ``03/15/2020 14:30`` in a date32 column yields ``2020-03-15``. + +Type inference of date and time columns always uses the ISO-8601 formats +only, regardless of :member:`ConvertOptions::timestamp_parsers`. + Nulls ----- diff --git a/python/pyarrow/_csv.pyx b/python/pyarrow/_csv.pyx index 4789b9532e2e..f8d5cbcec5b1 100644 --- a/python/pyarrow/_csv.pyx +++ b/python/pyarrow/_csv.pyx @@ -700,6 +700,14 @@ cdef class ConvertOptions(_Weakrefable): value ISO8601() can also be given). By default, a fast built-in ISO-8601 parser is used. + These parsers are also used as a fallback when converting columns + explicitly typed (using `column_types`) as date32, date64, time32 + or time64, after the built-in ISO-8601 parser failed on a value. + A timestamp produced by a fallback parser is floored to the day + boundary for dates, and reduced to the time of day for times (like + casting a timestamp to a date or time type). Type inference of + date and time columns is not affected. + Examples -------- diff --git a/python/pyarrow/tests/test_csv.py b/python/pyarrow/tests/test_csv.py index 275a7e808ddd..26a68d28d296 100644 --- a/python/pyarrow/tests/test_csv.py +++ b/python/pyarrow/tests/test_csv.py @@ -1165,6 +1165,65 @@ def test_dates(self): 'b': [datetime(1970, 1, 2), datetime(1971, 1, 2)], } + def test_dates_with_timestamp_parsers(self): + # Custom timestamp parsers are used as a fallback for explicitly-typed + # date columns (GH-28303, GH-41488) + rows = b"a,b\n15/10/2015,1970-01-02\n18/06/1990,15/10/2015\n" + opts = ConvertOptions() + opts.column_types = {'a': pa.date32(), 'b': pa.date64()} + with pytest.raises(pa.ArrowInvalid, + match="CSV conversion error to date"): + self.read_bytes(rows, convert_options=opts) + + opts.timestamp_parsers = ['%d/%m/%Y'] + table = self.read_bytes(rows, convert_options=opts) + schema = pa.schema([('a', pa.date32()), + ('b', pa.date64())]) + assert table.schema == schema + # ISO values keep working alongside custom-parsed ones + assert table.to_pydict() == { + 'a': [date(2015, 10, 15), date(1990, 6, 18)], + 'b': [date(1970, 1, 2), date(2015, 10, 15)], + } + + # Month names are parsed case-insensitively + rows = b"a\n15-OCT-15\n18-Jun-90\n" + opts = ConvertOptions(column_types={'a': pa.date32()}, + timestamp_parsers=['%d-%b-%y']) + table = self.read_bytes(rows, convert_options=opts) + assert table.to_pydict() == { + 'a': [date(2015, 10, 15), date(1990, 6, 18)], + } + + # Inference is unaffected by the fallback: a non-ISO date string + # still infers as timestamp, not date32 + rows = b"a\n15/10/2015\n" + opts = ConvertOptions(timestamp_parsers=['%d/%m/%Y']) + table = self.read_bytes(rows, convert_options=opts) + assert table.schema == pa.schema([('a', pa.timestamp('s'))]) + + def test_times_with_timestamp_parsers(self): + # Custom timestamp parsers are used as a fallback for explicitly-typed + # time columns (GH-41488) + from datetime import time + + rows = b"a,b\n7:55:00,12:01:02\n23:59:59,0:00:01\n" + opts = ConvertOptions() + opts.column_types = {'a': pa.time32('s'), 'b': pa.time64('us')} + with pytest.raises(pa.ArrowInvalid, + match="CSV conversion error to time"): + self.read_bytes(rows, convert_options=opts) + + opts.timestamp_parsers = ['%H:%M:%S'] + table = self.read_bytes(rows, convert_options=opts) + schema = pa.schema([('a', pa.time32('s')), + ('b', pa.time64('us'))]) + assert table.schema == schema + assert table.to_pydict() == { + 'a': [time(7, 55, 0), time(23, 59, 59)], + 'b': [time(12, 1, 2), time(0, 0, 1)], + } + def test_times(self): # Times are inferred as time32[s] by default from datetime import time From d19c0b39d241edb324c389a8bb28bdcc015dfa89 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Wed, 7 Oct 2026 13:59:10 +0300 Subject: [PATCH 02/14] Avoid copying ConvertOptions for each inferred column Pass an is_type_inference flag through Converter::Make to MakeDateTimeConverter instead of keeping a per-column copy of ConvertOptions with timestamp_parsers cleared. The copy made memory usage quadratic in the number of columns when timestamp_parsers was set. Co-Authored-By: Claude Opus 5.5 (1M context) --- cpp/src/arrow/csv/converter.cc | 24 ++++++++++++++++-------- cpp/src/arrow/csv/converter.h | 11 +++++++---- cpp/src/arrow/csv/inference_internal.h | 16 ++-------------- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index b164e61786a4..f19223478361 100644 --- a/cpp/src/arrow/csv/converter.cc +++ b/cpp/src/arrow/csv/converter.cc @@ -765,9 +765,12 @@ std::shared_ptr MakeTimestampConverter(const std::shared_ptr class ConverterType, typename T> std::shared_ptr MakeDateTimeConverter(const std::shared_ptr& type, const ConvertOptions& options, - MemoryPool* pool) { - if (options.timestamp_parsers.empty()) { - // Default to ISO-8601 + MemoryPool* pool, + bool is_type_inference) { + if (is_type_inference || options.timestamp_parsers.empty()) { + // Default to ISO-8601. Type inference must not use the user-defined + // timestamp parsers, otherwise a value with a time-of-day (resp. date) part + // could be inferred as a date (resp. time) and be silently truncated. return std::make_shared>>(type, options, pool); } @@ -816,7 +819,8 @@ DictionaryConverter::DictionaryConverter(const std::shared_ptr& value_ Result> Converter::Make(const std::shared_ptr& type, const ConvertOptions& options, - MemoryPool* pool) { + MemoryPool* pool, + bool is_type_inference) { std::shared_ptr ptr; switch (type->id()) { @@ -861,19 +865,23 @@ Result> Converter::Make(const std::shared_ptr(type, options, pool); + ptr = MakeDateTimeConverter(type, options, pool, + is_type_inference); break; case Type::DATE64: - ptr = MakeDateTimeConverter(type, options, pool); + ptr = MakeDateTimeConverter(type, options, pool, + is_type_inference); break; case Type::TIME32: - ptr = MakeDateTimeConverter(type, options, pool); + ptr = MakeDateTimeConverter(type, options, pool, + is_type_inference); break; case Type::TIME64: - ptr = MakeDateTimeConverter(type, options, pool); + ptr = MakeDateTimeConverter(type, options, pool, + is_type_inference); break; case Type::STRING: diff --git a/cpp/src/arrow/csv/converter.h b/cpp/src/arrow/csv/converter.h index c6254bd7ca1f..472c33ae79b2 100644 --- a/cpp/src/arrow/csv/converter.h +++ b/cpp/src/arrow/csv/converter.h @@ -42,10 +42,13 @@ class ARROW_EXPORT Converter { std::shared_ptr type() const { return type_; } - // Create a Converter for the given data type - static Result> Make( - const std::shared_ptr& type, const ConvertOptions& options, - MemoryPool* pool = default_memory_pool()); + // Create a Converter for the given data type. + // If is_type_inference is true, date and time converters ignore + // options.timestamp_parsers and accept only ISO-8601 values. + static Result> Make(const std::shared_ptr& type, + const ConvertOptions& options, + MemoryPool* pool = default_memory_pool(), + bool is_type_inference = false); protected: ARROW_DISALLOW_COPY_AND_ASSIGN(Converter); diff --git a/cpp/src/arrow/csv/inference_internal.h b/cpp/src/arrow/csv/inference_internal.h index 94b60e647cb5..bcd3a34f48ee 100644 --- a/cpp/src/arrow/csv/inference_internal.h +++ b/cpp/src/arrow/csv/inference_internal.h @@ -46,15 +46,7 @@ enum class InferKind { class InferStatus { public: explicit InferStatus(const ConvertOptions& options) - : kind_(InferKind::Null), can_loosen_type_(true), options_(options) { - if (!options.timestamp_parsers.empty()) { - // Date and time inference must not use the user-defined timestamp parsers, - // otherwise a value with a time-of-day (resp. date) part could be inferred - // as a date (resp. time) and be silently truncated. - date_time_options_ = std::make_unique(options); - date_time_options_->timestamp_parsers.clear(); - } - } + : kind_(InferKind::Null), can_loosen_type_(true), options_(options) {} InferKind kind() const { return kind_; } @@ -116,8 +108,7 @@ class InferStatus { auto make_date_time_converter = [&](std::shared_ptr type) -> Result> { - return Converter::Make(type, date_time_options_ ? *date_time_options_ : options_, - pool); + return Converter::Make(type, options_, pool, /*is_type_inference=*/true); }; auto make_dict_converter = @@ -173,9 +164,6 @@ class InferStatus { InferKind kind_; bool can_loosen_type_; const ConvertOptions& options_; - // Copy of options_ with timestamp_parsers cleared, for date and time inference. - // Only allocated when custom timestamp parsers are configured. - std::unique_ptr date_time_options_; }; } // namespace csv From 4a0fb79c28ffb21174ad6bfa1344e23de6a33a16 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Thu, 8 Oct 2026 16:41:00 +0300 Subject: [PATCH 03/14] Accept ISO-8601 fractional seconds in date columns The ISO-8601 fallback parsed dates in seconds, so values with fractional seconds were rejected before the flooring to the day could run. Discard the fractional digits before parsing, which keeps the full date range of parsing in seconds. Co-Authored-By: Claude Fable 5.1 --- cpp/src/arrow/csv/converter.cc | 36 +++++++++++++++++++++++++++-- cpp/src/arrow/csv/converter_test.cc | 31 +++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index f19223478361..c124b617aa62 100644 --- a/cpp/src/arrow/csv/converter.cc +++ b/cpp/src/arrow/csv/converter.cc @@ -482,6 +482,29 @@ struct MultipleParsersTimestampValueDecoder : public ValueDecoder { // timestamp parsers // +// Discards the fractional seconds (up to 9 digits) of an ISO-8601 timestamp +// "YYYY-MM-DD[ T]hh:mm:ss.s{1,9}" with an optional zone offset. Returns `s` +// if there are no fractional seconds, a copy without them in `buffer`, or +// nullptr if they are malformed. +const char* DiscardISO8601Subseconds(const char* s, uint32_t* size, char (&buffer)[64]) { + if (*size <= 20 || s[19] != '.') { + return s; + } + uint32_t fraction_length = 0; + while (20 + fraction_length < *size && s[20 + fraction_length] >= '0' && + s[20 + fraction_length] <= '9') { + ++fraction_length; + } + const uint32_t rest_length = *size - 20 - fraction_length; + if (fraction_length == 0 || fraction_length > 9 || 19 + rest_length > sizeof(buffer)) { + return nullptr; + } + std::memcpy(buffer, s, 19); + std::memcpy(buffer + 19, s + 20 + fraction_length, rest_length); + *size = 19 + rest_length; + return buffer; +} + // Tries the ISO-8601 format first, then the user-defined timestamp parsers. // A timestamp produced by a user-defined parser is floored to the day // boundary for dates, and reduced to the time of day for times (consistent @@ -508,8 +531,17 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { for (const auto& parser : parsers_) { int64_t timestamp = 0; bool zone_offset_present = false; - if (parser->operator()(reinterpret_cast(data), size, parse_unit_, - ×tamp, &zone_offset_present) && + const char* s = reinterpret_cast(data); + uint32_t s_size = size; + char buffer[64]; + if (is_date_type::value && std::strcmp(parser->kind(), "iso8601") == 0) { + // Dates discard fractional seconds, so accept them in any number + s = DiscardISO8601Subseconds(s, &s_size, buffer); + if (s == nullptr) { + continue; + } + } + if (parser->operator()(s, s_size, parse_unit_, ×tamp, &zone_offset_present) && !zone_offset_present) { // Floor division, to handle values before the epoch int64_t days = timestamp / ticks_per_day_; diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 462d0868da2f..93c3d5fd0710 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -504,6 +504,29 @@ TEST(Date32Conversion, UserDefinedParsers) { AssertConversionError(type, {"24-12-2020\n"}, {0}, options); options.timestamp_parsers = {TimestampParser::MakeStrptime("%m/%d/%Y %z")}; AssertConversionError(type, {"01/02/1970 +0000\n"}, {0}, options); + + // ISO-8601 fractional seconds are discarded, also before the epoch and + // outside the range of nanosecond timestamps + options.timestamp_parsers = {TimestampParser::MakeISO8601()}; + AssertConversion( + type, + {"2020-03-15 14:30:00.123,1969-12-31 23:59:59.123456789,1600-01-01 00:00:00.5\n"}, + {{18336}, {-1}, {-135140}}, options); + // With the "T" separator and the remaining numbers of fractional digits + AssertConversion( + type, + {"2020-03-15T14:30:00.12,2020-03-15T14:30:00.1234,2020-03-15T14:30:00.12345," + "2020-03-15T14:30:00.123456,2020-03-15T14:30:00.1234567," + "2020-03-15T14:30:00.12345678\n"}, + {{18336}, {18336}, {18336}, {18336}, {18336}, {18336}}, options); + AssertConversionError(type, {"2020-03-15 14:30:00.1234567890\n"}, {0}, options); + AssertConversionError(type, {"2020-03-15 14:30:00.\n"}, {0}, options); + AssertConversionError(type, {"2020-03-15 14:30:00.12x\n"}, {0}, options); + // Zone offsets are rejected, also after fractional seconds + AssertConversionError(type, + {"2020-03-15 14:30:00.5Z,2020-03-15 14:30:00.5+05," + "2020-03-15 14:30:00.5+0530,2020-03-15T14:30:00.5+05:30\n"}, + {0, 1, 2, 3}, options); } TEST(Date64Conversion, Basics) { @@ -551,6 +574,14 @@ TEST(Date64Conversion, UserDefinedParsers) { // Test errors AssertConversionError(type, {"24-12-2020\n"}, {0}, options); + + // ISO-8601 fractional seconds are discarded, also before the epoch and + // outside the range of nanosecond timestamps + options.timestamp_parsers = {TimestampParser::MakeISO8601()}; + AssertConversion( + type, + {"2020-03-15 14:30:00.123,1969-12-31 23:59:59.123456789,1600-01-01 00:00:00.5\n"}, + {{1584230400000LL}, {-86400000LL}, {-11676096000000LL}}, options); } TEST(Time32Conversion, Seconds) { From 52395269c05173b6413435932fc86f1ed5df3257 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Thu, 8 Oct 2026 17:09:11 +0300 Subject: [PATCH 04/14] Extract the time of day without overflowing days * ticks_per_day can overflow near the minimum of nanosecond timestamps even though the time of day is representable. Use a normalized remainder instead. Co-Authored-By: Claude Fable 5.1 --- cpp/src/arrow/csv/converter.cc | 7 ++++++- cpp/src/arrow/csv/converter_test.cc | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index c124b617aa62..5809418677a1 100644 --- a/cpp/src/arrow/csv/converter.cc +++ b/cpp/src/arrow/csv/converter.cc @@ -552,7 +552,12 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { *out = days * kMillisPerDay; } else { static_assert(is_time_type::value); - *out = static_cast(timestamp - days * ticks_per_day_); + // Normalized remainder, as days * ticks_per_day_ can overflow + int64_t time_of_day = timestamp % ticks_per_day_; + if (time_of_day < 0) { + time_of_day += ticks_per_day_; + } + *out = static_cast(time_of_day); } return Status::OK(); } diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 93c3d5fd0710..d04556fbdbde 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -676,6 +676,13 @@ TEST(Time64Conversion, UserDefinedParsers) { // Test errors AssertConversionError(time64(TimeUnit::MICRO), {"24:00:00\n"}, {0}, options); + + // The time of day is extracted without overflow near the minimum of + // nanosecond timestamps (1677-09-21 00:12:43.145224192), and before the epoch + options.timestamp_parsers = {TimestampParser::MakeISO8601()}; + AssertConversion(time64(TimeUnit::NANO), + {"1677-09-21 00:12:44,1969-12-31 23:59:59\n"}, + {{764000000000LL}, {86399000000000LL}}, options); } TEST(TimestampConversion, Basics) { From 4e14bcb1a29f3e610a20f972542613bf70809de8 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Thu, 8 Oct 2026 17:57:06 +0300 Subject: [PATCH 05/14] Extract the time of day from dates outside the time unit's range Parsing in the time unit limits time64[ns] columns to dates between 1677 and 2262 although the time of day is representable. Retry in seconds when parsing in the unit fails, and for the ISO-8601 parser split the fractional seconds off so that they are parsed separately in the time unit, keeping their precision for any date. Co-Authored-By: Claude Fable 5.1 --- cpp/src/arrow/csv/converter.cc | 49 ++++++++++++++++++++--------- cpp/src/arrow/csv/converter_test.cc | 23 ++++++++++++++ 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index 5809418677a1..466de0063e7a 100644 --- a/cpp/src/arrow/csv/converter.cc +++ b/cpp/src/arrow/csv/converter.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -482,11 +483,13 @@ struct MultipleParsersTimestampValueDecoder : public ValueDecoder { // timestamp parsers // -// Discards the fractional seconds (up to 9 digits) of an ISO-8601 timestamp +// Splits the fractional seconds (up to 9 digits) off an ISO-8601 timestamp // "YYYY-MM-DD[ T]hh:mm:ss.s{1,9}" with an optional zone offset. Returns `s` // if there are no fractional seconds, a copy without them in `buffer`, or -// nullptr if they are malformed. -const char* DiscardISO8601Subseconds(const char* s, uint32_t* size, char (&buffer)[64]) { +// nullptr if they are malformed. The split-off digits are returned in +// `subseconds`. +const char* SplitISO8601Subseconds(const char* s, uint32_t* size, char (&buffer)[64], + std::string_view* subseconds) { if (*size <= 20 || s[19] != '.') { return s; } @@ -501,6 +504,7 @@ const char* DiscardISO8601Subseconds(const char* s, uint32_t* size, char (&buffe } std::memcpy(buffer, s, 19); std::memcpy(buffer + 19, s + 20 + fraction_length, rest_length); + *subseconds = std::string_view(s + 20, fraction_length); *size = 19 + rest_length; return buffer; } @@ -534,30 +538,47 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { const char* s = reinterpret_cast(data); uint32_t s_size = size; char buffer[64]; - if (is_date_type::value && std::strcmp(parser->kind(), "iso8601") == 0) { - // Dates discard fractional seconds, so accept them in any number - s = DiscardISO8601Subseconds(s, &s_size, buffer); + std::string_view subseconds; + if (std::strcmp(parser->kind(), "iso8601") == 0) { + // Set the fractional seconds aside, so that any number of digits is + // accepted: dates discard them, times add them back below + s = SplitISO8601Subseconds(s, &s_size, buffer, &subseconds); if (s == nullptr) { continue; } } - if (parser->operator()(s, s_size, parse_unit_, ×tamp, &zone_offset_present) && - !zone_offset_present) { + TimeUnit::type unit = parse_unit_; + bool parsed = parser->operator()(s, s_size, unit, ×tamp, &zone_offset_present); + if (!parsed && unit != TimeUnit::SECOND) { + // The date may be out of the range of the time unit while the time of + // day is representable: retry in seconds, scaled below + unit = TimeUnit::SECOND; + parsed = parser->operator()(s, s_size, unit, ×tamp, &zone_offset_present); + } + if (parsed && !zone_offset_present) { + const int64_t ticks_per_day = TicksPerDay(unit); // Floor division, to handle values before the epoch - int64_t days = timestamp / ticks_per_day_; - days -= (timestamp % ticks_per_day_) < 0; + int64_t days = timestamp / ticks_per_day; + days -= (timestamp % ticks_per_day) < 0; if constexpr (std::is_same_v) { *out = static_cast(days); } else if constexpr (std::is_same_v) { *out = days * kMillisPerDay; } else { static_assert(is_time_type::value); - // Normalized remainder, as days * ticks_per_day_ can overflow - int64_t time_of_day = timestamp % ticks_per_day_; + // Normalized remainder, as days * ticks_per_day can overflow + int64_t time_of_day = timestamp % ticks_per_day; if (time_of_day < 0) { - time_of_day += ticks_per_day_; + time_of_day += ticks_per_day; + } + uint32_t subseconds_ticks = 0; + if (!subseconds.empty() && + !arrow::internal::detail::ParseSubSeconds( + subseconds.data(), subseconds.size(), parse_unit_, &subseconds_ticks)) { + continue; // the time unit cannot represent the fractional seconds } - *out = static_cast(time_of_day); + *out = static_cast(time_of_day * (ticks_per_day_ / ticks_per_day) + + subseconds_ticks); } return Status::OK(); } diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index d04556fbdbde..442fc53dec54 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -632,6 +632,16 @@ TEST(Time32Conversion, UserDefinedParsers) { // Test errors AssertConversionError(time32(TimeUnit::SECOND), {"24:00:00\n"}, {0}, options); + + // Fractional seconds are kept for times, but rejected when the time unit + // cannot represent them + options.timestamp_parsers = {TimestampParser::MakeISO8601()}; + AssertConversion( + time32(TimeUnit::MILLI), {"2020-03-15 07:55:00.5\n"}, {{28500500}}, options); + AssertConversionError(time32(TimeUnit::SECOND), {"2020-03-15 07:55:00.5\n"}, {0}, + options); + AssertConversionError(time32(TimeUnit::MILLI), {"2020-03-15 07:55:00.5000\n"}, {0}, + options); } TEST(Time64Conversion, Micros) { @@ -683,6 +693,19 @@ TEST(Time64Conversion, UserDefinedParsers) { AssertConversion(time64(TimeUnit::NANO), {"1677-09-21 00:12:44,1969-12-31 23:59:59\n"}, {{764000000000LL}, {86399000000000LL}}, options); + + // The time of day is extracted from dates outside the range of nanosecond + // timestamps, with and without fractional seconds + AssertConversion( + time64(TimeUnit::NANO), + {"9999-12-31 07:55:00,1600-01-01 07:55:00,9999-12-31 07:55:00.123456789," + "1600-01-01 07:55:00.5\n"}, + {{28500000000000LL}, {28500000000000LL}, {28500123456789LL}, {28500500000000LL}}, + options); + options.timestamp_parsers = {TimestampParser::MakeStrptime("%Y-%m-%d %H:%M:%S")}; + AssertConversion( + time64(TimeUnit::NANO), {"9999-12-31 07:55:00,1600-01-01 07:55:00\n"}, + {{28500000000000LL}, {28500000000000LL}}, options); } TEST(TimestampConversion, Basics) { From 90ca7675018fa6182111a5645ffa72ecf9993e6e Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Thu, 8 Oct 2026 18:15:38 +0300 Subject: [PATCH 06/14] Reject dates outside the range of date32 and date64 Narrowing the day count to date32 wrapped silently, and scaling it to milliseconds for date64 could overflow. Both are reachable with a user-defined timestamp parser, so return a conversion error instead. Co-Authored-By: Claude Fable 5.1 --- cpp/src/arrow/csv/converter.cc | 9 +++++++- cpp/src/arrow/csv/converter_test.cc | 34 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index 466de0063e7a..29bfe87f7326 100644 --- a/cpp/src/arrow/csv/converter.cc +++ b/cpp/src/arrow/csv/converter.cc @@ -37,6 +37,7 @@ #include "arrow/type_traits.h" #include "arrow/util/checked_cast.h" #include "arrow/util/decimal.h" +#include "arrow/util/int_util_overflow.h" #include "arrow/util/trie_internal.h" #include "arrow/util/utf8_internal.h" #include "arrow/util/value_parsing.h" // IWYU pragma: keep @@ -561,9 +562,15 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { int64_t days = timestamp / ticks_per_day; days -= (timestamp % ticks_per_day) < 0; if constexpr (std::is_same_v) { + if (days < std::numeric_limits::min() || + days > std::numeric_limits::max()) { + continue; // out of the range of date32 + } *out = static_cast(days); } else if constexpr (std::is_same_v) { - *out = days * kMillisPerDay; + if (arrow::internal::MultiplyWithOverflow(days, kMillisPerDay, out)) { + continue; // out of the range of date64 + } } else { static_assert(is_time_type::value); // Normalized remainder, as days * ticks_per_day can overflow diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 442fc53dec54..510911ece3af 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -171,6 +171,25 @@ void AssertConversionError(const std::shared_ptr& type, } } +// Parses seconds since the epoch given as an integer, to produce timestamps +// out of the reach of the built-in parsers (which require a 4-digit year) +class EpochSecondsParser : public TimestampParser { + public: + bool operator()(const char* s, size_t length, TimeUnit::type out_unit, int64_t* out, + bool* out_zone_offset_present) const override { + int64_t seconds = 0; + if (!::arrow::internal::ParseValue(Int64Type{}, s, length, &seconds)) { + return false; + } + if (out_zone_offset_present != nullptr) { + *out_zone_offset_present = false; + } + return ::arrow::util::CastSecondsToUnit(out_unit, seconds, out); + } + + const char* kind() const override { return "epoch_seconds"; } +}; + ////////////////////////////////////////////////////////////////////////// // Converter tests @@ -527,6 +546,12 @@ TEST(Date32Conversion, UserDefinedParsers) { {"2020-03-15 14:30:00.5Z,2020-03-15 14:30:00.5+05," "2020-03-15 14:30:00.5+0530,2020-03-15T14:30:00.5+05:30\n"}, {0, 1, 2, 3}, options); + + // Days outside the range of date32 are rejected rather than wrapped + options.timestamp_parsers = {std::make_shared()}; + AssertConversion(type, {"185542587100800,-185542587187200\n"}, + {{INT32_MAX}, {INT32_MIN}}, options); + AssertConversionError(type, {"185542587187200,-185542587273600\n"}, {0, 1}, options); } TEST(Date64Conversion, Basics) { @@ -582,6 +607,15 @@ TEST(Date64Conversion, UserDefinedParsers) { type, {"2020-03-15 14:30:00.123,1969-12-31 23:59:59.123456789,1600-01-01 00:00:00.5\n"}, {{1584230400000LL}, {-86400000LL}, {-11676096000000LL}}, options); + + // Days outside the range of date64 are rejected rather than overflowing + options.timestamp_parsers = {std::make_shared()}; + AssertConversion( + type, {"9223372036828800,-9223372036828800\n"}, + {{9223372036828800000LL}, {-9223372036828800000LL}}, options); + AssertConversionError(type, + {"9223372036915200,-9223372036915200,9223372036854775807\n"}, + {0, 1, 2}, options); } TEST(Time32Conversion, Seconds) { From 445f9748083d8b10dd3ebbd989844b199b35c415 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Tue, 13 Oct 2026 11:57:44 +0300 Subject: [PATCH 07/14] Reject anything after the ISO-8601 fractional seconds in date and time columns The fractional seconds were split off and the remainder of the value was handed to the ISO-8601 parser, so a second fractional part was accepted (e.g. "14:30:00.5.5" as 14:30:01) and could yield times of day out of range. Require the fractional seconds to end the value; zone offsets are rejected for dates and times anyway. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/csv/converter.cc | 48 +++++++++++++---------------- cpp/src/arrow/csv/converter_test.cc | 13 ++++++++ 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index 29bfe87f7326..af7921649408 100644 --- a/cpp/src/arrow/csv/converter.cc +++ b/cpp/src/arrow/csv/converter.cc @@ -484,30 +484,27 @@ struct MultipleParsersTimestampValueDecoder : public ValueDecoder { // timestamp parsers // -// Splits the fractional seconds (up to 9 digits) off an ISO-8601 timestamp -// "YYYY-MM-DD[ T]hh:mm:ss.s{1,9}" with an optional zone offset. Returns `s` -// if there are no fractional seconds, a copy without them in `buffer`, or -// nullptr if they are malformed. The split-off digits are returned in -// `subseconds`. -const char* SplitISO8601Subseconds(const char* s, uint32_t* size, char (&buffer)[64], - std::string_view* subseconds) { +// Splits the fractional seconds (1 to 9 digits) off the end of an ISO-8601 +// timestamp "YYYY-MM-DD[ T]hh:mm:ss.s{1,9}" into `subseconds`, and shortens +// `size` to exclude them. Returns false if the fractional seconds are +// malformed or followed by anything (zone offsets are rejected for dates and +// times anyway). +bool SplitISO8601Subseconds(const char* s, uint32_t* size, std::string_view* subseconds) { if (*size <= 20 || s[19] != '.') { - return s; + return true; } - uint32_t fraction_length = 0; - while (20 + fraction_length < *size && s[20 + fraction_length] >= '0' && - s[20 + fraction_length] <= '9') { - ++fraction_length; + const uint32_t fraction_length = *size - 20; + if (fraction_length > 9) { + return false; } - const uint32_t rest_length = *size - 20 - fraction_length; - if (fraction_length == 0 || fraction_length > 9 || 19 + rest_length > sizeof(buffer)) { - return nullptr; + for (uint32_t i = 20; i < *size; ++i) { + if (s[i] < '0' || s[i] > '9') { + return false; + } } - std::memcpy(buffer, s, 19); - std::memcpy(buffer + 19, s + 20 + fraction_length, rest_length); *subseconds = std::string_view(s + 20, fraction_length); - *size = 19 + rest_length; - return buffer; + *size = 19; + return true; } // Tries the ISO-8601 format first, then the user-defined timestamp parsers. @@ -538,15 +535,12 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { bool zone_offset_present = false; const char* s = reinterpret_cast(data); uint32_t s_size = size; - char buffer[64]; std::string_view subseconds; - if (std::strcmp(parser->kind(), "iso8601") == 0) { - // Set the fractional seconds aside, so that any number of digits is - // accepted: dates discard them, times add them back below - s = SplitISO8601Subseconds(s, &s_size, buffer, &subseconds); - if (s == nullptr) { - continue; - } + // Set the fractional seconds aside, so that any number of digits is + // accepted: dates discard them, times add them back below + if (std::strcmp(parser->kind(), "iso8601") == 0 && + !SplitISO8601Subseconds(s, &s_size, &subseconds)) { + continue; } TimeUnit::type unit = parse_unit_; bool parsed = parser->operator()(s, s_size, unit, ×tamp, &zone_offset_present); diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 510911ece3af..bc2a1c73b2e7 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -676,6 +676,19 @@ TEST(Time32Conversion, UserDefinedParsers) { options); AssertConversionError(time32(TimeUnit::MILLI), {"2020-03-15 07:55:00.5000\n"}, {0}, options); + + // A second fractional part, or a zone offset after the fractional part, is + // rejected rather than parsed as part of the value (a second fraction also + // produced times of day out of range) + AssertConversionError(time32(TimeUnit::MILLI), + {"2020-03-15 14:30:00.5.5,2020-03-15 23:59:59.9.9," + "2020-03-15 14:30:00.5Z,2020-03-15 14:30:00.5+05," + "2020-03-15 14:30:00.5+0530,2020-03-15T14:30:00.5+05:30\n"}, + {0, 1, 2, 3, 4, 5}, options); + AssertConversionError(time64(TimeUnit::MICRO), {"2020-03-15 14:30:00.5.5\n"}, {0}, + options); + AssertConversionError(time64(TimeUnit::NANO), {"2020-03-15 14:30:00.5.5\n"}, {0}, + options); } TEST(Time64Conversion, Micros) { From 0ff0951a2af5b348e86742e31fbfb2d5b203de00 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Tue, 13 Oct 2026 12:41:10 +0300 Subject: [PATCH 08/14] Reject leap seconds in the strptime timestamp parser strptime accepts a seconds value of 60 (glibc also 61) and the parser added it to the time of day, wrapping it into the next minute: "12:30:60" became 12:31:00 and "2020-12-31 23:59:60" became 2021-01-01 in a date32 column. Arrow cannot represent leap seconds, and the ISO-8601 parser already rejects them, so reject them here too. This also affects timestamp columns. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/csv/converter_test.cc | 10 ++++++++++ cpp/src/arrow/util/value_parsing.h | 7 +++++++ cpp/src/arrow/util/value_parsing_test.cc | 6 +++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index bc2a1c73b2e7..239478c981c9 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -552,6 +552,10 @@ TEST(Date32Conversion, UserDefinedParsers) { AssertConversion(type, {"185542587100800,-185542587187200\n"}, {{INT32_MAX}, {INT32_MIN}}, options); AssertConversionError(type, {"185542587187200,-185542587273600\n"}, {0, 1}, options); + + // Leap seconds are rejected rather than wrapped into the next day + options.timestamp_parsers = {TimestampParser::MakeStrptime("%Y-%m-%d %H:%M:%S")}; + AssertConversionError(type, {"2020-12-31 23:59:60\n"}, {0}, options); } TEST(Date64Conversion, Basics) { @@ -689,6 +693,12 @@ TEST(Time32Conversion, UserDefinedParsers) { options); AssertConversionError(time64(TimeUnit::NANO), {"2020-03-15 14:30:00.5.5\n"}, {0}, options); + + // Leap seconds are rejected, as without parsers, rather than wrapped into the + // next minute + options.timestamp_parsers = {TimestampParser::MakeStrptime("%H:%M:%S")}; + AssertConversionError(time32(TimeUnit::SECOND), {"23:59:60,12:30:60\n"}, {0, 1}, + options); } TEST(Time64Conversion, Micros) { diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index 195cdc843ac6..18fed4d6ac5d 100644 --- a/cpp/src/arrow/util/value_parsing.h +++ b/cpp/src/arrow/util/value_parsing.h @@ -826,6 +826,13 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, if (!allow_trailing_chars && static_cast(ret - clean_copy.c_str()) != length) { return false; } + // Reject leap seconds, which Arrow cannot represent, instead of adding them + // to the time of day (which wraps them into the next minute); consistent with + // the ISO-8601 parser. See GH-50146 for how strptime implementations and + // standards differ on these values. + if (result.tm_sec >= 60) { + 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) / diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc index b61f777685b7..5122c5945322 100644 --- a/cpp/src/arrow/util/value_parsing_test.cc +++ b/cpp/src/arrow/util/value_parsing_test.cc @@ -869,7 +869,11 @@ TEST(TimestampParser, StrptimeParser) { } // Unparseable strings - std::vector unparseables = {"foo", "5/1/2000", "5/1/2000 12:34:56:6"}; + std::vector unparseables = {"foo", "5/1/2000", "5/1/2000 12:34:56:6", + // Leap seconds are rejected, as by the + // ISO-8601 parser + "5/31/2000 12:30:60", "5/31/2000 23:59:60", + "5/31/2000 23:59:61"}; for (auto& value : unparseables) { int64_t dummy; ASSERT_FALSE((*parser)(value.c_str(), value.size(), TimeUnit::SECOND, &dummy)); From 20deafe068bec15c1044b509f9e97c3dd0bf47de Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Tue, 13 Oct 2026 13:56:42 +0300 Subject: [PATCH 09/14] Skip timestamp parsers that cannot produce the column's kind of value With timestamp_parsers applied to date and time columns, a value of the other kind was accepted with the missing part defaulted: "7:55:00" became 1900-01-01 in a date column and "15-OCT-15" or "2020-03-15" became midnight in a time column. Skip strptime parsers whose format has no date directive for date columns, or no time directive for time columns, and reject date-only ISO-8601 values for time columns. AM/PM alone does not count as a time directive, since it only qualifies an hour given by another directive. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/csv/converter.cc | 99 ++++++++++++++++++++++++++++- cpp/src/arrow/csv/converter_test.cc | 21 ++++++ cpp/src/arrow/csv/options.h | 7 +- docs/source/cpp/csv.rst | 4 ++ python/pyarrow/_csv.pyx | 8 ++- 5 files changed, 132 insertions(+), 7 deletions(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index af7921649408..2284c8cd6c6c 100644 --- a/cpp/src/arrow/csv/converter.cc +++ b/cpp/src/arrow/csv/converter.cc @@ -507,6 +507,82 @@ bool SplitISO8601Subseconds(const char* s, uint32_t* size, std::string_view* sub return true; } +// Whether a strptime format can produce a value of type T: a date for date +// types, a time of day for time types. Directives not listed here count as +// both, so that a usable parser is never skipped. +template +bool StrptimeFormatCanProduce(const char* format) { + const char* p = format; + while (*p != '\0') { + if (*p++ != '%') { + continue; + } + // Skip flags, field width and the E/O modifiers + while (*p != '\0' && std::strchr("_-0^#123456789EO", *p) != nullptr) { + ++p; + } + switch (*p == '\0' ? '\0' : *p++) { + case '\0': + return false; + case 'Y': + case 'y': + case 'C': + case 'G': + case 'g': + case 'm': + case 'b': + case 'B': + case 'h': + case 'd': + case 'e': + case 'j': + case 'U': + case 'W': + case 'V': + case 'D': + case 'F': + case 'x': + case 'v': + if constexpr (is_date_type::value) { + return true; + } + break; + case 'H': + case 'I': + case 'k': + case 'l': + case 'M': + case 'S': + case 'R': + case 'T': + case 'r': + case 'X': + if constexpr (is_time_type::value) { + return true; + } + break; + case 'a': + case 'A': + case 'u': + case 'w': + case 'z': + case 'Z': + case 'n': + case 't': + case '%': + case 'p': + case 'P': + // Weekday, zone offset, whitespace, literal '%', and AM/PM, which only + // qualifies an hour given by another directive + break; + default: + // %c, %s, %+ and directives not listed here + return true; + } + } + return false; +} + // Tries the ISO-8601 format first, then the user-defined timestamp parsers. // A timestamp produced by a user-defined parser is floored to the day // boundary for dates, and reduced to the time of day for times (consistent @@ -522,7 +598,7 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { concrete_type_(checked_cast(*type)), parse_unit_(GetParseUnit(concrete_type_)), ticks_per_day_(TicksPerDay(parse_unit_)), - parsers_(GetTimestampParsers(options_)) {} + parsers_(GetParsers(options_)) {} Status Decode(const uint8_t* data, uint32_t size, bool quoted, value_type* out) { TrimWhiteSpace(&data, &size); @@ -537,9 +613,11 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { uint32_t s_size = size; std::string_view subseconds; // Set the fractional seconds aside, so that any number of digits is - // accepted: dates discard them, times add them back below + // accepted: dates discard them, times add them back below. A date-only + // value ("YYYY-MM-DD") has no time of day. if (std::strcmp(parser->kind(), "iso8601") == 0 && - !SplitISO8601Subseconds(s, &s_size, &subseconds)) { + (!SplitISO8601Subseconds(s, &s_size, &subseconds) || + (is_time_type::value && s_size == 10))) { continue; } TimeUnit::type unit = parse_unit_; @@ -590,6 +668,21 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { protected: static constexpr int64_t kMillisPerDay = 86400000; + // The user-defined parsers, except strptime parsers whose format cannot + // produce a T, so that a value of the other kind is not silently accepted + // with the missing part defaulted + static std::vector GetParsers(const ConvertOptions& options) { + std::vector parsers; + for (const auto* parser : GetTimestampParsers(options)) { + if (std::strcmp(parser->kind(), "strptime") == 0 && + !StrptimeFormatCanProduce(parser->format())) { + continue; + } + parsers.push_back(parser); + } + return parsers; + } + static TimeUnit::type GetParseUnit(const T& type) { if constexpr (is_time_type::value) { // Parse in the time type's own unit, so that the time of day can be diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 239478c981c9..7f04e2a8c086 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -556,6 +556,13 @@ TEST(Date32Conversion, UserDefinedParsers) { // Leap seconds are rejected rather than wrapped into the next day options.timestamp_parsers = {TimestampParser::MakeStrptime("%Y-%m-%d %H:%M:%S")}; AssertConversionError(type, {"2020-12-31 23:59:60\n"}, {0}, options); + + // Parsers that cannot produce a date are not tried, so that a time does not + // silently become 1900-01-01 + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y"), + TimestampParser::MakeStrptime("%H:%M:%S")}; + AssertConversion(type, {"15-OCT-15\n"}, {{16723}}, options); + AssertConversionError(type, {"7:55:00\n"}, {0}, options); } TEST(Date64Conversion, Basics) { @@ -699,6 +706,20 @@ TEST(Time32Conversion, UserDefinedParsers) { options.timestamp_parsers = {TimestampParser::MakeStrptime("%H:%M:%S")}; AssertConversionError(time32(TimeUnit::SECOND), {"23:59:60,12:30:60\n"}, {0, 1}, options); + + // Parsers that cannot produce a time are not tried, and date-only ISO-8601 + // values are rejected, so that a date does not silently become midnight + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y"), + TimestampParser::MakeStrptime("%H:%M:%S")}; + AssertConversion(time32(TimeUnit::SECOND), {"7:55:00\n"}, + {{28500}}, options); + AssertConversionError(time32(TimeUnit::SECOND), {"15-OCT-15\n"}, {0}, options); + options.timestamp_parsers = {TimestampParser::MakeISO8601()}; + AssertConversionError(time32(TimeUnit::SECOND), {"2020-03-15\n"}, {0}, options); + + // A format with only AM/PM has no hour, so it is not tried for times + options.timestamp_parsers = {TimestampParser::MakeStrptime("%p")}; + AssertConversionError(time32(TimeUnit::SECOND), {"PM\n"}, {0}, options); } TEST(Time64Conversion, Micros) { diff --git a/cpp/src/arrow/csv/options.h b/cpp/src/arrow/csv/options.h index 18a203bc0833..80652034fad5 100644 --- a/cpp/src/arrow/csv/options.h +++ b/cpp/src/arrow/csv/options.h @@ -138,8 +138,11 @@ struct ARROW_EXPORT ConvertOptions { /// as date32, date64, time32 or time64, after the built-in ISO-8601 parser /// failed on a value. A timestamp produced by a fallback parser is floored /// to the day boundary for dates, and reduced to the time of day for times - /// (like casting a timestamp to a date or time type). Type inference of - /// date and time columns is not affected. + /// (like casting a timestamp to a date or time type). Parsers that cannot + /// produce the column's kind of value are skipped: strptime parsers without + /// a date directive for date columns, without a time directive for time + /// columns, and for time columns ISO-8601 values without a time of day. + /// Type inference of date and time columns is not affected. std::vector> timestamp_parsers; /// Create conversion options with default values, including conventional diff --git a/docs/source/cpp/csv.rst b/docs/source/cpp/csv.rst index cffa7f992c8d..1b4b84b05690 100644 --- a/docs/source/cpp/csv.rst +++ b/docs/source/cpp/csv.rst @@ -359,6 +359,10 @@ A timestamp produced by such a parser is floored to the day boundary for dates, and reduced to the time of day for times, like casting a timestamp to a date or time type. For example, with the parser ``"%m/%d/%Y %H:%M"``, the value ``03/15/2020 14:30`` in a date32 column yields ``2020-03-15``. +Parsers that cannot produce the column's kind of value are skipped: for +example, ``"%H:%M:%S"`` is not tried for date columns and ``"%d-%b-%y"`` is +not tried for time columns, and a date-only ISO-8601 value is rejected in a +time column. Type inference of date and time columns always uses the ISO-8601 formats only, regardless of :member:`ConvertOptions::timestamp_parsers`. diff --git a/python/pyarrow/_csv.pyx b/python/pyarrow/_csv.pyx index f8d5cbcec5b1..22744d470486 100644 --- a/python/pyarrow/_csv.pyx +++ b/python/pyarrow/_csv.pyx @@ -705,8 +705,12 @@ cdef class ConvertOptions(_Weakrefable): or time64, after the built-in ISO-8601 parser failed on a value. A timestamp produced by a fallback parser is floored to the day boundary for dates, and reduced to the time of day for times (like - casting a timestamp to a date or time type). Type inference of - date and time columns is not affected. + casting a timestamp to a date or time type). Parsers that cannot + produce the column's kind of value are skipped: strptime parsers + without a date directive for date columns, without a time directive + for time columns, and for time columns ISO-8601 values without a + time of day. Type inference of date and time columns is not + affected. Examples -------- From 1e9bf128495f650a6c7585836b009fc19180a120 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Tue, 13 Oct 2026 14:33:55 +0300 Subject: [PATCH 10/14] Reject impossible dates in the strptime timestamp parser strptime only checks the day of month to be in 1..31, and the parser built the date without validating it, so an impossible date rolled over into the next month: "30-FEB-15" became 2015-03-02 and "2021-02-29 10:00:00" became 2021-03-01, in date columns and, before this PR, in timestamp columns. The ISO-8601 parser already rejects such dates, so reject them here too. Formats without a day directive are unaffected (the day still defaults to the first). Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/csv/converter_test.cc | 10 ++++++++++ cpp/src/arrow/util/value_parsing.h | 13 ++++++++++--- cpp/src/arrow/util/value_parsing_test.cc | 23 +++++++++++++++++------ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 7f04e2a8c086..124d88e710f5 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -557,6 +557,12 @@ TEST(Date32Conversion, UserDefinedParsers) { options.timestamp_parsers = {TimestampParser::MakeStrptime("%Y-%m-%d %H:%M:%S")}; AssertConversionError(type, {"2020-12-31 23:59:60\n"}, {0}, options); + // Impossible dates are rejected rather than rolled over into the next month + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y")}; + AssertConversionError(type, {"30-FEB-15,31-APR-15,29-FEB-21\n"}, {0, 1, 2}, options); + options.timestamp_parsers = {TimestampParser::MakeStrptime("%Y-%m-%d %H:%M:%S")}; + AssertConversionError(type, {"2021-02-29 10:00:00\n"}, {0}, options); + // Parsers that cannot produce a date are not tried, so that a time does not // silently become 1900-01-01 options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y"), @@ -869,6 +875,10 @@ TEST(TimestampConversion, UserDefinedParsers) { options.timestamp_parsers.push_back(TimestampParser::MakeISO8601()); AssertConversion(type, {"01/02/1970,1970-01-03\n"}, {{86400000}, {172800000}}, options); + + // Impossible dates are rejected rather than rolled over into the next month + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y")}; + AssertConversionError(type, {"30-FEB-15\n"}, {0}, options); } TEST(TimestampConversion, UserDefinedParsersWithZone) { diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index 18fed4d6ac5d..f5aa14bf1b60 100644 --- a/cpp/src/arrow/util/value_parsing.h +++ b/cpp/src/arrow/util/value_parsing.h @@ -833,10 +833,17 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, if (result.tm_sec >= 60) { return false; } + // Reject impossible dates such as February 30, which strptime accepts (the + // day of month is only checked to be in 1..31), instead of rolling them over + // into the next month; consistent with the ISO-8601 parser. + const arrow_vendored::date::year_month_day ymd = + arrow_vendored::date::year(result.tm_year + 1900) / (result.tm_mon + 1) / + std::max(result.tm_mday, 1); + if (!ymd.ok()) { + 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)); + arrow_vendored::date::sys_seconds secs = arrow_vendored::date::sys_days(ymd); if (!ignore_time_in_day) { secs += (std::chrono::hours(result.tm_hour) + std::chrono::minutes(result.tm_min) + std::chrono::seconds(result.tm_sec)); diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc index 5122c5945322..b94337f75fb1 100644 --- a/cpp/src/arrow/util/value_parsing_test.cc +++ b/cpp/src/arrow/util/value_parsing_test.cc @@ -856,7 +856,8 @@ TEST(TimestampParser, StrptimeParser) { }; std::vector cases = {{"5/31/2000 12:34:56", "2000-05-31 12:34:56"}, - {"5/31/2000 00:00:00", "2000-05-31 00:00:00"}}; + {"5/31/2000 00:00:00", "2000-05-31 00:00:00"}, + {"2/29/2000 12:34:56", "2000-02-29 12:34:56"}}; for (auto unit : TimeUnit::values()) { for (const auto& case_ : cases) { @@ -869,15 +870,25 @@ TEST(TimestampParser, StrptimeParser) { } // Unparseable strings - std::vector unparseables = {"foo", "5/1/2000", "5/1/2000 12:34:56:6", - // Leap seconds are rejected, as by the - // ISO-8601 parser - "5/31/2000 12:30:60", "5/31/2000 23:59:60", - "5/31/2000 23:59:61"}; + std::vector unparseables = { + "foo", "5/1/2000", "5/1/2000 12:34:56:6", + // Leap seconds are rejected, as by the + // ISO-8601 parser + "5/31/2000 12:30:60", "5/31/2000 23:59:60", "5/31/2000 23:59:61", + // Impossible dates are rejected rather + // than rolled over into the next month + "2/30/2000 12:34:56", "4/31/2000 12:34:56", "2/29/2001 12:34:56"}; for (auto& value : unparseables) { int64_t dummy; ASSERT_FALSE((*parser)(value.c_str(), value.size(), TimeUnit::SECOND, &dummy)); } + + // A format without a day still parses (the day defaults to the first) + auto month_parser = TimestampParser::MakeStrptime("%Y-%m"); + int64_t converted, expected; + ASSERT_TRUE((*month_parser)("2000-02", 7, TimeUnit::SECOND, &converted)); + ASSERT_TRUE(ParseTimestampISO8601("2000-02-01", 10, TimeUnit::SECOND, &expected)); + ASSERT_EQ(expected, converted); } TEST(TimestampParser, StrptimeZoneOffset) { From 5e683d01858f12199675bcbf882574467eda1e96 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Tue, 13 Oct 2026 17:52:21 +0300 Subject: [PATCH 11/14] Reject a zero day in the strptime timestamp parser The day of month defaulted to the first when strptime left it at zero, so that formats without a day directive (such as "%Y-%m") still parse. This also turned an explicitly parsed zero day into the first of the month wherever strptime accepts "00" for %d (the vendored musl version used on Windows, and macOS): "00-FEB-15" became 2015-02-01 and "2020-01-00" became 2020-01-01, in date and timestamp columns. Default the day to the first before calling strptime instead, so that a parsed zero day is rejected as an impossible date. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/csv/converter_test.cc | 17 +++++++++++++++++ cpp/src/arrow/util/value_parsing.h | 11 +++++++---- cpp/src/arrow/util/value_parsing_test.cc | 5 ++++- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 124d88e710f5..a51fd6362f2c 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -563,6 +563,12 @@ TEST(Date32Conversion, UserDefinedParsers) { options.timestamp_parsers = {TimestampParser::MakeStrptime("%Y-%m-%d %H:%M:%S")}; AssertConversionError(type, {"2021-02-29 10:00:00\n"}, {0}, options); + // A zero day is rejected rather than taken as the first of the month + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y")}; + AssertConversionError(type, {"00-FEB-15\n"}, {0}, options); + options.timestamp_parsers = {TimestampParser::MakeStrptime("%Y-%m-%d")}; + AssertConversionError(type, {"2020-01-00\n"}, {0}, options); + // Parsers that cannot produce a date are not tried, so that a time does not // silently become 1900-01-01 options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y"), @@ -633,6 +639,12 @@ TEST(Date64Conversion, UserDefinedParsers) { AssertConversionError(type, {"9223372036915200,-9223372036915200,9223372036854775807\n"}, {0, 1, 2}, options); + + // A zero day is rejected rather than taken as the first of the month + options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y")}; + AssertConversionError(type, {"00-FEB-15\n"}, {0}, options); + options.timestamp_parsers = {TimestampParser::MakeStrptime("%Y-%m-%d")}; + AssertConversionError(type, {"2020-01-00\n"}, {0}, options); } TEST(Time32Conversion, Seconds) { @@ -879,6 +891,11 @@ TEST(TimestampConversion, UserDefinedParsers) { // Impossible dates are rejected rather than rolled over into the next month options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y")}; AssertConversionError(type, {"30-FEB-15\n"}, {0}, options); + + // A zero day is rejected rather than taken as the first of the month + AssertConversionError(type, {"00-FEB-15\n"}, {0}, options); + options.timestamp_parsers = {TimestampParser::MakeStrptime("%Y-%m-%d")}; + AssertConversionError(type, {"2020-01-00\n"}, {0}, options); } TEST(TimestampConversion, UserDefinedParsersWithZone) { diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index f5aa14bf1b60..625fbc60ea78 100644 --- a/cpp/src/arrow/util/value_parsing.h +++ b/cpp/src/arrow/util/value_parsing.h @@ -815,6 +815,9 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, std::string clean_copy(buf, length); struct tm result; memset(&result, 0, sizeof(struct tm)); + // Default the day to the first for formats without a day directive, so that + // an explicitly parsed zero day is not mistaken for an omitted one + result.tm_mday = 1; #ifdef _WIN32 char* ret = arrow_strptime(clean_copy.c_str(), format, &result); #else @@ -833,12 +836,12 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, if (result.tm_sec >= 60) { return false; } - // Reject impossible dates such as February 30, which strptime accepts (the - // day of month is only checked to be in 1..31), instead of rolling them over - // into the next month; consistent with the ISO-8601 parser. + // Reject impossible dates such as February 30 or a zero day, which strptime + // may accept (the day of month is checked against 31 at most), instead of + // rolling them over into the next month; consistent with the ISO-8601 parser. const arrow_vendored::date::year_month_day ymd = arrow_vendored::date::year(result.tm_year + 1900) / (result.tm_mon + 1) / - std::max(result.tm_mday, 1); + result.tm_mday; if (!ymd.ok()) { return false; } diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc index b94337f75fb1..ba10b1d9323b 100644 --- a/cpp/src/arrow/util/value_parsing_test.cc +++ b/cpp/src/arrow/util/value_parsing_test.cc @@ -877,7 +877,10 @@ TEST(TimestampParser, StrptimeParser) { "5/31/2000 12:30:60", "5/31/2000 23:59:60", "5/31/2000 23:59:61", // Impossible dates are rejected rather // than rolled over into the next month - "2/30/2000 12:34:56", "4/31/2000 12:34:56", "2/29/2001 12:34:56"}; + "2/30/2000 12:34:56", "4/31/2000 12:34:56", "2/29/2001 12:34:56", + // A zero day is rejected rather than + // taken as the first of the month + "2/00/2000 12:34:56"}; for (auto& value : unparseables) { int64_t dummy; ASSERT_FALSE((*parser)(value.c_str(), value.size(), TimeUnit::SECOND, &dummy)); From 292cd37b181f6683e684db009d7d3264588f42b3 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Tue, 13 Oct 2026 18:14:34 +0300 Subject: [PATCH 12/14] Reject years out of range in the strptime timestamp parser The year was computed as tm_year + 1900 in int and passed to date::year, which holds only 16 bits. With a format such as "%s", a large value overflowed the int addition (undefined behavior) or wrapped the year into the 16-bit range, so "1000000000000000" became the year -28716 and the year 69664 became 4128, in date and timestamp columns. Compute the year in 64 bits and reject it when date::year cannot hold it. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/util/value_parsing.h | 9 ++++++++- cpp/src/arrow/util/value_parsing_test.cc | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index 625fbc60ea78..7d3d38649c92 100644 --- a/cpp/src/arrow/util/value_parsing.h +++ b/cpp/src/arrow/util/value_parsing.h @@ -836,11 +836,18 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, if (result.tm_sec >= 60) { return false; } + // tm_year + 1900 can overflow int (e.g. with %s), and date::year only + // holds 16 bits + const int64_t year = static_cast(result.tm_year) + 1900; + if (year < static_cast(arrow_vendored::date::year::min()) || + year > static_cast(arrow_vendored::date::year::max())) { + return false; + } // Reject impossible dates such as February 30 or a zero day, which strptime // may accept (the day of month is checked against 31 at most), instead of // rolling them over into the next month; consistent with the ISO-8601 parser. const arrow_vendored::date::year_month_day ymd = - arrow_vendored::date::year(result.tm_year + 1900) / (result.tm_mon + 1) / + arrow_vendored::date::year(static_cast(year)) / (result.tm_mon + 1) / result.tm_mday; if (!ymd.ok()) { return false; diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc index ba10b1d9323b..4e68bf58d607 100644 --- a/cpp/src/arrow/util/value_parsing_test.cc +++ b/cpp/src/arrow/util/value_parsing_test.cc @@ -894,6 +894,23 @@ TEST(TimestampParser, StrptimeParser) { ASSERT_EQ(expected, converted); } +TEST(TimestampParser, StrptimeYearOutOfRange) { + auto parser = TimestampParser::MakeStrptime("%s"); + int64_t converted = 0; + if (!(*parser)("1584282600", 10, TimeUnit::SECOND, &converted)) { + GTEST_SKIP() << "strptime does not support %s on this platform"; + } + ASSERT_EQ(1584282600, converted); + + // Years that do not fit in a date::year are rejected rather than wrapped, + // also where tm_year + 1900 would overflow int + for (const std::string value : + {"1000000000000000", "67768036000000000", "67768036191676799"}) { + SCOPED_TRACE(value); + ASSERT_FALSE((*parser)(value.c_str(), value.size(), TimeUnit::SECOND, &converted)); + } +} + TEST(TimestampParser, StrptimeZoneOffset) { if (!kStrptimeSupportsZone) { GTEST_SKIP() << "strptime does not support %z on this platform"; From 71652c4c27641e8e68fa9d39b1c866ebc4a735f2 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Tue, 13 Oct 2026 19:05:01 +0300 Subject: [PATCH 13/14] Check for leap seconds only when the time of day is used The leap-second check in ParseTimestampStrptime also ran when ignore_time_in_day is true, which is how Gandiva's to_date calls it, so "2020-12-31 23:59:60" with "YYYY-MM-DD HH24:MI:SS" became invalid although the time part is thrown away. Move the check to where the time of day is added. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/util/value_parsing.h | 14 +++++++------- cpp/src/gandiva/to_date_holder_test.cc | 9 +++++++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index 7d3d38649c92..a3e882b190a6 100644 --- a/cpp/src/arrow/util/value_parsing.h +++ b/cpp/src/arrow/util/value_parsing.h @@ -829,13 +829,6 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, if (!allow_trailing_chars && static_cast(ret - clean_copy.c_str()) != length) { return false; } - // Reject leap seconds, which Arrow cannot represent, instead of adding them - // to the time of day (which wraps them into the next minute); consistent with - // the ISO-8601 parser. See GH-50146 for how strptime implementations and - // standards differ on these values. - if (result.tm_sec >= 60) { - return false; - } // tm_year + 1900 can overflow int (e.g. with %s), and date::year only // holds 16 bits const int64_t year = static_cast(result.tm_year) + 1900; @@ -855,6 +848,13 @@ static inline bool ParseTimestampStrptime(const char* buf, size_t length, // ignore the time part arrow_vendored::date::sys_seconds secs = arrow_vendored::date::sys_days(ymd); if (!ignore_time_in_day) { + // Reject leap seconds, which Arrow cannot represent, instead of adding them + // to the time of day (which wraps them into the next minute); consistent + // with the ISO-8601 parser. See GH-50146 for how strptime implementations + // and standards differ on these values. + if (result.tm_sec >= 60) { + return false; + } secs += (std::chrono::hours(result.tm_hour) + std::chrono::minutes(result.tm_min) + std::chrono::seconds(result.tm_sec)); #if !defined(_WIN32) && !defined(_AIX) diff --git a/cpp/src/gandiva/to_date_holder_test.cc b/cpp/src/gandiva/to_date_holder_test.cc index 1612d9b4f440..ada3e55d2fdc 100644 --- a/cpp/src/gandiva/to_date_holder_test.cc +++ b/cpp/src/gandiva/to_date_holder_test.cc @@ -85,6 +85,15 @@ TEST_F(TestToDateHolder, TestSimpleDateTime) { millis_since_epoch = to_date(&execution_context_, s.data(), (int)s.length(), true, &out_valid); EXPECT_EQ(millis_since_epoch, 534643200000); + + // A leap second does not make the value invalid, as the time is ignored + EXPECT_OK_AND_ASSIGN(auto to_date_holder24, + ToDateHolder::Make("YYYY-MM-DD HH24:MI:SS", 1)); + s = std::string("2020-12-31 23:59:60"); + millis_since_epoch = (*to_date_holder24)(&execution_context_, s.data(), (int)s.length(), + true, &out_valid); + EXPECT_TRUE(out_valid); + EXPECT_EQ(millis_since_epoch, 1609372800000); } TEST_F(TestToDateHolder, TestSimpleDate) { From 8ccc1ceb85c6d2eebd75b8ef32f9bf69b69f983e Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Tue, 13 Oct 2026 22:44:54 +0300 Subject: [PATCH 14/14] Skip the %s year-range test where strptime parses %s into 32 bits Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/util/value_parsing_test.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc index 4e68bf58d607..b248785fe85d 100644 --- a/cpp/src/arrow/util/value_parsing_test.cc +++ b/cpp/src/arrow/util/value_parsing_test.cc @@ -897,10 +897,12 @@ TEST(TimestampParser, StrptimeParser) { TEST(TimestampParser, StrptimeYearOutOfRange) { auto parser = TimestampParser::MakeStrptime("%s"); int64_t converted = 0; - if (!(*parser)("1584282600", 10, TimeUnit::SECOND, &converted)) { - GTEST_SKIP() << "strptime does not support %s on this platform"; + // A value past 2038 (2100-01-01), as a strptime that parses %s into a 32-bit + // time_t wraps larger values into ordinary dates + if (!(*parser)("4102444800", 10, TimeUnit::SECOND, &converted) || + converted != 4102444800) { + GTEST_SKIP() << "strptime does not support 64-bit %s values on this platform"; } - ASSERT_EQ(1584282600, converted); // Years that do not fit in a date::year are rejected rather than wrapped, // also where tm_year + 1900 would overflow int