Skip to content

GH-41488: [C++][Python] Apply timestamp_parsers as fallback when parsing CSV date and time columns - #50146

Open
pearu wants to merge 15 commits into
apache:mainfrom
pearu:pearu/fix-csv-date-time-parsers
Open

pearu wants to merge 15 commits into
apache:mainfrom
pearu:pearu/fix-csv-date-time-parsers

Conversation

@pearu

@pearu pearu commented Jun 10, 2026 •

Copy link
Copy Markdown
Contributor

Rationale for this change

CSV columns explicitly typed as date32, date64, time32 or time64 can only be parsed from strict ISO-8601 strings: ConvertOptions::timestamp_parsers is consulted only for timestamp columns. Reading e.g. 15-OCT-15 into a date32 column fails even with timestamp_parsers=["%d-%b-%y"], and 7:55:00 (non-zero-padded hour) fails for time32[s]. Users currently work around this by declaring such columns as timestamp, reading, then casting back to the date/time type.

Effect on the issues collected in #41488:

What changes are included in this PR?

  • A new DateTimeWithParsersValueDecoder in csv/converter.cc, used for date32/date64/time32/time64 columns when timestamp_parsers is 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.
  • Parsers that cannot produce the column's kind of value are skipped: a strptime parser whose format has no date directive is not tried for date columns, one without a time directive is not tried for time columns (AM/PM alone does not count), and date-only ISO-8601 values are rejected in time columns. This prevents a value of the other kind from being silently accepted with the missing part defaulted (e.g. 7:55:00 becoming 1900-01-01 in a date column). Parsers of other kinds are always tried.
  • ISO-8601 fractional seconds (1 to 9 digits, which must end the value) are discarded for dates and kept for times; a time is parsed in seconds when its date is outside the range of the time unit (e.g. time64[ns] before 1677 or after 2262), and values outside the range of date32/date64 are rejected.
  • Type inference is unaffected: the inference stages create date/time converters with is_type_inference=true, which ignores timestamp_parsers (otherwise a value with a time-of-day part could be inferred as a date and silently truncated).
  • Documentation of the fallback and flooring semantics in ConvertOptions::timestamp_parsers (C++ and Python docstrings) and a new "Date and time parsing" section in the C++ CSV user guide.
  • C-locale name tables for the vendored musl strptime used on Windows, where nl_langinfo() is unavailable. Previously the %a/%A/%b/%B/%h/%p/%c/%r/%x/%X specifiers 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_LANGINFO allows compiling the fallback on Linux for manual testing (not exercised in CI).

Are these changes tested?

Yes:

  • New C++ tests (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.
  • New Python tests with the reproducers from [C++] Unable to read date64 or date32 in specific format from CSV #28303 and CSV reader cannot parse dates or times #41488, plus an inference-unchanged guard.
  • Regression tests for the strptime parser changes in value_parsing_test.cc (leap seconds, impossible dates, zero days, out-of-range years via %s, skipped where %s is unsupported) and in Gandiva's to_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

ParseTimestampStrptime added tm_sec to the time of day, so a seconds value of 60 or 61 rolled into the next minute: 2020-12-31 23:59:60 became 2021-01-01 in a date32 column. Likewise, strptime checks the day of month against 31 at most, so an impossible date rolled into the next month (30-FEB-15 with %d-%b-%y became 2015-03-02), and where strptime accepts day 00, it became the first of the month (00-FEB-15 became 2015-02-01). A year that does not fit in the 16 bits of date::year wrapped around, and computing it could overflow int, both reachable with %s (1000000000000000 became 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 of ParseTimestampStrptime:

caller effect
CSV timestamp columns with strptime timestamp_parsers (and the date/time columns added by this PR) conversion error
strptime compute function error, or null with error_is_null
Gandiva to_date impossible dates, zero days and out-of-range years only (it ignores the time of day); error, or null when errors are suppressed
Flight cookie Expires the cookie is treated as expired; RFC 6265 §5.1.1 also rejects seconds above 59, days below 1 and dates that do not exist

Arrow'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:60 was 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:

value ISO 8601-1:2019 RFC 3339 POSIX strptime glibc musl (Windows) Arrow ISO-8601 parser Python fromisoformat
…-12-31 23:59:60 valid (positive leap second) valid (leap second) accepted accepted accepted rejected rejected
12:30:60 not stated in §3 invalid accepted accepted accepted rejected rejected
…:61 not stated in §3 invalid rejected accepted rejected rejected rejected

Sources disagree on impossible dates in the same way:

value ISO 8601-1:2019 RFC 3339 POSIX strptime glibc musl (Windows) Arrow ISO-8601 parser Python (fromisoformat, time.strptime)
February 30, April 31 invalid (months have 28 to 31 days, §3) invalid (per-month maximum) accepted (%d is 01–31) accepted accepted rejected rejected
February 29, non-leap year invalid invalid accepted accepted accepted rejected rejected
day 00 not stated in §3 invalid (date-mday is 01–31) rejected (%d is 01–31) rejected accepted rejected rejected

Are there any user-facing changes?

Yes: ConvertOptions::timestamp_parsers now also applies, as a fallback after ISO-8601, to columns explicitly typed as date32/date64/time32/time64 (previously such values always errored). Behavior without timestamp_parsers is 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.

@pearu

pearu commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Two out-of-scope discoveries made while working on this, recorded here rather than folded into the PR to keep it minimal:

  1. MultipleParsersTimestampValueDecoder::Decode (pre-existing, csv/converter.cc) declares its zone_offset_present flag once outside the parser loop. The built-in parsers happen to write the out-parameter on every call (the strptime parser unconditionally, the ISO-8601 parser resets it to false before scanning), so this is currently harmless — but TimestampParser is a public interface, and a user-implemented parser that only writes the flag when an offset is found could observe a stale value from a previous loop iteration. The new decoder in this PR declares the flag per-iteration; the timestamp decoder could get the same two-line treatment as a MINOR follow-up.

  2. The "Timestamp inference/parsing" section of the C++ CSV user guide (docs/source/cpp/csv.rst) does not mention ConvertOptions::timestamp_parsers at all — custom timestamp parsing was undocumented in the user guide before the date/time subsection added here. A short paragraph there could be a docs follow-up.

@github-actions github-actions Bot added the awaiting review Awaiting review label Jun 10, 2026
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #41488 has been automatically assigned in GitHub to PR creator.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #41488 has been automatically assigned in GitHub to PR creator.

@pearu
pearu force-pushed the pearu/fix-csv-date-time-parsers branch from 78ca3cb to e0af29e Compare June 10, 2026 10:08
@pearu

pearu commented Jun 10, 2026 •

Copy link
Copy Markdown
Contributor Author

CI triage of the first run (3 failing jobs, all Windows): all three shared one root cause — the tests used the %d-%b-%y format from the original issue reproducer, but month names (%b) are not supported by the vendored musl strptime used on Windows: cpp/src/arrow/vendored/musl/strptime.c force-undefines HAVE_LANGINFO on _WIN32, which compiles out the %a/%A/%b/%B/%h/%c/%p/%r/%x/%X cases entirely. The feature code is unaffected; numeric-format and time tests passed on Windows.

Fixed (amended) by making numeric formats the primary test coverage and keeping the month-name reproducer guarded to non-Windows (#ifndef _WIN32 in C++, sys.platform != "win32" in Python, following the existing kStrptimeSupportsZone / test_strftime precedents).
UPDATE: sorry for this noise, Claude was too eager to post comments, it is better restrained now.

A third discovery for the list above: this %b limitation is pre-existing and applies equally to timestamp columns with timestamp_parsers on Windows — it just had no CI coverage because the existing timestamp tests only use numeric formats. Could deserve its own issue (either implementing C-locale month names in the vendored strptime, or documenting the limitation in timestamp_parsers docs).

@pearu
pearu force-pushed the pearu/fix-csv-date-time-parsers branch from e0af29e to e75ecec Compare June 10, 2026 13:19
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #41488 has been automatically assigned in GitHub to PR creator.

@pearu
pearu marked this pull request as ready for review June 10, 2026 14:13
@pearu
pearu requested review from AlenkaF, raulcd and rok as code owners June 10, 2026 14:13
@pearu

pearu commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

The single CI failure (AMD64 Conda C++ AVX2) is unrelated to this PR: Gandiva's TestTime.TestCastTimestampWithTZ fails identically on main since this morning (passing at 4e25461, failing from ca47cd1 on — see e.g. this main run). castTIMESTAMP_utf8 returns 0 for the Canada/Pacific tz name, pointing at tz-database resolution in the CI conda environment — code this PR does not touch.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_parsers after ISO parsing and applies flooring/time-of-day extraction.
  • Ensure CSV type inference for date/time remains ISO-only even when timestamp_parsers are configured.
  • Update C++/Python docs and add C++/Python tests; improve vendored Windows strptime support 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.

Comment thread cpp/src/arrow/csv/converter.cc
Comment thread python/pyarrow/tests/test_csv.py
@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jun 22, 2026
@pearu
pearu force-pushed the pearu/fix-csv-date-time-parsers branch from e75ecec to fcf4f95 Compare June 22, 2026 18:17
@pearu
pearu requested a review from pitrou as a code owner June 22, 2026 18:17
@pitrou

pitrou commented Jun 23, 2026

Copy link
Copy Markdown
Member

@jorisvandenbossche Do you want to take a look at this PR?

@manyi-w

manyi-w commented Jul 2, 2026

Copy link
Copy Markdown

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!

@pearu

pearu commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@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 timestamp_parsers is set explicitly (not the default).

Comment thread cpp/src/arrow/util/value_parsing.h Outdated
Comment thread cpp/src/arrow/util/value_parsing.h Outdated
Comment thread cpp/src/arrow/util/value_parsing.h Outdated
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 13, 2026
pearu added 3 commits October 13, 2026 17:52
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.
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #41488 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 13, 2026
@pearu
pearu requested a review from ianmcook October 13, 2026 16:34
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #41488 has been automatically assigned in GitHub to PR creator.

Comment thread cpp/src/arrow/util/value_parsing_test.cc Outdated
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 13, 2026
Drafted by Claude Code (an AI agent) and reviewed & approved by pearu.
@github-actions github-actions Bot added Component: Gandiva awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 13, 2026
@pearu

pearu commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@github-actions crossbow submit test-debian-13-cpp-i386

@github-actions

Copy link
Copy Markdown
Only contributors can submit requests to this bot. Please ask someone from the community for help with getting the first commit in.
The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/38457559027

@ianmcook

Copy link
Copy Markdown
Member

@github-actions crossbow submit test-debian-13-cpp-i386

@github-actions

Copy link
Copy Markdown

Revision: 8ccc1ce

Submitted crossbow builds: ursacomputing/crossbow @ actions-e78ab2e343

Task Status
test-debian-13-cpp-i386 GitHub Actions

@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 13, 2026
@github-actions github-actions Bot added awaiting merge Awaiting merge and removed awaiting changes Awaiting changes labels Oct 13, 2026
@ianmcook

Copy link
Copy Markdown
Member

@pitrou or @kou — I worked with @pearu to refine this. Could you please do a final review pass?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow ConvertOptions.timestamp_parsers for date types [C++] Unable to read date64 or date32 in specific format from CSV

5 participants