Repository navigation
GH-41488: [C++][Python] Apply timestamp_parsers as fallback when parsing CSV date and time columns - #50146
GH-41488: [C++][Python] Apply timestamp_parsers as fallback when parsing CSV date and time columns#50146pearu wants to merge 15 commits into
Conversation
|
Two out-of-scope discoveries made while working on this, recorded here rather than folded into the PR to keep it minimal:
|
|
|
|
|
78ca3cb to
e0af29e
Compare
|
A third discovery for the list above: this |
e0af29e to
e75ecec
Compare
|
|
|
The single CI failure ( |
There was a problem hiding this comment.
Pull request overview
This PR extends the CSV reader’s ConvertOptions::timestamp_parsers behavior so that, when columns are explicitly typed as date32/date64/time32/time64, the reader first attempts the existing ISO-8601 parsing and then falls back to the user-provided timestamp parsers (with flooring/extracting semantics consistent with casting). It also keeps type inference strict (ISO-only) to avoid silent truncation.
Changes:
- Add a new C++ CSV date/time value decoder that falls back to
timestamp_parsersafter ISO parsing and applies flooring/time-of-day extraction. - Ensure CSV type inference for date/time remains ISO-only even when
timestamp_parsersare configured. - Update C++/Python docs and add C++/Python tests; improve vendored Windows
strptimesupport for C-locale day/month names.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/pyarrow/tests/test_csv.py | Adds Python coverage for date/time typed columns using timestamp_parsers fallback and inference guard. |
| python/pyarrow/_csv.pyx | Documents the new fallback behavior in Python ConvertOptions docstring. |
| docs/source/cpp/csv.rst | Adds a “Date and time parsing” section documenting fallback + semantics. |
| cpp/src/arrow/vendored/musl/strptime.c | Adds a C-locale nl_langinfo fallback table for Windows/testing to support %b/%B/%p/.... |
| cpp/src/arrow/csv/options.h | Documents fallback semantics for timestamp_parsers in C++ API docs. |
| cpp/src/arrow/csv/inference_internal.h | Ensures date/time inference ignores configured timestamp_parsers. |
| cpp/src/arrow/csv/converter.cc | Implements fallback decoder + converter factory changes for date/time types. |
| cpp/src/arrow/csv/converter_test.cc | Adds C++ tests for date/time fallback parsing behavior and edge cases. |
e75ecec to
fcf4f95
Compare
|
@jorisvandenbossche Do you want to take a look at this PR? |
|
Hi, a small quick question — I noticed that you mentioned multiple issues in the PR description. I was wondering, does this PR fix all of them? Thanks! |
|
@manyifire Good question — mostly, but not every one in the same way:
So: yes for #28303/#33357 (and #26783/#26224), yes-on-Windows for #31816/#31971, and for #37180 only when |
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.
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.
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.
|
|
|
|
Drafted by Claude Code (an AI agent) and reviewed & approved by pearu.
|
@github-actions crossbow submit test-debian-13-cpp-i386 |
|
|
@github-actions crossbow submit test-debian-13-cpp-i386 |
|
Revision: 8ccc1ce Submitted crossbow builds: ursacomputing/crossbow @ actions-e78ab2e343
|
Rationale for this change
CSV columns explicitly typed as
date32,date64,time32ortime64can only be parsed from strict ISO-8601 strings:ConvertOptions::timestamp_parsersis consulted only fortimestampcolumns. Reading e.g.15-OCT-15into adate32column fails even withtimestamp_parsers=["%d-%b-%y"], and7:55:00(non-zero-padded hour) fails fortime32[s]. Users currently work around this by declaring such columns astimestamp, reading, then casting back to the date/time type.Effect on the issues collected in #41488:
%d-%b-%y).date32column withtimestamp_parsers=["%Y/%m/%d"]— now works as requested).ArrowInvalid: CSV conversion error to date32[day]: invalid value '2000-01-01 00:00:00' #37180 is addressed but not auto-closed: it asks for ISO timestamp strings (2000-01-01 00:00:00) in adate32column to convert by default. With this PR that works by opting in viatimestamp_parsers=[ISO8601]; the no-parsers default still errors, deliberately, so that time-of-day truncation only happens when the user asked for it.%b/%Bfail on Windows) and [C++][Python] strptime fails to parse with %p on Windows #31971 (%pfails on Windows) are also resolved for alltimestamp_parsersusers;%zremains unsupported on Windows (kStrptimeSupportsZone, unchanged).What changes are included in this PR?
DateTimeWithParsersValueDecoderincsv/converter.cc, used for date32/date64/time32/time64 columns whentimestamp_parsersis non-empty. It tries the built-in ISO-8601 parser first (preserving all 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. Values carrying a zone offset are rejected, as for zone-less timestamp columns.7:55:00becoming1900-01-01in a date column). Parsers of other kinds are always tried.time64[ns]before 1677 or after 2262), and values outside the range of date32/date64 are rejected.is_type_inference=true, which ignorestimestamp_parsers(otherwise a value with a time-of-day part could be inferred as a date and silently truncated).ConvertOptions::timestamp_parsers(C++ and Python docstrings) and a new "Date and time parsing" section in the C++ CSV user guide.strptimeused on Windows, wherenl_langinfo()is unavailable. Previously the%a/%A/%b/%B/%h/%p/%c/%r/%x/%Xspecifiers were compiled out on Windows, so the month-name formats from the original issue reports (%d-%b-%y) could not work there for any column type. Name matching is case-insensitive.ARROW_TEST_FALLBACK_LANGINFOallows compiling the fallback on Linux for manual testing (not exercised in CI).Are these changes tested?
Yes:
Date32Conversion.UserDefinedParsers,Date64Conversion.UserDefinedParsers,Time32Conversion.UserDefinedParsers,Time64Conversion.UserDefinedParsers) covering custom formats, mixed ISO + custom values in one column (backward compatibility of ISO values when parsers are set), pre-epoch flooring with a time-of-day component, time-of-day extraction from pre-epoch timestamps, zone-offset rejection, and error cases.value_parsing_test.cc(leap seconds, impossible dates, zero days, out-of-range years via%s, skipped where%sis unsupported) and in Gandiva'sto_date_holder_test.cc(leap second with the time of day ignored).Behaviour change: the strptime parser rejects leap seconds, impossible dates and out-of-range years
ParseTimestampStrptimeaddedtm_secto the time of day, so a seconds value of 60 or 61 rolled into the next minute:2020-12-31 23:59:60became2021-01-01in adate32column. Likewise, strptime checks the day of month against 31 at most, so an impossible date rolled into the next month (30-FEB-15with%d-%b-%ybecame2015-03-02), and where strptime accepts day00, it became the first of the month (00-FEB-15became2015-02-01). A year that does not fit in the 16 bits ofdate::yearwrapped around, and computing it could overflowint, both reachable with%s(1000000000000000became the year −28716). All of these now fail to parse, except that a leap second is still accepted when the time of day is ignored. This affects every caller ofParseTimestampStrptime:timestampcolumns with strptimetimestamp_parsers(and thedate/timecolumns added by this PR)strptimecompute functionerror_is_nullto_dateExpiresArrow's format spec excludes leap seconds from time and timestamp values and leaves their correction to the caller, so there is no representation that keeps them. Rejecting seconds ≥ 60 makes the strptime parsers consistent with Arrow's ISO-8601 parser, with conversions without parsers (
23:59:60was already an error), and with Python. Impossible dates are likewise already rejected by the ISO-8601 parser.How standards and implementations differ
Sources disagree on leap seconds:
fromisoformat…-12-31 23:59:6012:30:60…:61Sources disagree on impossible dates in the same way:
fromisoformat,time.strptime)%dis 01–31)00date-mdayis 01–31)%dis 01–31)Are there any user-facing changes?
Yes:
ConvertOptions::timestamp_parsersnow also applies, as a fallback after ISO-8601, to columns explicitly typed as date32/date64/time32/time64 (previously such values always errored). Behavior withouttimestamp_parsersis untouched, ISO values keep parsing when parsers are set, and type inference is unchanged. One behavior change affects existing users of the strptime parser: leap seconds, impossible dates and out-of-range years are now rejected (see above). All language bindings gain the behavior without API changes.AI usage disclosure
This PR was developed with AI assistance (Claude Code): the decoder, tests and documentation were AI-generated under my direction, then reviewed line-by-line and iterated on by me (design decisions: fallback-after-ISO semantics, silent flooring, inference isolation, and several implementation details adjusted during review). I own and can debug these changes.