[laughingman7743]

WHAT

Closes #1308.

• pyathena/converter.py: the timestamp, date, time, timestamp with time zone, and time with time zone converters parse with datetime.fromisoformat(), date.fromisoformat(), and time.fromisoformat() instead of datetime.strptime(). On Python 3.11 to 3.15, fromisoformat() reads Athena's text with 0 to 12 fractional digits and truncates digits beyond microseconds, as before. The private _parse_datetime() and _parse_time() helpers, added in 4.0.0, are removed.
• PandasCursor reads time columns of CSV results as text and converts them to time objects after reading, as it already does for timestamp, date, and timestamp with time zone columns. Before, pandas parsed them through parse_dates. pandas cannot infer a format for time text, so it fell back to dateutil for each value. With parse_dates, dtype, or converters given to execute(), pandas still parses them, and PyAthena converts the parsed values to time objects as before.
• _trunc_date(), which converts those parsed values, goes through _replace_by_label() and skips columns that pandas did not parse. This fixes three cases:
  • On master, parse_dates that does not name a time column raised AttributeError: Can only use .dt accessor with datetimelike values. The column now stays as pandas read it.
  • On master, index_col naming a time column raised KeyError. With the default options, the index now holds time objects.
  • With index_col naming a time column and parse_dates, dtype, or converters given, the parsed index level also becomes time objects.
• docs/pandas.md: the paragraph on parse_dates, dtype, and converters now includes time columns.

Behavior change

fromisoformat() accepts more ISO 8601 forms than the old strptime() formats, such as a T separator, basic forms like 20240102, and a trailing offset. Athena never writes these for these types, so only text from other sources is affected: such text now converts instead of raising ValueError.

WHY

#1308 reported the cost of the temporal conversion in the 4.2.0 benchmark. Profiling locally showed one cause for each section.

1. Row output: these functions convert each value in the rows of Cursor, DictCursor, and S3FSCursor, and of the async cursors, including nested values with type hints. They also convert the timestamp with time zone, time, and time with time zone values that ArrowCursor and PolarsCursor keep as text, and PandasCursor's time with time zone values. strptime() took 2.6 to 4.3 µs a value; the new code takes 0.1 to 1.1 µs. Values that pandas, pyarrow, or Polars parse themselves, such as PandasCursor's datetime64 columns and UNLOAD results, do not use these functions.
2. PandasCursor CSV reads: under cProfile, in a 1-million-row read the time column took 33.6 s of 37.6 s, almost all in dateutil. The timestamp, date, and timestamp with time zone conversions took 1.25 s.

Local measurements: Apple Silicon, Python 3.13, pandas 3.0.6, master 0a58be36 against 9ebd8ff3 of this PR, on the same interpreter. The later commit f5029cfe only keeps time values that pyarrow parsed and skips converted columns in _trunc_date(). On it, the 1M-row temporal whole read took 3.85–3.95 s in 3 runs.

Per-value conversion, the minimum of 5 repeats:

Value	master	this PR
timestamp 2024-01-02 03:04:05.123	3.79 µs	0.12 µs
date	2.65 µs	0.15 µs
timestamp with time zone, UTC / Asia/Tokyo	4.33 / 4.31 µs	0.59 / 0.60 µs
timestamp with time zone, +09:00	3.86 µs	0.31 µs
time	2.98 µs	0.11 µs
time with time zone	4.04 µs	1.06 µs

The data has the benchmark's temporal shape: three flat columns plus event_time, event_date, three timestamp with time zone columns (UTC, Asia/Tokyo, +09:00), and event_clock (time(3)).

Row conversion of 300,000 rows, the median of 5 repeats in one process:

Path	master	this PR
6 temporal columns, as in Cursor and S3FSCursor	7.04 s	0.92 s
4 per-value columns, as in ArrowCursor and PolarsCursor	4.77 s	0.66 s

PandasCursor reads of 1 million rows, the medians of 5 runs alternating between master and this PR:

Case	master time	PR time	master peak RSS	PR peak RSS
Whole read, flat	0.70 s	0.69 s	308 MiB	307 MiB
Whole read, temporal	14.31 s	3.78 s	732 MiB	790 MiB
chunksize=100000, temporal	14.25 s	3.77 s	611 MiB	631 MiB

The peak RSS of the temporal whole read varied from run to run: 678–853 MiB on master and 740–864 MiB with this PR. A second set of 5 runs gave medians of 709 and 806 MiB. RSS sampled at each conversion step points to allocator retention, not to memory that stays allocated:

• After read_csv(), master's RSS was sometimes much lower, once 352 MiB against about 610 MiB with this PR.
• The final RSS overlapped: 653–783 MiB on master and 760–806 MiB with this PR.

I tried two other time conversions:

• Converting in blocks of 10,000 values lowered the traced peak of the conversion itself from 97 to 54 MiB, but not the peak RSS of the read.
• A vectorized conversion through _to_timestamps() was no lower in peak RSS and slower: 0.20 s against 0.13–0.16 s per 1 million values.

This PR keeps the simple per-value conversion. The script that measures the reads also holds the CSV text in memory, which is about 300 MiB of each figure, as in the flat row.

The benchmark itself (the temporal shape at the large scale on r7g.2xlarge) was not rerun.

Release notes

• Faster conversion of timestamp, date, time, timestamp with time zone, and time with time zone values in the rows of Cursor, DictCursor, S3FSCursor, and their async variants, of the timestamp with time zone, time, and time with time zone values in the rows of ArrowCursor and PolarsCursor, and of time columns in PandasCursor's CSV reads (Reduce the cost of the temporal conversion in row output and PandasCursor CSV reads #1308).
• PandasCursor no longer fails when parse_dates leaves out a time column, or when index_col names a time column (Reduce the cost of the temporal conversion in row output and PandasCursor CSV reads #1308).

TEST

Tested commit: f5029cf, except where noted.

• just lint: passed.

• uv run --env-file .env pytest -n 4 tests/pyathena/test_converter.py tests/pyathena/pandas/test_result_set.py tests/pyathena/pandas/test_converter.py tests/pyathena/s3fs/test_result_set.py tests/pyathena/polars/test_result_set.py tests/pyathena/arrow/test_result_set.py tests/pyathena/test_result_set.py tests/pyathena/test_parser.py: 572 passed.

• Against Athena: uv run --env-file .env pytest -n 4 -k "temporal or time_zone or date or time" tests/pyathena, excluding the filesystem, sqlalchemy, aio/sqlalchemy, and spark directories: 219 passed, 7 skipped. This covers the temporal corpus tests of every sync and async cursor.

• On 3bdd65d, the first commit:

  • just docs lint: 0 errors. The docs did not change after it.
  • just benchmark test: 256 passed, 1 skipped.

• New and changed tests in tests/pyathena/pandas/test_result_set.py:

  • test_time_index, with the default options and with dtype={}
  • the time case of test_temporal_conversion_error
  • a time column in test_temporal_options_read_by_pandas
  • a TIME(0) column in test_temporal_options_none: with dtype=None, pyarrow parses it into time values, which must be kept

  With the source changes reverted, their cases that check the changed behavior fail with KeyError, without a DataError, or with AttributeError. With only the second commit reverted, the two dtype={} cases of test_time_index fail. With only the third commit reverted, test_temporal_options_none[dtype-pyarrow] fails.

• Updated the raw-read expectation of test_read_csv_with_pyarrow_matches_pandas[types-*]: with the default options, the time column is read as pyarrow text before the conversion.

• Not run locally: the full just test pyathena and the SQLAlchemy suites. The AWS CI runs them when this PR is marked Ready.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (implementation behavior): CLEAN

Base 0a58be36ff188fc1dc9541d1d28e5383500a8110, head 3bdd65d9e0d61628da7e3341c970c2238c481f90.

Covered:

• Behavior and failure paths:
  • Callers of the changed functions: _DEFAULT_CONVERTERS (Cursor, DictCursor, S3FSCursor, and the nested values of TypedValueConverter in pyathena/parser.py, which does not catch converter errors); the Arrow, Polars, and pandas mappings; and _to_utc_timestamps().
  • fromisoformat() on Python 3.11 and 3.14 for 0 to 12 fractional digits and years 1 and 9999. Results equal the old truncation.
  • Text outside Athena's formats (T separator, basic forms, offsets) is now accepted; noted under "Behavior change" in the PR description.
• Data boundaries, _to_times() (pyathena/pandas/result_set.py:454):
  • NULL from the C engine (NaN, with future.infer_string on and off), from pyarrow strings (pd.NA), and as an empty string becomes None.
  • Header-only and all-NULL time columns give the same dtype and values as master for whole, chunked, and engine="pyarrow" reads.
• Options:
  • dtype={}, converters={}, usecols, keep_default_na, chunksize, and engine="pyarrow" give the same values as master.
  • parse_dates=[] and parse_dates=["ts"] failed on master with AttributeError; a time column left out of parse_dates now stays text with the c, python, and pyarrow engines.
  • index_col="t" failed on master with KeyError; the index now holds time objects.
  • _get_fetch_types() leaves out the time columns, whose values are already time objects.
• Tests: with the source changes reverted, the 7 new or changed cases fail; the updated _types_frame expectation pins the raw text read.

Pre-existing and out of scope: since 36b23f0, the values.dtype.kind == "M" branches of _to_timestamps() and _to_utc_timestamps() (pyathena/pandas/result_set.py:432, :496) look unreachable, because the temporal converters are empty whenever parse_dates, dtype, or converters is given. _to_times() does not add such a branch. This PR leaves them unchanged.

[laughingman7743]

Round one follow-up on the repair 9ebd8ff3 (affected scope): CLEAN

git range-diff scope: _to_times() accepts parsed datetime values, and _trunc_date() calls _replace_by_label().

• Column that pandas parsed: _to_times() keeps master's .dt.time.astype(object).where(notna, None), so NULL is None as before.
• Text column that pandas did not parse: it is reassigned unchanged; parse_dates=[] and parse_dates=["ts"] still give text with the c, python, and pyarrow engines.
• Index level: _replace_by_label() rebuilds it, as for the other temporal columns.
• Labels that are absent or shared:
  • usecols that leaves the column out is skipped.
  • A duplicate label, for which df[name] is a DataFrame, is skipped by _replace_by_label()'s Series check. Duplicate CSV names get distinct labels from _key_csv_columns_by_labels().
• Tests: test_time_index[*-pandas_parses] fails with only the repair reverted, and passes on the head.

[laughingman7743]

Round one follow-up on the repair f5029cfe (independent-review repairs, affected scope): CLEAN

• _to_times(): pyarrow's time64 values come out of to_numpy(dtype=object) as datetime.time and are kept. datetime is not a subclass of time, so no other value type passes through. The order is unchanged: text, then time, then None for NaN and pd.NA.
• _trunc_date(): it skips only the labels in _csv_temporal_converters, which hold time objects. With parse_dates, dtype, or converters given, the mapping is empty, so pandas-parsed columns and index levels are still converted.
• Re-checked: the edge matrix (options, engines, usecols, index_col, chunks) and the dtype=None matrix with TIME(0) and TIME(3), whole and with index_col. Results match master, except for the intended fixes.

[laughingman7743]

Self-review round two (claims, callers, evidence): FINDINGS, repaired

Base 0a58be36ff188fc1dc9541d1d28e5383500a8110, reviewed head 3bdd65d9e0d61628da7e3341c970c2238c481f90, repaired head 9ebd8ff35c115d431af91572e9972729444690a1.

Claims checked: the PR description, the commit message, the docs/pandas.md paragraph, and the changed docstrings.

Findings:

1. The PandasCursor table did not belong to the published head. Its times and peak RSS came from a vectorized _to_times() that was later reverted. Repair: re-measured on 9ebd8ff3, with 5 runs alternating with master on the same interpreter. Peak RSS is reported as measured: whole-read medians 732 → 790 MiB, chunked 611 → 631 MiB, with the run-to-run ranges and the phase sampling that points to allocator retention. I tried blocks of 10,000 values, which lowered the traced peak of the conversion from 97 to 54 MiB but not the read's peak RSS, so the simple code stays.
2. Overclaim in "the fetch methods of every cursor". PandasCursor's datetime64 columns and UNLOAD results never call these functions. Repair: the WHY section and the release notes now name the actual callers.
3. The docs claim that "time columns that pandas parses become time objects" did not hold for an index level. With index_col="tm" and dtype={}, _trunc_date() skipped the parsed index, which stayed datetime64 with today's date. Repair: _trunc_date() goes through _replace_by_label(), as the after-read conversion does. test_time_index[*-pandas_parses] covers this and fails with only the repair reverted.

Also checked:

• fromisoformat() on Python 3.12, 3.13, and 3.15, in addition to 3.11 and 3.14: same results for 12 fractional digits.
• The removed private helpers _parse_datetime() and _parse_time() were added in 8b083aa (v4.0.0) and have no other callers in the repository.
• The micro-benchmark numbers were re-measured with the head's functions.

Re-run on 9ebd8ff3: just lint; 572 offline tests; and the Athena temporal subset of every cursor (219 passed, 7 skipped).

[laughingman7743]

Round two follow-up on the repair f5029cfe: CLEAN after correcting claims

• Correction: in this round's first pass, I wrote that the kind == "M" branches looked unreachable. That was wrong: dtype=None reaches them (see the independent review thread).
• Docstring check: the new _TemporalConverter sentence says that a column pyarrow already parsed with dtype=None "gets the same dtype". Checked for each type:
  • timestamp: datetime64 goes to as_unit("us").
  • timestamp with time zone: converted to UTC.
  • date: pyarrow's date objects take the text path of _to_timestamps().
  • time: time objects are kept.
• PR description: TEST now names f5029cfe and the added test_temporal_options_none case. The measurements stay attributed to 9ebd8ff3, with a timing check on f5029cfe.

[laughingman7743]

Independent review (relayed): Codex CLI

• Reviewer: codex exec -s read-only, Codex v0.163.0, model gpt-6-astra, reasoning effort max, session 01a154d0-2a67-7a33-b37a-3398e2e70d46.
• Scope: base 0a58be36ff188fc1dc9541d1d28e5383500a8110, head 9ebd8ff35c115d431af91572e9972729444690a1, a detached snapshot.
• Prompt: no PR number, description, commit messages, or prior findings.
• Static review only: no edits, builds, tests, or GitHub access. The snapshot stayed clean.
• Covered:
  • the shared converters through Cursor/DictCursor/S3FSCursor, nested typed conversion, Arrow/Polars/pandas, and the sync, threaded async, and aio callers;
  • NULL and empty strings, truncation, time zones, exception types, and column names;
  • whole and chunked reads, managed results, the three CSV engines, future.infer_string, option overrides, indexes, usecols, duplicate names, and the pandas fetch methods;
  • tests, docs, conventions, and allocation costs.
• No regression found in the scalar fromisoformat() substitutions for Athena's formats.

Result: FINDINGS (2 introduced by the diff)

1. P2. Times that pyarrow already parsed silently become NULL (pyathena/pandas/result_set.py, _to_times(); line 472 at the reviewed head).

• Scenario: engine="pyarrow" and dtype=None (which keeps _TemporalConverter but replaces the string dtype mapping), no chunksize, a result above the size threshold, and a TIME(0) column with 03:04:05. pyarrow infers time values, and the comprehension turned every non-string into None. At base, the column went through parse_dates and the datetime-to-time conversion.
• Verified: reproduced: master gives time(3, 4, 5), the reviewed head gave None in the DataFrame, in fetchone(), and in the index with index_col. TIME with a fraction was not affected.
• Repair f5029cfe:
  • _to_times() keeps time values.
  • test_temporal_options_none now has a TIME(0) column; its dtype-pyarrow case fails with the repair reverted and passes on the head.
  • The _TemporalConverter docstring now names the actual case for already parsed columns, dtype=None.

Pre-existing observation (P3 docs), folded into the same repair: the _TemporalConverter docstring said that a dtype given to execute() leads to already parsed columns, but a non-None dtype disables these converters.

Correction to my round-two record: I wrote that the kind == "M" branches of _to_timestamps() and _to_utc_timestamps() looked unreachable. They are reachable with dtype=None, as this finding shows.

[laughingman7743]

Independent follow-up (relayed): Codex CLI. CLEAN

• Reviewer: codex exec -s read-only, model gpt-6-astra, session 01a154dc-c629-72f0-abc5-5a3dab21b681.
• Scope: the repair only, 9ebd8ff35c115d431af91572e9972729444690a1..f5029cfefee44133cfddda1ccc9b9940c48c9191, base unchanged. Detached snapshot at f5029cfe, which stayed clean.
• Static review only: no tests, builds, edits, or GitHub access.
• Covered:
  • NULL handling;
  • the c, python, and pyarrow engines, and future.infer_string on and off;
  • whole and chunked reads, index_col including MultiIndex levels, usecols, and the fetch methods;
  • the changed docstrings and the test's distinctness.

Result:

• _to_times() keeps the time values that pyarrow parsed.
• _trunc_date() skips the labels that _TemporalConverter converted and still converts the time columns and index levels that pandas parsed.
• The docstrings are accurate.
• The amended test_temporal_options_none protects a distinct contract (TIME(0) with engine="pyarrow", dtype=None) and would receive None without the repair.

[laughingman7743]

Independent review (relayed), finding 2. P3. A converted time index is rebuilt again

• Location: pyathena/pandas/result_set.py, _trunc_date(); line 1630 at the reviewed head.
• Scenario: with index_col=["tm", "id"], _TemporalConverter converts tm and rebuilds the MultiIndex. _trunc_date() then called _replace_by_label() for tm again. Its callback returned the values unchanged, but the helper still materialized every level and called MultiIndex.from_arrays().
• Basis: static, not benchmarked by the reviewer.
• Verified: by reading the code. _trunc_date() runs after the after-read conversion, and a converted time column holds time objects, never datetimes.
• Repair f5029cfe: _trunc_date() skips the labels in _csv_temporal_converters. There is no added test, because the effect is cost, not values. test_time_index (default and dtype={}, whole and chunked) still passes.

Pre-existing, not changed: malformed scalar text such as 25:00:00 raises ValueError without a column name in row conversion (pyathena/result_set.py:1113, and the Arrow and Polars equivalents). Only DataError gets the column name. This was the same with strptime(), so it is left out of this PR.

Validation on f5029cfe:

• just lint
• 572 offline tests
• the Athena temporal subset of every cursor: 219 passed, 7 skipped
• a 1M-row temporal PandasCursor whole read: 3.85–3.95 s, unchanged