From 5b4967daf9d31f64c909c0f2a3f5a28691326978 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Mon, 19 Oct 2026 20:32:34 +0300 Subject: [PATCH 1/5] GH-54641: [C++] Support locale directives in the vendored strptime on Windows The vendored musl strptime, used on Windows, handles %a, %A, %b, %B, %h, %c, %p, %r, %x and %X only when nl_langinfo() is available, which it is not on Windows. Provide a C-locale replacement for the items used. ARROW_TEST_FALLBACK_LANGINFO compiles the replacement on other platforms. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/util/value_parsing_test.cc | 32 ++++++++++++++++++++++++ cpp/src/arrow/vendored/musl/strptime.c | 19 +++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc index b61f777685b7..6eb387fc0319 100644 --- a/cpp/src/arrow/util/value_parsing_test.cc +++ b/cpp/src/arrow/util/value_parsing_test.cc @@ -876,6 +876,38 @@ TEST(TimestampParser, StrptimeParser) { } } +TEST(TimestampParser, StrptimeLocaleDirectives) { + // Directives that use the names and formats of the C locale + struct Case { + std::string format; + std::string value; + std::string iso8601; + }; + + std::vector cases = { + {"%a %d %b %Y", "Sun 15 Feb 2015", "2015-02-15 00:00:00"}, + {"%A %d %B %Y", "Sunday 15 February 2015", "2015-02-15 00:00:00"}, + {"%d %h %Y", "15 Feb 2015", "2015-02-15 00:00:00"}, + {"%d-%b-%Y", "15-feb-2015", "2015-02-15 00:00:00"}, + {"%c", "Sun Feb 15 14:30:00 2015", "2015-02-15 14:30:00"}, + {"%m/%d/%Y %I:%M %p", "02/15/2015 02:30 PM", "2015-02-15 14:30:00"}, + {"%Y-%m-%d %r", "2015-02-15 02:30:00 PM", "2015-02-15 14:30:00"}, + {"%x", "02/15/15", "2015-02-15 00:00:00"}, + {"%Y-%m-%d %X", "2015-02-15 14:30:00", "2015-02-15 14:30:00"}, + }; + + for (const auto& case_ : cases) { + SCOPED_TRACE(case_.format); + auto parser = TimestampParser::MakeStrptime(case_.format); + int64_t converted, expected; + ASSERT_TRUE( + (*parser)(case_.value.c_str(), case_.value.size(), TimeUnit::SECOND, &converted)); + ASSERT_TRUE(ParseTimestampISO8601(case_.iso8601.c_str(), case_.iso8601.size(), + TimeUnit::SECOND, &expected)); + ASSERT_EQ(expected, converted); + } +} + TEST(TimestampParser, StrptimeZoneOffset) { if (!kStrptimeSupportsZone) { GTEST_SKIP() << "strptime does not support %z on this platform"; 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 From 1c34edb7a571aeeff9ffc463bfd6842e11aa8a74 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Mon, 19 Oct 2026 20:43:34 +0300 Subject: [PATCH 2/5] GH-54643: [C++] Reject impossible dates and a zero day in the strptime timestamp parser strptime checks the day of month against 31 at most, and some implementations (macOS, the vendored musl used on Windows) accept day 00. The parser rolled impossible dates such as February 30 over into the next month and took a zero day as the first of the month. Initialize the day to the first before parsing, so that a format without a day directive still gives the first of the month, and reject the result if it is not a valid date, as the ISO-8601 parser does. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/util/value_parsing.h | 16 +++++++++++++--- cpp/src/arrow/util/value_parsing_test.cc | 21 +++++++++++++++++++-- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index 195cdc843ac6..1b300074851a 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 @@ -826,10 +829,17 @@ 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 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) / + result.tm_mday; + 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 6eb387fc0319..db7a12c49ad9 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,7 +870,14 @@ 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", + // 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", + // 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)); @@ -908,6 +916,15 @@ TEST(TimestampParser, StrptimeLocaleDirectives) { } } +TEST(TimestampParser, StrptimeWithoutDay) { + // A format without a day directive gives the first of the month + auto parser = TimestampParser::MakeStrptime("%Y-%m"); + int64_t converted, expected; + ASSERT_TRUE((*parser)("2000-02", 7, TimeUnit::SECOND, &converted)); + ASSERT_TRUE(ParseTimestampISO8601("2000-02-01", 10, TimeUnit::SECOND, &expected)); + ASSERT_EQ(expected, converted); +} + TEST(TimestampParser, StrptimeZoneOffset) { if (!kStrptimeSupportsZone) { GTEST_SKIP() << "strptime does not support %z on this platform"; From 39d8b5be15fb762e3efa3477b7c506b91e404718 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Mon, 19 Oct 2026 20:59:43 +0300 Subject: [PATCH 3/5] GH-54647: [C++] 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. Arrow cannot represent leap seconds, and the ISO-8601 parser rejects them, so reject them here too. The check applies only when the time of day is used, so Gandiva's to_date, which ignores the time, still accepts such values. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/util/value_parsing.h | 6 ++++++ cpp/src/arrow/util/value_parsing_test.cc | 19 +++++++++++-------- cpp/src/gandiva/to_date_holder_test.cc | 9 +++++++++ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/cpp/src/arrow/util/value_parsing.h b/cpp/src/arrow/util/value_parsing.h index 1b300074851a..fae2dc141505 100644 --- a/cpp/src/arrow/util/value_parsing.h +++ b/cpp/src/arrow/util/value_parsing.h @@ -841,6 +841,12 @@ 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. + 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/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc index db7a12c49ad9..5af1c639ed20 100644 --- a/cpp/src/arrow/util/value_parsing_test.cc +++ b/cpp/src/arrow/util/value_parsing_test.cc @@ -870,14 +870,17 @@ TEST(TimestampParser, StrptimeParser) { } // Unparseable strings - std::vector unparseables = {"foo", "5/1/2000", "5/1/2000 12:34:56:6", - // 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", - // A zero day is rejected rather than - // taken as the first of the month - "2/00/2000 12:34:56"}; + std::vector unparseables = { + "foo", "5/1/2000", "5/1/2000 12:34:56:6", + // 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", + // A zero day is rejected rather than + // taken as the first of the month + "2/00/2000 12:34:56", + // 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)); 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 e99961dd703b6c1aefbde30af456d3ac0d8e898c Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Mon, 19 Oct 2026 21:56:20 +0300 Subject: [PATCH 4/5] GH-54650: [C++][Python] Use timestamp_parsers as a fallback for CSV date columns Columns explicitly typed as date32 or date64 were parsed only from the ISO-8601 format. Try ConvertOptions::timestamp_parsers after the ISO-8601 parser fails, and floor the parsed timestamp to the day boundary, as a cast from timestamp to date does. Strptime parsers without a date directive are skipped, so that a time is not accepted with a default date. Type inference of date columns is not affected. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/csv/converter.cc | 233 +++++++++++++++++++++++-- cpp/src/arrow/csv/converter.h | 11 +- cpp/src/arrow/csv/converter_test.cc | 160 +++++++++++++++++ cpp/src/arrow/csv/inference_internal.h | 7 +- cpp/src/arrow/csv/options.h | 7 + docs/source/cpp/csv.rst | 15 ++ python/pyarrow/_csv.pyx | 8 + python/pyarrow/tests/test_csv.py | 37 ++++ 8 files changed, 462 insertions(+), 16 deletions(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index 0b7b9f5c8af5..00b5649df44c 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 @@ -36,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 @@ -440,6 +442,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 +459,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 +474,189 @@ struct MultipleParsersTimestampValueDecoder : public ValueDecoder { } protected: - using ParserVector = std::vector; + TimeUnit::type unit_; + bool expect_timezone_; + std::vector parsers_; +}; + +// +// Value decoder for dates, with fallback to user-defined timestamp parsers +// + +// 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 +// anyway). +bool SplitISO8601Subseconds(const char* s, uint32_t* size, std::string_view* subseconds) { + if (*size <= 20 || s[19] != '.') { + return true; + } + const uint32_t fraction_length = *size - 20; + if (fraction_length > 9) { + return false; + } + for (uint32_t i = 20; i < *size; ++i) { + if (s[i] < '0' || s[i] > '9') { + return false; + } + } + *subseconds = std::string_view(s + 20, fraction_length); + *size = 19; + return true; +} + +// Whether a strptime format can produce a value of type T, i.e. a date. +// Directives not listed here count as producing one, 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': + // Time of day + 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 (consistent with casting a timestamp to date32/date64). +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)), + parsers_(GetParsers(options_)) {} - 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(); + 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; + const char* s = reinterpret_cast(data); + 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 + if (std::strcmp(parser->kind(), "iso8601") == 0 && + !SplitISO8601Subseconds(s, &s_size, &subseconds)) { + continue; + } + if (parser->operator()(s, s_size, TimeUnit::SECOND, ×tamp, + &zone_offset_present) && + !zone_offset_present) { + // Floor division, to handle values before the epoch + int64_t days = timestamp / kSecondsPerDay; + days -= (timestamp % kSecondsPerDay) < 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 { + static_assert(std::is_same_v); + if (arrow::internal::MultiplyWithOverflow(days, kMillisPerDay, out)) { + continue; // out of the range of date64 + } + } + return Status::OK(); + } + } + return GenericConversionError(type_, data, size); + } + + protected: + static constexpr int64_t kSecondsPerDay = 86400; + static constexpr int64_t kMillisPerDay = 86400000; + + // The user-defined parsers, except strptime parsers whose format cannot + // produce a date, so that a time is not silently accepted with the date + // 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; } - TimeUnit::type unit_; - bool expect_timezone_; + const T& concrete_type_; + arrow::internal::StringConverter string_converter_; std::vector parsers_; }; @@ -675,6 +856,27 @@ 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, + 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 part could be + // inferred as a date and be silently truncated. + 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 // @@ -715,7 +917,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()) { @@ -746,8 +949,6 @@ Result> Converter::Make(const std::shared_ptr> Converter::Make(const std::shared_ptr(type, options, pool); break; + case Type::DATE32: + ptr = MakeDateTimeConverter(type, options, pool, + is_type_inference); + break; + + case Type::DATE64: + ptr = MakeDateTimeConverter(type, options, pool, + is_type_inference); + break; + case Type::STRING: if (options.check_utf8) { ptr = std::make_shared>>( diff --git a/cpp/src/arrow/csv/converter.h b/cpp/src/arrow/csv/converter.h index c6254bd7ca1f..542a9ff4fa32 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 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/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 5dc078e7fd80..21d369b2a32f 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 @@ -472,6 +491,92 @@ 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); + + // 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); + + // 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); + + // 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); + + // 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); + + // 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"), + TimestampParser::MakeStrptime("%H:%M:%S")}; + AssertConversion(type, {"15-OCT-15\n"}, {{16723}}, options); + AssertConversionError(type, {"7:55:00\n"}, {0}, options); +} + TEST(Date64Conversion, Basics) { AssertConversion(date64(), {"1945-05-08\n", "2020-03-15\n"}, {{-777945600000LL, 1584230400000LL}}); @@ -487,6 +592,61 @@ 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); + + // 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); + + // 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); + + // 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) { const auto type = time32(TimeUnit::SECOND); diff --git a/cpp/src/arrow/csv/inference_internal.h b/cpp/src/arrow/csv/inference_internal.h index c08407ff7f6f..14b7db9b8060 100644 --- a/cpp/src/arrow/csv/inference_internal.h +++ b/cpp/src/arrow/csv/inference_internal.h @@ -106,6 +106,11 @@ class InferStatus { return Converter::Make(type, options_, pool); }; + auto make_date_time_converter = + [&](std::shared_ptr type) -> Result> { + return Converter::Make(type, options_, pool, /*is_type_inference=*/true); + }; + auto make_dict_converter = [&](std::shared_ptr type) -> Result> { ARROW_ASSIGN_OR_RAISE(auto dict_converter, @@ -122,7 +127,7 @@ 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)); case InferKind::Timestamp: diff --git a/cpp/src/arrow/csv/options.h b/cpp/src/arrow/csv/options.h index 41d0b63dacbb..2c63d536b8d9 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 or date64, after the built-in ISO-8601 parser failed on a + /// value. A timestamp produced by a fallback parser is floored to the day + /// boundary (like casting a timestamp to a date type). Strptime parsers + /// without a date directive are skipped. Type inference of date 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 74ee0bb4fbbf..0a2f5ea6fa8a 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 parsing +------------ + +Columns explicitly typed as date32 or date64 are parsed from the ISO-8601 +format (e.g. ``2021-01-01``). 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, like casting a timestamp to a date 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 a date are skipped: for +example, ``"%H:%M:%S"`` is not tried for date columns. + +Type inference of date columns always uses the ISO-8601 format only, +regardless of :member:`ConvertOptions::timestamp_parsers`. + Nulls ----- diff --git a/python/pyarrow/_csv.pyx b/python/pyarrow/_csv.pyx index 4789b9532e2e..d50a3ae88f8c 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 or date64, after + the built-in ISO-8601 parser failed on a value. A timestamp produced + by a fallback parser is floored to the day boundary (like casting a + timestamp to a date type). Strptime parsers without a date + directive are skipped. Type inference of date columns is not + affected. + Examples -------- diff --git a/python/pyarrow/tests/test_csv.py b/python/pyarrow/tests/test_csv.py index 275a7e808ddd..68fe76c0b79a 100644 --- a/python/pyarrow/tests/test_csv.py +++ b/python/pyarrow/tests/test_csv.py @@ -1165,6 +1165,43 @@ 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, GH-54650) + 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(self): # Times are inferred as time32[s] by default from datetime import time From ad4c8e0d6d51b4902448298ee8c04cd456bb91f6 Mon Sep 17 00:00:00 2001 From: Pearu Peterson Date: Mon, 19 Oct 2026 22:12:21 +0300 Subject: [PATCH 5/5] GH-54651: [C++][Python] Use timestamp_parsers as a fallback for CSV time columns Columns explicitly typed as time32 or time64 were parsed only from the ISO-8601 format. Try ConvertOptions::timestamp_parsers after the ISO-8601 parser fails, and take the time of day of the parsed timestamp, as a cast from timestamp to time does. Strptime parsers without a time directive, and ISO-8601 values without a time of day, are skipped, so that a date is not accepted as midnight. Type inference of time columns is not affected. Drafted by Claude Code (an AI agent) and reviewed & approved by pearu. --- cpp/src/arrow/csv/converter.cc | 112 +++++++++++++++++++------ cpp/src/arrow/csv/converter.h | 2 +- cpp/src/arrow/csv/converter_test.cc | 105 +++++++++++++++++++++++ cpp/src/arrow/csv/inference_internal.h | 2 +- cpp/src/arrow/csv/options.h | 13 +-- docs/source/cpp/csv.rst | 32 +++---- python/pyarrow/_csv.pyx | 14 ++-- python/pyarrow/tests/test_csv.py | 22 +++++ 8 files changed, 251 insertions(+), 51 deletions(-) diff --git a/cpp/src/arrow/csv/converter.cc b/cpp/src/arrow/csv/converter.cc index 00b5649df44c..2284c8cd6c6c 100644 --- a/cpp/src/arrow/csv/converter.cc +++ b/cpp/src/arrow/csv/converter.cc @@ -480,14 +480,15 @@ struct MultipleParsersTimestampValueDecoder : public ValueDecoder { }; // -// Value decoder for dates, with fallback to user-defined timestamp parsers +// Value decoder for dates and times, with fallback to user-defined +// timestamp parsers // // 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 -// anyway). +// 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 true; @@ -506,9 +507,9 @@ 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, i.e. a date. -// Directives not listed here count as producing one, so that a usable parser -// is never skipped. +// 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; @@ -556,7 +557,9 @@ bool StrptimeFormatCanProduce(const char* format) { case 'T': case 'r': case 'X': - // Time of day + if constexpr (is_time_type::value) { + return true; + } break; case 'a': case 'A': @@ -582,7 +585,8 @@ bool StrptimeFormatCanProduce(const char* format) { // 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 (consistent with casting a timestamp to date32/date64). +// 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; @@ -592,6 +596,8 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { 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_(GetParsers(options_)) {} Status Decode(const uint8_t* data, uint32_t size, bool quoted, value_type* out) { @@ -607,28 +613,51 @@ 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 + // 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; } - if (parser->operator()(s, s_size, TimeUnit::SECOND, ×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 / kSecondsPerDay; - days -= (timestamp % kSecondsPerDay) < 0; + 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 { - static_assert(std::is_same_v); + } else if constexpr (std::is_same_v) { 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 + int64_t time_of_day = timestamp % ticks_per_day; + if (time_of_day < 0) { + 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 * (ticks_per_day_ / ticks_per_day) + + subseconds_ticks); } return Status::OK(); } @@ -637,12 +666,11 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { } protected: - static constexpr int64_t kSecondsPerDay = 86400; static constexpr int64_t kMillisPerDay = 86400000; // The user-defined parsers, except strptime parsers whose format cannot - // produce a date, so that a time is not silently accepted with the date - // defaulted + // 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)) { @@ -655,8 +683,34 @@ struct DateTimeWithParsersValueDecoder : public ValueDecoder { 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 + // 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_; }; @@ -857,7 +911,7 @@ std::shared_ptr MakeTimestampConverter(const std::shared_ptr class ConverterType, typename T> @@ -867,8 +921,8 @@ std::shared_ptr MakeDateTimeConverter(const std::shared_ptr 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 part could be - // inferred as a date and be silently truncated. + // 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); } @@ -949,8 +1003,6 @@ Result> Converter::Make(const std::shared_ptr)) CONVERTER_CASE(Type::BINARY, @@ -974,6 +1026,16 @@ Result> Converter::Make(const std::shared_ptr(type, options, pool, + is_type_inference); + break; + + case Type::TIME64: + ptr = MakeDateTimeConverter(type, options, pool, + is_type_inference); + break; + case Type::STRING: if (options.check_utf8) { ptr = std::make_shared>>( diff --git a/cpp/src/arrow/csv/converter.h b/cpp/src/arrow/csv/converter.h index 542a9ff4fa32..472c33ae79b2 100644 --- a/cpp/src/arrow/csv/converter.h +++ b/cpp/src/arrow/csv/converter.h @@ -43,7 +43,7 @@ class ARROW_EXPORT Converter { std::shared_ptr type() const { return type_; } // Create a Converter for the given data type. - // If is_type_inference is true, date converters ignore + // 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, diff --git a/cpp/src/arrow/csv/converter_test.cc b/cpp/src/arrow/csv/converter_test.cc index 21d369b2a32f..8d3b01015eb5 100644 --- a/cpp/src/arrow/csv/converter_test.cc +++ b/cpp/src/arrow/csv/converter_test.cc @@ -673,6 +673,73 @@ 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); + + // 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); + + // 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); + + // 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); + + // 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) { const auto type = time64(TimeUnit::MICRO); @@ -699,6 +766,44 @@ 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); + + // 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); + + // 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) { auto type = timestamp(TimeUnit::SECOND); diff --git a/cpp/src/arrow/csv/inference_internal.h b/cpp/src/arrow/csv/inference_internal.h index 14b7db9b8060..bcd3a34f48ee 100644 --- a/cpp/src/arrow/csv/inference_internal.h +++ b/cpp/src/arrow/csv/inference_internal.h @@ -129,7 +129,7 @@ class InferStatus { case InferKind::Date: 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: diff --git a/cpp/src/arrow/csv/options.h b/cpp/src/arrow/csv/options.h index 2c63d536b8d9..80652034fad5 100644 --- a/cpp/src/arrow/csv/options.h +++ b/cpp/src/arrow/csv/options.h @@ -135,11 +135,14 @@ struct ARROW_EXPORT ConvertOptions { /// built-in ISO-8601 parser. /// /// These parsers are also used as a fallback for columns explicitly typed - /// as date32 or date64, after the built-in ISO-8601 parser failed on a - /// value. A timestamp produced by a fallback parser is floored to the day - /// boundary (like casting a timestamp to a date type). Strptime parsers - /// without a date directive are skipped. Type inference of date columns is - /// not affected. + /// 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). 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 0a2f5ea6fa8a..1b4b84b05690 100644 --- a/docs/source/cpp/csv.rst +++ b/docs/source/cpp/csv.rst @@ -348,20 +348,24 @@ values afterwards using the ``assume_timezone`` compute function. | | 2021-01-01T00:00:00Z | | +-------------------+------------------------------+-------------------+ -Date parsing ------------- - -Columns explicitly typed as date32 or date64 are parsed from the ISO-8601 -format (e.g. ``2021-01-01``). 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, like casting a timestamp to a date 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 a date are skipped: for -example, ``"%H:%M:%S"`` is not tried for date columns. - -Type inference of date columns always uses the ISO-8601 format only, -regardless of :member:`ConvertOptions::timestamp_parsers`. +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``. +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`. Nulls ----- diff --git a/python/pyarrow/_csv.pyx b/python/pyarrow/_csv.pyx index d50a3ae88f8c..22744d470486 100644 --- a/python/pyarrow/_csv.pyx +++ b/python/pyarrow/_csv.pyx @@ -701,11 +701,15 @@ cdef class ConvertOptions(_Weakrefable): ISO-8601 parser is used. These parsers are also used as a fallback when converting columns - explicitly typed (using `column_types`) as date32 or date64, after - the built-in ISO-8601 parser failed on a value. A timestamp produced - by a fallback parser is floored to the day boundary (like casting a - timestamp to a date type). Strptime parsers without a date - directive are skipped. Type inference of date columns is not + 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). 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 diff --git a/python/pyarrow/tests/test_csv.py b/python/pyarrow/tests/test_csv.py index 68fe76c0b79a..b299e502396c 100644 --- a/python/pyarrow/tests/test_csv.py +++ b/python/pyarrow/tests/test_csv.py @@ -1202,6 +1202,28 @@ def test_dates_with_timestamp_parsers(self): 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, GH-54651) + 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