Do not convert ArrowCursor fallback values twice

[laughingman7743]

WHAT

AthenaArrowResultSet now records in __init__ whether its table was read from an S3 result file (_convert_rows).

• With an S3 result file, _fetch() converts the values with the converters for the result columns, as before. It resolves them when it fetches, so a converter.set() after execute() still applies.
• With the GetQueryResults fallback (no S3 output location, e.g. managed query result storage), _fetch() returns the values unchanged, because _fetch_all_rows() has already converted them with DefaultTypeConverter. The fetch methods now return the values that as_arrow() holds.

_fetch() also builds the converters dictionary once per record batch instead of once per cell.

Release note (4.0.0, fix): with managed query result storage, ArrowCursor fetches TIME, VARBINARY, and JSON object columns instead of raising TypeError, and no longer decodes JSON string scalars twice (broken since v3.28.0).

WHY

Closes #1021.

_as_arrow_from_api() builds the table from converted rows, and _fetch() converted them again with converters that expect strings. PolarsCursor had the same defect and was fixed in #1005.

TEST

Tested commit: 808cc34

• just lint: passed.
• TestArrowCursor::test_fetch_all_rows (default and managed) now selects TIME, VARBINARY, a json_parse object, and a CAST(text AS JSON) string scalar, and expects the same tuple in both modes. With the source change reverted, managed fails (TypeError: strptime() argument 1 must be str, not datetime.time) and default passes; with it, both pass.
• uv run --env-file .env pytest -n 4 tests/pyathena/arrow tests/pyathena/aio/arrow: 105 passed.
• Live, S3 path: a converter.set("json", ...) after execute() applies to the next fetchone(), as on master.
• Measured on master 785be36 before the change (managed work group): see the table in ArrowCursor converts GetQueryResults fallback values twice in the fetch methods #1021.
• Not run locally: the full just test pyathena suite; it runs in CI when the PR is marked Ready.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (implementation behavior): CLEAN

Base 33d3a0700b90f3fe73a98768bbc01d1581a17960, head 70a1b877bb8abb3d25f07fd1459ab9dc6ebbe548.

Covered: AthenaArrowResultSet.__init__ for the S3 CSV, UNLOAD Parquet, GetQueryResults fallback, and failed paths; _fetch(), fetchone()/fetchmany()/fetchall(), and close(); the sync, async, and aio Arrow cursors (all build this result set); the extended test_fetch_all_rows.

Checks:

• _row_converters is computed after _as_arrow() updates the UNLOAD metadata, so the S3 paths get the same converters that _fetch() used to build lazily (description does not change after __init__).
• Converter.get() always returns a function for a described column, so c(v) if c else v only skips conversion when the column has no converter, which happens only on the fallback path ({}).
• The fallback table comes from _fetch_all_rows() with DefaultTypeConverter, so the fetched values equal the as_arrow() values. Live: default and managed return the same tuple for TIME, VARBINARY, a JSON object, and a JSON string scalar.

Pre-existing, not changed: close() sets _batches = [] (a list, not an iterator), so a _fetch() after close() would raise TypeError: 'list' object is not an iterator. That is outside this diff.

[laughingman7743]

Self-review round two (claims, callers, operations): CLEAN

Base 33d3a0700b90f3fe73a98768bbc01d1581a17960, head 70a1b877bb8abb3d25f07fd1459ab9dc6ebbe548. Full pass over the PR body, the commit message, the comment, and #1021.

• "broken since v3.28.0": 83566ef1 (Fetching results does not work with Athena managed query result storage #664, managed query result storage) is first in v3.28.0.
• "_fetch() rebuilt converters for every cell": the base calls the converters property inside the per-value generator. The head looks up each column's converter once per record batch.
• The ArrowCursor converts GetQueryResults fallback values twice in the fetch methods #1021 table (TIME, VARBINARY, and JSON object fail; DATE and DECIMAL pass; a JSON string scalar is decoded twice) matches the measured base behavior. The extended test covers each failing type plus the string scalar.
• Callers: no public signature changes. The converters property is unchanged and still public. No additional AWS requests.
• No docs describe the fallback fetch values for ArrowCursor, so none need changes.

[laughingman7743]

Independent review (relayed): FINDINGS (1 regression, 3 pre-existing)

Reviewer: Codex CLI 0.160.0, model gpt-6-astra, reasoning effort high, sandbox read-only, session 01a101ff-9276-7291-8c1f-de87bbbae0cc. Static review only. Snapshot: detached worktree at head 70a1b877bb8abb3d25f07fd1459ab9dc6ebbe548, base 33d3a0700b90f3fe73a98768bbc01d1581a17960. The prompt contained the literal diff and the intended behavior only. Afterwards, the snapshot and the PR worktree were clean at 70a1b877.

Covered (reviewer): S3 CSV, UNLOAD Parquet and metadata replacement, API fallback, failed and empty construction, sync/async/aio callers, fetch batching, exhaustion, and close, column names, converters, and the changed test and comment.

Findings and author verification:

1. Regression, P2 pyathena/arrow/result_set.py:151: caching self.converters in __init__ ignores converter.set(...) calls made after execute(), whereas the base resolved the converters at fetch time. Verified by reading the code. Action: fix.
2. Pre-existing, P2 pyathena/result_set.py:748: _rows_to_columnar() keys the columns by name, so on the fallback SELECT 1 AS x, 2 AS x collapses into one column. Verified by reading the code. Action: report to the maintainer; out of scope.
3. Pre-existing, P2 pyathena/result_set.py:721: _fetch_all_rows() checks every page for a header row, while _pre_fetch() checks only the first page. A later page whose first row equals the column names loses that row. Verified by reading the code. Action: report to the maintainer; out of scope.
4. Pre-existing, P3 pyathena/arrow/result_set.py:432: close() assigns a list to _batches, so a later _fetch() raises TypeError. Already noted in round one; out of scope.

[laughingman7743]

Repair: 808cc34 (finding 1)

__init__ now records only self._convert_rows = bool(self.output_location). _fetch() resolves self.converters once per record batch when it converts rows, and leaves the rows unchanged on the fallback. A converter.set() after execute() reaches the next fetched batch again, and the converters are still built once per batch instead of once per cell.

Validation:

• just lint passed.
• Live, S3 path: with DefaultArrowTypeConverter, execute(json_parse(...)), then converter.set("json", lambda v: ("raw", v)), fetchone() returned (('raw', '{"a":1}'),).
• tests/pyathena/arrow and tests/pyathena/aio/arrow: 105 passed, including test_fetch_all_rows[managed].

Self-review of the repair: round one (behavior) and round two (claims) found nothing. The comment ("The fetch methods convert only the values read from a result file") matches both branches. The fix is now one boolean, with no cached state that a converter change can miss.

[laughingman7743]

Independent follow-up (relayed): CLEAN for 70a1b877..808cc34ffaec7b2d433d7232966a6716bd0bdc5a.

Reviewer: Codex CLI 0.160.0, gpt-6-astra, effort high, read-only, session 01a10207-a155-73d2-a876-602195cfec74. Static review; afterwards, the snapshot was clean at 808cc34f. Covered: S3 CSV/TXT, UNLOAD Parquet and schema metadata, the GetQueryResults fallback, failed and empty construction, converter mutation, batch buffering, fetch methods, exhaustion, close, and the comment. The S3 converters are resolved per fetched batch, so a converter.set() after execute() applies again. Rows already buffered keep their conversion, as on master. Fallback rows get no second conversion. No regressions.