Skip to content

Commit 78ca3cb

Browse files
pearuclaude
andcommitted
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. Closes GH-28303. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ca47cd1 commit 78ca3cb

7 files changed

Lines changed: 324 additions & 16 deletions

File tree

cpp/src/arrow/csv/converter.cc

Lines changed: 126 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "arrow/csv/converter.h"
1919

20+
#include <algorithm>
2021
#include <array>
2122
#include <cstring>
2223
#include <limits>
@@ -440,6 +441,13 @@ struct SingleParserTimestampValueDecoder : public ValueDecoder {
440441
const TimestampParser& parser_;
441442
};
442443

444+
std::vector<const TimestampParser*> GetTimestampParsers(const ConvertOptions& options) {
445+
std::vector<const TimestampParser*> parsers(options.timestamp_parsers.size());
446+
std::ranges::transform(options.timestamp_parsers, parsers.begin(),
447+
[](const auto& parser) { return parser.get(); });
448+
return parsers;
449+
}
450+
443451
struct MultipleParsersTimestampValueDecoder : public ValueDecoder {
444452
using value_type = int64_t;
445453

@@ -449,7 +457,7 @@ struct MultipleParsersTimestampValueDecoder : public ValueDecoder {
449457
: ValueDecoder(type, options, trie_cache),
450458
unit_(checked_cast<const TimestampType&>(*type_).unit()),
451459
expect_timezone_(!checked_cast<const TimestampType&>(*type_).timezone().empty()),
452-
parsers_(GetParsers(options_)) {}
460+
parsers_(GetTimestampParsers(options_)) {}
453461

454462
Status Decode(const uint8_t* data, uint32_t size, bool quoted, value_type* out) {
455463
bool zone_offset_present = false;
@@ -464,18 +472,93 @@ struct MultipleParsersTimestampValueDecoder : public ValueDecoder {
464472
}
465473

466474
protected:
467-
using ParserVector = std::vector<const TimestampParser*>;
475+
TimeUnit::type unit_;
476+
bool expect_timezone_;
477+
std::vector<const TimestampParser*> parsers_;
478+
};
479+
480+
//
481+
// Value decoder for dates and times, with fallback to user-defined
482+
// timestamp parsers
483+
//
484+
485+
// Tries the ISO-8601 format first, then the user-defined timestamp parsers.
486+
// A timestamp produced by a user-defined parser is floored to the day
487+
// boundary for dates, and reduced to the time of day for times (consistent
488+
// with casting a timestamp to date32/date64/time32/time64).
489+
template <typename T>
490+
struct DateTimeWithParsersValueDecoder : public ValueDecoder {
491+
using value_type = typename T::c_type;
492+
493+
DateTimeWithParsersValueDecoder(const std::shared_ptr<DataType>& type,
494+
const ConvertOptions& options,
495+
const TrieCache* trie_cache)
496+
: ValueDecoder(type, options, trie_cache),
497+
concrete_type_(checked_cast<const T&>(*type)),
498+
parse_unit_(GetParseUnit(concrete_type_)),
499+
ticks_per_day_(TicksPerDay(parse_unit_)),
500+
parsers_(GetTimestampParsers(options_)) {}
468501

469-
static ParserVector GetParsers(const ConvertOptions& options) {
470-
ParserVector parsers(options.timestamp_parsers.size());
471-
for (size_t i = 0; i < options.timestamp_parsers.size(); ++i) {
472-
parsers[i] = options.timestamp_parsers[i].get();
502+
Status Decode(const uint8_t* data, uint32_t size, bool quoted, value_type* out) {
503+
TrimWhiteSpace(&data, &size);
504+
if (ARROW_PREDICT_TRUE(string_converter_.Convert(
505+
concrete_type_, reinterpret_cast<const char*>(data), size, out))) {
506+
return Status::OK();
507+
}
508+
for (const auto& parser : parsers_) {
509+
int64_t timestamp = 0;
510+
bool zone_offset_present = false;
511+
if (parser->operator()(reinterpret_cast<const char*>(data), size, parse_unit_,
512+
&timestamp, &zone_offset_present) &&
513+
!zone_offset_present) {
514+
// Floor division, to handle values before the epoch
515+
int64_t days = timestamp / ticks_per_day_;
516+
days -= (timestamp % ticks_per_day_) < 0;
517+
if constexpr (std::is_same_v<T, Date32Type>) {
518+
*out = static_cast<value_type>(days);
519+
} else if constexpr (std::is_same_v<T, Date64Type>) {
520+
*out = days * kMillisPerDay;
521+
} else {
522+
static_assert(is_time_type<T>::value);
523+
*out = static_cast<value_type>(timestamp - days * ticks_per_day_);
524+
}
525+
return Status::OK();
526+
}
473527
}
474-
return parsers;
528+
return GenericConversionError(type_, data, size);
475529
}
476530

477-
TimeUnit::type unit_;
478-
bool expect_timezone_;
531+
protected:
532+
static constexpr int64_t kMillisPerDay = 86400000;
533+
534+
static TimeUnit::type GetParseUnit(const T& type) {
535+
if constexpr (is_time_type<T>::value) {
536+
// Parse in the time type's own unit, so that the time of day can be
537+
// extracted without further conversion
538+
return type.unit();
539+
} else {
540+
return TimeUnit::SECOND;
541+
}
542+
}
543+
544+
static int64_t TicksPerDay(TimeUnit::type unit) {
545+
switch (unit) {
546+
case TimeUnit::SECOND:
547+
return 86400LL;
548+
case TimeUnit::MILLI:
549+
return 86400000LL;
550+
case TimeUnit::MICRO:
551+
return 86400000000LL;
552+
case TimeUnit::NANO:
553+
return 86400000000000LL;
554+
}
555+
return -1; // unreachable
556+
}
557+
558+
const T& concrete_type_;
559+
arrow::internal::StringConverter<T> string_converter_;
560+
const TimeUnit::type parse_unit_;
561+
const int64_t ticks_per_day_;
479562
std::vector<const TimestampParser*> parsers_;
480563
};
481564

@@ -672,6 +755,24 @@ std::shared_ptr<Converter> MakeTimestampConverter(const std::shared_ptr<DataType
672755
}
673756
}
674757

758+
//
759+
// Concrete Converter factory for dates and times
760+
//
761+
762+
template <template <typename, typename> class ConverterType, typename T>
763+
std::shared_ptr<Converter> MakeDateTimeConverter(const std::shared_ptr<DataType>& type,
764+
const ConvertOptions& options,
765+
MemoryPool* pool) {
766+
if (options.timestamp_parsers.empty()) {
767+
// Default to ISO-8601
768+
return std::make_shared<ConverterType<T, NumericValueDecoder<T>>>(type, options,
769+
pool);
770+
}
771+
// Try ISO-8601 first, then the user-defined timestamp parsers
772+
return std::make_shared<ConverterType<T, DateTimeWithParsersValueDecoder<T>>>(
773+
type, options, pool);
774+
}
775+
675776
//
676777
// Concrete Converter factory for reals
677778
//
@@ -743,10 +844,6 @@ Result<std::shared_ptr<Converter>> Converter::Make(const std::shared_ptr<DataTyp
743844
NUMERIC_CONVERTER_CASE(Type::FLOAT, FloatType)
744845
NUMERIC_CONVERTER_CASE(Type::DOUBLE, DoubleType)
745846
REAL_CONVERTER_CASE(Type::DECIMAL, Decimal128Type, DecimalValueDecoder)
746-
NUMERIC_CONVERTER_CASE(Type::DATE32, Date32Type)
747-
NUMERIC_CONVERTER_CASE(Type::DATE64, Date64Type)
748-
NUMERIC_CONVERTER_CASE(Type::TIME32, Time32Type)
749-
NUMERIC_CONVERTER_CASE(Type::TIME64, Time64Type)
750847
NUMERIC_CONVERTER_CASE(Type::DURATION, DurationType)
751848
CONVERTER_CASE(Type::BOOL, (PrimitiveConverter<BooleanType, BooleanValueDecoder>))
752849
CONVERTER_CASE(Type::BINARY,
@@ -760,6 +857,22 @@ Result<std::shared_ptr<Converter>> Converter::Make(const std::shared_ptr<DataTyp
760857
ptr = MakeTimestampConverter<PrimitiveConverter>(type, options, pool);
761858
break;
762859

860+
case Type::DATE32:
861+
ptr = MakeDateTimeConverter<PrimitiveConverter, Date32Type>(type, options, pool);
862+
break;
863+
864+
case Type::DATE64:
865+
ptr = MakeDateTimeConverter<PrimitiveConverter, Date64Type>(type, options, pool);
866+
break;
867+
868+
case Type::TIME32:
869+
ptr = MakeDateTimeConverter<PrimitiveConverter, Time32Type>(type, options, pool);
870+
break;
871+
872+
case Type::TIME64:
873+
ptr = MakeDateTimeConverter<PrimitiveConverter, Time64Type>(type, options, pool);
874+
break;
875+
763876
case Type::STRING:
764877
if (options.check_utf8) {
765878
ptr = std::make_shared<PrimitiveConverter<StringType, BinaryValueDecoder<true>>>(

cpp/src/arrow/csv/converter_test.cc

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,35 @@ TEST(Date32Conversion, Errors) {
472472
AssertConversionError(date32(), {"2020-13-01\n"}, {0});
473473
}
474474

475+
TEST(Date32Conversion, UserDefinedParsers) {
476+
auto options = ConvertOptions::Defaults();
477+
const auto type = date32();
478+
479+
// Test a single parser
480+
options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y")};
481+
AssertConversion<Date32Type, int32_t>(type, {"15-OCT-15,18-JUN-90\n"},
482+
{{16723}, {7473}}, options);
483+
484+
// ISO-8601 values are still accepted when parsers are given
485+
AssertConversion<Date32Type, int32_t>(type, {"2020-03-15,15-OCT-15\n"},
486+
{{18336}, {16723}}, options);
487+
488+
// Test multiple parsers, with a pre-epoch value
489+
options.timestamp_parsers.push_back(TimestampParser::MakeStrptime("%d-%m-%Y"));
490+
AssertConversion<Date32Type, int32_t>(type, {"15-OCT-15,08-05-1945\n"},
491+
{{16723}, {-9004}}, options);
492+
493+
// Parsed timestamps are floored to the day boundary, also before the epoch
494+
options.timestamp_parsers = {TimestampParser::MakeStrptime("%m/%d/%Y %H:%M")};
495+
AssertConversion<Date32Type, int32_t>(type, {"03/15/2020 14:30,05/08/1945 14:30\n"},
496+
{{18336}, {-9004}}, options);
497+
498+
// Test errors
499+
AssertConversionError(type, {"24-12-2020\n"}, {0}, options);
500+
options.timestamp_parsers = {TimestampParser::MakeStrptime("%m/%d/%Y %z")};
501+
AssertConversionError(type, {"01/02/1970 +0000\n"}, {0}, options);
502+
}
503+
475504
TEST(Date64Conversion, Basics) {
476505
AssertConversion<Date64Type, int64_t>(date64(), {"1945-05-08\n", "2020-03-15\n"},
477506
{{-777945600000LL, 1584230400000LL}});
@@ -487,6 +516,33 @@ TEST(Date64Conversion, Errors) {
487516
AssertConversionError(date64(), {"2020-13-01\n"}, {0});
488517
}
489518

519+
TEST(Date64Conversion, UserDefinedParsers) {
520+
auto options = ConvertOptions::Defaults();
521+
const auto type = date64();
522+
523+
// Test a single parser
524+
options.timestamp_parsers = {TimestampParser::MakeStrptime("%d-%b-%y")};
525+
AssertConversion<Date64Type, int64_t>(type, {"15-OCT-15,18-JUN-90\n"},
526+
{{1444867200000LL}, {645667200000LL}}, options);
527+
528+
// ISO-8601 values are still accepted when parsers are given
529+
AssertConversion<Date64Type, int64_t>(type, {"2020-03-15,15-OCT-15\n"},
530+
{{1584230400000LL}, {1444867200000LL}}, options);
531+
532+
// Test multiple parsers, with a pre-epoch value
533+
options.timestamp_parsers.push_back(TimestampParser::MakeStrptime("%d-%m-%Y"));
534+
AssertConversion<Date64Type, int64_t>(type, {"15-OCT-15,08-05-1945\n"},
535+
{{1444867200000LL}, {-777945600000LL}}, options);
536+
537+
// Parsed timestamps are floored to the day boundary, also before the epoch
538+
options.timestamp_parsers = {TimestampParser::MakeStrptime("%m/%d/%Y %H:%M")};
539+
AssertConversion<Date64Type, int64_t>(type, {"03/15/2020 14:30,05/08/1945 14:30\n"},
540+
{{1584230400000LL}, {-777945600000LL}}, options);
541+
542+
// Test errors
543+
AssertConversionError(type, {"24-12-2020\n"}, {0}, options);
544+
}
545+
490546
TEST(Time32Conversion, Seconds) {
491547
const auto type = time32(TimeUnit::SECOND);
492548

@@ -513,6 +569,30 @@ TEST(Time32Conversion, Millis) {
513569
AssertConversionError(type, {"23:59:60\n"}, {0});
514570
}
515571

572+
TEST(Time32Conversion, UserDefinedParsers) {
573+
auto options = ConvertOptions::Defaults();
574+
575+
// Test a single parser, with non-zero-padded hours
576+
options.timestamp_parsers = {TimestampParser::MakeStrptime("%H:%M:%S")};
577+
AssertConversion<Time32Type, int32_t>(time32(TimeUnit::SECOND), {"7:55:00,12:01:02\n"},
578+
{{28500}, {43262}}, options);
579+
AssertConversion<Time32Type, int32_t>(time32(TimeUnit::MILLI), {"7:55:00\n"},
580+
{{28500000}}, options);
581+
582+
// ISO-8601 values are still accepted when parsers are given
583+
AssertConversion<Time32Type, int32_t>(time32(TimeUnit::SECOND), {"07:55:00,7:55:00\n"},
584+
{{28500}, {28500}}, options);
585+
586+
// The time of day is extracted from parsed timestamps, also before the epoch
587+
options.timestamp_parsers.push_back(TimestampParser::MakeStrptime("%Y-%m-%d %H:%M"));
588+
AssertConversion<Time32Type, int32_t>(time32(TimeUnit::SECOND),
589+
{"2020-03-15 07:55,1945-05-08 07:55\n"},
590+
{{28500}, {28500}}, options);
591+
592+
// Test errors
593+
AssertConversionError(time32(TimeUnit::SECOND), {"24:00:00\n"}, {0}, options);
594+
}
595+
516596
TEST(Time64Conversion, Micros) {
517597
const auto type = time64(TimeUnit::MICRO);
518598

@@ -539,6 +619,24 @@ TEST(Time64Conversion, Nanos) {
539619
AssertConversionError(type, {"23:59:60\n"}, {0});
540620
}
541621

622+
TEST(Time64Conversion, UserDefinedParsers) {
623+
auto options = ConvertOptions::Defaults();
624+
625+
// Test a single parser, with non-zero-padded hours
626+
options.timestamp_parsers = {TimestampParser::MakeStrptime("%H:%M:%S")};
627+
AssertConversion<Time64Type, int64_t>(time64(TimeUnit::MICRO), {"7:55:00\n"},
628+
{{28500000000LL}}, options);
629+
AssertConversion<Time64Type, int64_t>(time64(TimeUnit::NANO), {"7:55:00\n"},
630+
{{28500000000000LL}}, options);
631+
632+
// ISO-8601 values are still accepted when parsers are given
633+
AssertConversion<Time64Type, int64_t>(time64(TimeUnit::MICRO), {"07:55:00.123456\n"},
634+
{{28500123456LL}}, options);
635+
636+
// Test errors
637+
AssertConversionError(time64(TimeUnit::MICRO), {"24:00:00\n"}, {0}, options);
638+
}
639+
542640
TEST(TimestampConversion, Basics) {
543641
auto type = timestamp(TimeUnit::SECOND);
544642

cpp/src/arrow/csv/inference_internal.h

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,15 @@ enum class InferKind {
4646
class InferStatus {
4747
public:
4848
explicit InferStatus(const ConvertOptions& options)
49-
: kind_(InferKind::Null), can_loosen_type_(true), options_(options) {}
49+
: kind_(InferKind::Null), can_loosen_type_(true), options_(options) {
50+
if (!options.timestamp_parsers.empty()) {
51+
// Date and time inference must not use the user-defined timestamp parsers,
52+
// otherwise a value with a time-of-day (resp. date) part could be inferred
53+
// as a date (resp. time) and be silently truncated.
54+
date_time_options_ = std::make_unique<ConvertOptions>(options);
55+
date_time_options_->timestamp_parsers.clear();
56+
}
57+
}
5058

5159
InferKind kind() const { return kind_; }
5260

@@ -106,6 +114,12 @@ class InferStatus {
106114
return Converter::Make(type, options_, pool);
107115
};
108116

117+
auto make_date_time_converter =
118+
[&](std::shared_ptr<DataType> type) -> Result<std::shared_ptr<Converter>> {
119+
return Converter::Make(type, date_time_options_ ? *date_time_options_ : options_,
120+
pool);
121+
};
122+
109123
auto make_dict_converter =
110124
[&](std::shared_ptr<DataType> type) -> Result<std::shared_ptr<Converter>> {
111125
ARROW_ASSIGN_OR_RAISE(auto dict_converter,
@@ -122,9 +136,9 @@ class InferStatus {
122136
case InferKind::Boolean:
123137
return make_converter(boolean());
124138
case InferKind::Date:
125-
return make_converter(date32());
139+
return make_date_time_converter(date32());
126140
case InferKind::Time:
127-
return make_converter(time32(TimeUnit::SECOND));
141+
return make_date_time_converter(time32(TimeUnit::SECOND));
128142
case InferKind::Timestamp:
129143
return make_converter(timestamp(TimeUnit::SECOND));
130144
case InferKind::TimestampNS:
@@ -159,6 +173,9 @@ class InferStatus {
159173
InferKind kind_;
160174
bool can_loosen_type_;
161175
const ConvertOptions& options_;
176+
// Copy of options_ with timestamp_parsers cleared, for date and time inference.
177+
// Only allocated when custom timestamp parsers are configured.
178+
std::unique_ptr<ConvertOptions> date_time_options_;
162179
};
163180

164181
} // namespace csv

cpp/src/arrow/csv/options.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,13 @@ struct ARROW_EXPORT ConvertOptions {
129129
/// the CSV conversion logic will try parsing values starting from the
130130
/// beginning of this vector. If no parsers are specified, we use the default
131131
/// built-in ISO-8601 parser.
132+
///
133+
/// These parsers are also used as a fallback for columns explicitly typed
134+
/// as date32, date64, time32 or time64, after the built-in ISO-8601 parser
135+
/// failed on a value. A timestamp produced by a fallback parser is floored
136+
/// to the day boundary for dates, and reduced to the time of day for times
137+
/// (like casting a timestamp to a date or time type). Type inference of
138+
/// date and time columns is not affected.
132139
std::vector<std::shared_ptr<TimestampParser>> timestamp_parsers;
133140

134141
/// Create conversion options with default values, including conventional

0 commit comments

Comments
 (0)