[laughingman7743]

WHAT

Cursor, DictCursor, S3FSCursor, ArrowCursor, and PolarsCursor (and their async and aio variants, which share these result sets) now raise DataError naming the column when a value fails to convert, in execute() for the first page of Cursor and DictCursor and in the fetch methods otherwise, such as a timestamp with time zone value with an unknown zone or a local time that its zone repeats or skips:

DataError: Column 'col_timestamp_tz': '2020-11-01 01:30:00.000 America/New_York' is ambiguous: the local time occurs twice in America/New_York. ...

The message keeps the converter's text, which names the value and, for an unknown zone, the zone. The format is the one PandasCursor uses since #1273.

• AthenaResultSet._convert_values() converts the values of one row and names the column of a DataError. The GetQueryResults rows of AthenaResultSet and AthenaDictResultSet and the CSV rows of AthenaS3FSResultSet go through it. This merges the separate branches for rows with and without type hints.
• AthenaArrowResultSet._fetch() names the column of a failure in its per-row conversion; each row is still converted in column order.
• PandasCursor: with parse_dates or dtype given to execute() (even dtype={}), or with a custom timestamp with time zone conversion function, pandas applies the column's conversion function to each value as it reads. That function is now wrapped in _TimeZoneConverter, which raises a DataError naming the column and keeps it, and _read_csv() raises that error as it is instead of wrapping it in OperationalError. A DataError from a function given in converters is still OperationalError in a whole read. Before, such a value raised OperationalError without the column name in a whole read and DataError without it in a chunked read. With converters given, PyAthena's conversion functions are replaced and these values stay text, as before.
• Both conversions keep using generators, so a StopIteration from a converter still raises RuntimeError instead of ending an iteration of the rows.
• PolarsDataFrameIterator names the column in its per-column conversion generator; an error still raises when the row is iterated, so a chunked read raises when it reaches the chunk with the value.

WHY

Closes #1217 (part of #1212). Time zone conversion failures raised DataError without the column name, so the failing column of a wide result could not be identified. The PandasCursor part was done in #1273, except when pandas converts the values as it reads.

The contract documentation and release notes are #1221.

Cost, measured locally on Python 3.15 by calling the result sets' conversion directly on in-memory rows (no I/O), as the minimum of 9 repeats, before (b8cec9d) and after:

Path	Rows × columns	Before	After
GetQueryResults rows (Cursor)	1,000 × 8	4.0 ms	4.3 ms
GetQueryResults rows (DictCursor)	1,000 × 8	4.3 ms	4.6–4.7 ms
CSV rows (S3FSCursor)	10,000 × 8	40.5–41.1 ms	41.5–42.5 ms
Record batch (ArrowCursor)	10,000 × 6	41.2–43.4 ms	41.2–42.2 ms

The PandasCursor wrapper adds under 1% to converting 30,000 timestamp with time zone values (101.6 ms → 102.2 ms). A GetQueryResults page holds at most 1,000 rows, so the added 0.3 ms per page is small next to the API call. A per-row method returning a tuple measured faster for the CSV rows but would let a StopIteration from a converter end an iteration of the rows, so the generator stays.

TEST

Python 3.15, local.

• just lint: passed on df71232.
• df71232: uv run --env-file .env pytest -n 4 tests/pyathena/pandas tests/pyathena/aio/pandas: 444 passed. test_read_error checks that a whole read still raises other errors as OperationalError: a DataError from a mapped conversion function read with a time zone column, one from a converters function, and invalid converters. Making the identity check always true or dropping its Mapping check fails one of these cases; making it always false fails the whole-read dtype={} cases of test_time_zone_errors.
• 37e9778, a live script for SELECT at_timezone(TIMESTAMP '2020-11-01 06:30:00 UTC', 'America/New_York'): DataError: Column 'col_timestamp_tz': ... by default and with dtype={}, parse_dates=[], dtype={} with chunksize=1, and dtype={} with engine="python"; with converters={}, the value stays text. On b8cec9d, the dtype/parse_dates cases raised OperationalError (whole) or DataError (chunked) without the column name.
• f81e5af: pytest -n 4 tests/pyathena/test_cursor.py tests/pyathena/s3fs/test_cursor.py tests/pyathena/arrow tests/pyathena/aio/arrow tests/pyathena/polars/test_cursor.py -k "ambiguous or arrow": 146 passed. A stateful custom Arrow converter sees the values in the same order as on b8cec9d (script).
• 80b8f91: pytest -n 4 tests/pyathena/test_cursor.py tests/pyathena/test_async_cursor.py tests/pyathena/test_result_set.py tests/pyathena/s3fs tests/pyathena/arrow tests/pyathena/polars tests/pyathena/aio/test_cursor.py tests/pyathena/aio/s3fs tests/pyathena/aio/arrow tests/pyathena/aio/polars: 573 passed, 3 skipped.
• The test_ambiguous_time_zone_value tests of each non-pandas cursor now match ^Column 'col_timestamp_tz': .* is ambiguous, and TestDictCursor gains one; with the source changes reverted, all 6 fail.
• A converter that raises StopIteration gives RuntimeError from the Arrow fetch and the GetQueryResults rows, before and after (script).
• AWS CI passed on f81e5af and 8f6101f (test, test-sqla, test-sqla-async); it reruns when the PR is marked Ready again.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (behavior and implementation)

Base b8cec9dfc98fba2bcfd8470e82e1800ba02d85d0, initial head bc27a086e5e69ddc5706be5947db09943fa02652.

Covered: AthenaResultSet._get_rows / AthenaDictResultSet._get_rows (GetQueryResults rows, including the _fetch_all_rows fallback of S3FSCursor), AthenaS3FSResultSet._fetch, AthenaArrowResultSet._fetch, PolarsDataFrameIterator._iter_row_blocks; the async and aio cursors, which use these result sets; the four tightened tests and the new DictCursor test.

Checks:

• Rows with and without type hints: hints is None per column, so convert() is still called without type_hint for a column without one, as custom converters may not accept it.
• Truncation: the row still gives as many values as the shorter of the columns and its values; DictCursor still zips names over those values.
• PEP 479: a StopIteration from a converter still becomes RuntimeError in the generator, as in the previous generator expression.
• Arrow: the table's columns come from the description with strict=True (pyathena/arrow/result_set.py:302), so indexing columns by description position is safe; with no description (DML), no column is converted, as before.
• Polars: the error still raises when the row is iterated, so a chunked read raises at its chunk.

Result: FINDINGS (1, minor).

1. The try wrapped the yield in _convert_values and in the Polars convert generator, so a DataError thrown into the generator at the yield would also be renamed. Repaired in 914200e: the yield is outside the try.

Limitations: async and aio variants are not tested separately because they share these result sets. The full affected suites are running locally.

[laughingman7743]

Self-review round two (claims, callers, evidence)

Base b8cec9dfc98fba2bcfd8470e82e1800ba02d85d0, head 914200e121dcbecb5e20bbf91d10b25d52c362f9 (full pass), repaired at 80b8f91d96c6c5401665f1fe49dcccb1d9ce1442.

Claims checked:

• "async and aio variants share these result sets": AsyncCursor/AsyncDictCursor build AthenaResultSet/AthenaDictResultSet (pyathena/async_cursor.py:115,381); AthenaAioResultSet/AthenaAioDictResultSet inherit _get_rows (pyathena/aio/result_set.py:32,323); the async and aio S3FS, Arrow, and Polars cursors build the same result set classes. Holds.
• "the fetch methods raise": incorrect for Cursor/DictCursor, whose first page is converted in execute() (the tests raise there). PR body corrected.
• "a try per value is free; 3.97 → 4.12 ms": the numbers came from a standalone emulation, not the changed code. Re-measured the real result sets against b8cec9d: GetQueryResults +7%, DictCursor +8%, CSV rows +2–3%, Arrow −20%. PR body now gives these measurements and states their scope (local, no I/O).
• "the yield is outside the try": holds.

FINDINGS (1, introduced regression):

1. The column-wise Arrow conversion used a list comprehension, so a StopIteration from a custom converter escaped _fetch() as StopIteration; under for row in cursor, __next__ would end the iteration silently. On b8cec9d, the row-wise generator expression raised RuntimeError. Repaired in 80b8f91: a generator with a comment (and noqa: C400). Checked with a converter that raises StopIteration: RuntimeError for Arrow and the GetQueryResults rows on both b8cec9d and 80b8f91. A per-row tuple-returning method was faster for the CSV rows but has the same flaw, so _convert_values stays a generator, now documented with a comment.

Evidence: each tightened or added test was seen failing with the source change reverted (6 failures). Local runs use Python 3.15 only; other Python versions and the full suite run in CI when the PR is marked Ready. Docs: only docs/pandas.md documents this error; the cross-cursor contract page is #1221.

[laughingman7743]

Independent review (relayed): Codex CLI 0.163.0 (codex exec -s read-only, model gpt-6-astra, reasoning effort max), session 01a14db5-b194-7502-b3a3-1d170d2ddc3e. Static review only: no edits, builds, tests, or network. Snapshot: detached worktree at head 80b8f91d96c6c5401665f1fe49dcccb1d9ce1442, base b8cec9dfc98fba2bcfd8470e82e1800ba02d85d0; the prompt gave the diff, intended behavior, and test policy, without the PR text or prior findings.

Covered: Cursor/DictCursor, S3FS CSV and API fallback, Arrow batches, Polars eager and chunked iteration, Pandas conversion callers, async/aio delegation; hints, truncation, dict_type, duplicate names, empty/DML results, exception boundaries, hot-path structure.

Result: FINDINGS

• P2 pyathena/arrow/result_set.py:224 (at 80b8f91): converting a whole column before the next changes the order in which converters see values. With two VARCHAR rows [("a", "b"), ("c", "d")] and a converter lambda v: next(counter) over iter([1, 2, 3, 4]), the base returns [(1, 2), (3, 4)] and the head [(1, 3), (2, 4)]; it can also change which failure is reported first.

Non-actionable observations:

• No other regression or double wrapping found; hint omission, short-row truncation, dict construction, positional column attribution, and StopIteration → RuntimeError are preserved.
• The tightened and added tests cover distinct entry points and fail on the base; their regexes were not anchored to the start of the message.
• Pre-existing: PandasCursor with dtype={} (any given dtype, parse_dates, or converters) bypasses _TemporalConverter (pyathena/pandas/result_set.py:1233), so the error is an unprefixed OperationalError (:1418). This is outside the changed paths and is the Give PandasCursor's temporal columns one dtype per Athena type #1273 design noted on Name the column in DataError for time zone values in every cursor #1217.
• Performance impact was not verified by the reviewer.

[laughingman7743]

Repair in f81e5af: the Arrow fetch converts each row in column order again, through a per-row generator that names the column of a DataError; the no-converter path keeps list(zip(*columns)). The tests anchor the prefix with ^.

Self-review of the repair (both perspectives, scope: git range-diff b8cec9df..80b8f91d b8cec9df..f81e5afa):

• Behavior: the reviewer's scenario now gives [(1, 2), (3, 4)] on both b8cec9d and f81e5af (script). StopIteration from a converter is still RuntimeError. With no description (DML), no converter runs, as on the base. Names and converters come from the description by position, as the base's converters did.
• Claims: the PR body no longer claims an Arrow speedup; re-measured Arrow 10,000 x 6: 41.2-42.2 ms vs 41.2-43.4 ms on the base. The commit message states the reason.
• Validation on f81e5af: just lint passed; pytest -n 4 tests/pyathena/test_cursor.py tests/pyathena/s3fs/test_cursor.py tests/pyathena/arrow tests/pyathena/aio/arrow tests/pyathena/polars/test_cursor.py -k "ambiguous or arrow": 146 passed.

The Pandas dtype/parse_dates/converters case is deferred as pre-existing (#1273's design, noted on #1217). An independent follow-up of this repair follows.

[laughingman7743]

Independent follow-up (relayed): Codex CLI 0.163.0 (codex exec -s read-only), session 01a14dbe-118f-7ba3-bbac-194f984d8826. Static review only. Snapshot: detached worktree at f81e5afa60d63e2180a6054830078ea93fb1c086; scope: the repair diff 80b8f91..f81e5af and git range-diff b8cec9df..80b8f91d b8cec9df..f81e5afa.

Result: CLEAN. Covered the Arrow fetch against b8cec9d (fetch and error propagation, converter order, default and DML paths, buffering) and the five anchored test assertions. The repair restores row-by-row conversion in column order; only a converter's DataError is wrapped, with the original as the cause; StopIteration still becomes RuntimeError; the list(zip(*columns)) path is intact. The reviewer did not measure performance. The snapshot and the PR worktree stayed unchanged.

[laughingman7743]

Self-review round one, scope expansion (behavior and implementation): PandasCursor when pandas converts the values as it reads. Range f81e5afa60d63e2180a6054830078ea93fb1c086..d555a2c29e617881f0e95c854942e080f2ed9afb on base b8cec9dfc98fba2bcfd8470e82e1800ba02d85d0.

Covered: _TimeZoneConverter, both converter sites (_get_csv_read_options when pandas resolves the labels, _key_csv_columns_by_labels otherwise), _read_json_columns_as_text (keeps non-JSON converters as they are), the PyArrow engine (it pops converters, and is not chosen while a timestamp with time zone column has a read converter), the GetQueryResults fallback (same read_csv), and the except DataError passthrough in _read_csv().

Checks:

• No other DataError can arise inside that try: _fetch_all_rows_as_csv(), whose _parse_result_rows() raises DataError for a malformed response, runs before it. The ExitStack still closes the stream on the way out.
• A whole read now raises DataError from the constructor, as the default path's _finish_csv_frame() already did; a chunked read raises it through PandasDataFrameIterator._read_chunk(), which marks the read failed as before.
• With converters given, PyAthena's converters are replaced, so a timestamp with time zone value stays text (live check); unchanged.
• Cost: the wrapper adds one call per value of timestamp with time zone columns only, under 1% of _to_datetime_with_tz (101.6 ms -> 102.2 ms for 30,000 values).

Result: FINDINGS (1, docstring). _get_csv_converter()'s Returns did not mention _TimeZoneConverter; repaired in d555a2c.

Tests: the dtype={} cases fail without the change (12), and each converter site's cases fail when only that site skips the wrapper.

[laughingman7743]

Self-review round two, scope expansion (claims, callers, evidence). Same range as round one.

Claims checked:

• "OperationalError without the column name in a whole read, and DataError without it in a chunked read" (commit message, PR body): reproduced live on b8cec9d with dtype={}, parse_dates=[], engine="python", and chunksize=1; after the change all give DataError: Column 'col_timestamp_tz': ....
• "even dtype={}": _get_csv_temporal_converters() treats any non-None option as given (pyathena/pandas/result_set.py, "None is the default of each option"). Holds.
• "with a custom conversion function": _get_csv_temporal_converters() skips a timestamp with time zone column whose mapping is not _to_datetime_with_tz, so read_csv gets that function, wrapped. Holds by construction; not tested separately.
• Documentation: docs/pandas.md already states that such a value raises DataError naming the column, without an exception for these options, and the section on parse_dates/dtype/converters says nothing about errors; it is now accurate as written.
• Callers: DataError is a DatabaseError, like OperationalError, so except DatabaseError still catches it; except OperationalError does not, which is the intended contract change (Name the column in DataError for time zone values in every cursor #1217) and matches the default path since Give PandasCursor's temporal columns one dtype per Athena type #1273.

Result: CLEAN. Validation: just lint passed; pytest -n 4 tests/pyathena/pandas tests/pyathena/aio/pandas: 441 passed on 067a1ef (d555a2c changes a docstring only). The scope expansion goes to an independent review next.

[laughingman7743]

Independent review of the scope expansion (relayed): Codex CLI 0.163.0 (codex exec -s read-only), session 01a14dfc-3524-7292-85fa-47901996c1d6. Static review only. Snapshot: detached worktree at d555a2c29e617881f0e95c854942e080f2ed9afb, base b8cec9dfc98fba2bcfd8470e82e1800ba02d85d0; scope f81e5afa..d555a2c2 (PandasCursor) with the full diff for context; no PR text or prior findings.

Covered: both converter builders, whole and chunked reads and fetch callers, converter overrides, duplicate labels, names/usecols, header-less .txt, json/binary handling, PyArrow, GetQueryResults fallback, cleanup, docstrings, test policy.

Result: FINDINGS

• P2 pyathena/pandas/result_set.py:1467 (at d555a2c): _read_csv() let every DataError through, not only the labeled one. A whole read with converters={"n": reject}, where reject raises DataError("invalid number"), raised OperationalError on the base and a raw DataError here; the comment "whose converter names the column" was false for it.

Non-actionable observations: the added parametrizations exercise independently built converter mappings and fail on the base (exception type in whole reads, missing label in chunked reads); successful conversions are unchanged; stream cleanup stays under the ExitStack and iterator failure handlers; performance not measured by the reviewer.

[laughingman7743]

Repair in 4dd1dac (and 8f6101f, a test docstring): _TimeZoneConverter raises _ColumnDataError, a private DataError subclass, and _read_csv() lets only that through; any other DataError in a whole read is OperationalError again, as on the base. test_converter_data_error covers the reviewer's scenario.

Self-review of the repair (both perspectives, scope git range-diff b8cec9df..d555a2c2 b8cec9df..8f6101f1):

• Behavior: _ColumnDataError is a DataError, so except DataError and except DatabaseError still catch the labeled error; chunked reads raise it through _read_chunk() unchanged. A custom timestamp with time zone mapping is wrapped too, so its DataError is labeled and passes through. A converters function's DataError in a chunked read is raw, as on the base (never wrapped there).
• Visible change: the labeled error's class name is _ColumnDataError in tracebacks of this path, while the default path raises plain DataError.
• Evidence: test_converter_data_error fails when _read_csv() lets every DataError through (checked) and passes on 8f6101f; just lint passed; pytest -n 4 tests/pyathena/pandas tests/pyathena/aio/pandas: 442 passed; the live script still gives the labeled error for dtype={}, parse_dates=[], chunked, and engine="python".

An independent follow-up of this repair follows.

[laughingman7743]

Independent follow-up (relayed): Codex CLI 0.163.0 (codex exec -s read-only), session 01a14e08-b175-7ab3-8b66-01571041678b. Static review only. Snapshot: detached worktree at 8f6101f1a556c3340e790ad06231a4224fb20abf; scope: the repair diff d555a2c..8f6101f and git range-diff b8cec9df..d555a2c2 b8cec9df..8f6101f1.

Result: CLEAN. Covered converter selection and overrides, whole-read wrapping, chunk propagation, and the tests. _TimeZoneConverter raises the private DataError subtype with the column prefix; the whole-read handler exempts only that subtype and wraps other DataErrors in OperationalError again; chunked reads keep the base's cleanup and re-raise; test_converter_data_error exercises a real whole CSV read with a caller-supplied converter and fails at d555a2c. The snapshot stayed unchanged.

[laughingman7743]

Repair at the maintainer's request in 37e9778: the labeled error's class is DataError again, as from the other paths, instead of the private _ColumnDataError. _TimeZoneConverter keeps the DataError it raised in error, and _read_csv() raises the caught exception as it is only when it is the error of one of the read's _TimeZoneConverters; any other exception is OperationalError as before.

Self-review of the repair (both perspectives, scope git range-diff b8cec9df..8f6101f1 b8cec9df..37e97783):

• Behavior: the converters that pandas calls are the objects in read_csv_kwargs["converters"] (_read_json_columns_as_text() rebuilds the dict but keeps non-JSON converters; _configure_binary_csv_read() wraps only binary columns), and read_csv_kwargs is built before the try. Converters are built per read, so error belongs to one read. Chunked reads raise the labeled DataError through _read_chunk() as before.
• Claims: PR body updated (no private subclass). The commit message's "as from the other paths" holds: the default path raises DataError from _TemporalConverter.convert().
• Evidence on 37e9778: just lint passed; pytest -n 4 tests/pyathena/pandas tests/pyathena/aio/pandas: 442 passed; the live script gives DataError: Column 'col_timestamp_tz': ... for dtype={}, parse_dates=[], chunked, and engine="python". Mutations: making the identity check always false fails the 6 whole-read test_time_zone_errors cases with dtype={}; making it accept any DataError fails test_converter_data_error.
• Upstream: master moved to d7f6710 (Raise OperationalError for failed manifest and Spark output reads (GH-1235) #1278 and a Spark change); the changes in pyathena/result_set.py (_read_data_manifest) and tests/pyathena/pandas/test_result_set.py (a pandas 3.0.0 condition) do not touch these contracts, and the branch merges cleanly.

An independent follow-up of this repair follows.

[laughingman7743]

Independent follow-up (relayed): Codex CLI 0.163.0 (codex exec -s read-only), session 01a14e29-acd8-7e43-b203-6d867689cf79. Static review only. Snapshot: detached worktree at 37e97783d41a57b8b9f75583b5249c9cf588b1e9; scope: repair diff 8f6101f..37e9778 and git range-diff b8cec9df..8f6101f1 b8cec9df..37e97783. Covered both converter builders, _key_csv_columns_by_labels, binary wrapping, JSON replacement, header relabeling, C/Python parsing, PyArrow selection, GetQueryResults fallback, whole/chunked propagation; the replacement paths keep the time zone converters' identity.

Result: FINDINGS

1. P2 pyathena/pandas/result_set.py:1474: with converters=[str] given to execute(), _read_json_columns_as_text() raises AttributeError, which the base wraps in OperationalError; the new handler called .values() on the list and leaked a second AttributeError.
2. P3 tests/pyathena/pandas/test_result_set.py:1111: replacing only converter.error is e with True passed the tests, because test_converter_data_error had no _TimeZoneConverter among its converters; a mixed case (a time zone column plus a mapped converter raising DataError, with dtype={}) was missing.

Repair in df71232: the handler scans the converters only when they are a Mapping. test_converter_data_error becomes test_read_error with three cases: mapping (the reviewer's mixed case), converters, and invalid_converters.

Self-review of the repair (both perspectives): mutations checked: identity always true fails test_read_error[mapping]; no Mapping guard fails test_read_error[invalid_converters]; identity always false still fails the whole-read dtype={} cases of test_time_zone_errors. The PR body's validation is updated. Evidence on df71232: just lint passed; pytest -n 4 tests/pyathena/pandas tests/pyathena/aio/pandas: 444 passed. An independent follow-up of this repair follows.

[laughingman7743]

Independent follow-up (relayed): Codex CLI 0.163.0 (codex exec -s read-only), session 01a14e34-2c8e-7492-89e1-7346e5eea1ac. Static review only. Snapshot: detached worktree at df712324f396a0127c1c6ee815c9b2c0056da2b5; scope: repair diff 37e9778..df71232 against base b8cec9d.

Result: CLEAN. Both findings resolved: the Mapping guard lets converters=[str] reach the base OperationalError wrapper, and the identity check still preserves only the time zone converter's recorded DataError; the mapping case of test_read_error would fail if only converter.error is e were replaced with True. The rewritten test uses declarative case parameters and builds converters, mocks, and result sets during execution. The snapshot stayed unchanged.