Keep columns with the same name, and key CSV column types by the reader's column labels

[laughingman7743]

WHAT

Fixes the three result set defects in #1032 for PandasCursor, ArrowCursor, and PolarsCursor, including their async and aio variants, which share these result sets. It also makes the column types follow the read options that rename or select columns.

1. Columns with the same name keep their own values, from the S3 result file and from the GetQueryResults fallback (managed query result storage).
   • pandas. When the read options parse the file as Athena writes it, _read_csv() resolves the labels that read_csv() gives the columns with pandas' own header parser (the existing _resolve_csv_column_names()). The labels cover renamed duplicates (x, x.1), names=, and usecols=. dtype, converters, parse_dates, the time truncation, and the binary NULL handling are keyed by these labels. The labels are resolved only when names repeat or such options are given.
     • dtype, converters, or parse_dates given to execute() are kept.
     • With custom parsing (quoting=QUOTE_NONE, another quotechar, dialect, skiprows, header), the keys stay as on master, so those values stay text as before (pinned by the existing test_binary_custom_* tests).
     • The pyarrow engine is not used when labels need resolving (repeated names, names=, usecols=, and the like). It does not rename duplicates and cannot read only the header, and with names= it applied the types by the original names.
     • Fetch reads rows by position.
   • Polars. schema_overrides is keyed by the CSV header with Polars' renamed duplicates (x_duplicated_0, from a parsed header line). The new_columns given to execute() are applied after reading, as Polars applies them (unless execute() also gives schema_overrides, which then reach Polars together as on master), because read_csv() and scan_csv() interpret a schema_overrides dict differently when new_columns renames only the first columns. The fetch converters are keyed by the resulting DataFrame names.
   • Arrow. Record batches and their fetch converters are taken by position instead of RecordBatch.to_pydict() and the name-keyed converters, which keep one column per name.
   • GetQueryResults fallback. The table or DataFrame is built from positional columns instead of _rows_to_columnar(), which merged same-named columns. It uses the column names of the S3 path: x, x.1 (pandas), x, x (Arrow), x, x_duplicated_0 (Polars). CSV read options do not apply to this path.
2. Only the first GetQueryResults page can carry the column labels. _fetch_all_rows() no longer drops a data row at the start of a later page because it equals the labels. This also applies to S3FSCursor's fallback.
3. Fetching from a closed AthenaArrowResultSet returns no rows instead of raising TypeError: 'list' object is not an iterator.

The public dtypes, converters, and parse_dates properties keep master's keys; the read paths build their own maps.

Also fixed by the same change (each failed on master):

• pandas names= that rename columns: types follow the renamed columns, e.g. names=["y", "x"] on SELECT 1 AS x, 'text' AS y.
• pandas usecols=: no KeyError in fetch, and no parse_dates or time truncation of unselected columns.
• Polars new_columns=: the types stay with their columns, including when new_columns renames only the first columns. With schema_overrides also given to execute(), both reach Polars as on master.
• pandas engine="pyarrow" with names=: falls back to the C engine instead of applying the types by the original names.
• Columns with the same name and different types now convert by their own types:
  • Polars: always.
  • pandas: when every such column has its own dtype or none of them has one, e.g. integer and varchar.
  • Arrow: on the managed path, and on the S3 path when they share an Arrow type, e.g. json and varchar.

Remaining in #1051:

• pandas: a duplicate column without its own dtype entry inherits the dtype of the first column with that name, e.g. a time column after an integer one, because read_csv(dtype=...) applies a name's entry to its renamed duplicates.
• Arrow on the S3 path with different Arrow types: pyarrow.csv column_types is keyed by name.

Not handled (also not on master):

• Polars columns= together with new_columns=: Polars' handling of a schema_overrides dict for that combination was inconsistent in local checks.
• Headerless .txt results with Polars: the default types are keyed by the description names, which never match Polars' generated column_N.

WHY

Fixes #1032. Measured on Athena before this change with SELECT 1 AS x, 2 AS x, 'a' AS y:

Cursor	S3 result file	managed
PandasCursor	[(1, 1, 'a')]	ValueError: All arrays must be of the same length
ArrowCursor	[(2, 'a')]	ArrowInvalid
PolarsCursor	[(1, 1, 'a')]	ShapeError

The design was compared with keying the pandas maps by column position. Position keys broke 5 existing tests:

• With custom quoting, the converters ran on quoted text.
• With usecols=, the python engine reads integer converter keys as positions among the selected columns, and the C engine as positions in the file.

Label keys were chosen after a consultation with Claude claude-fable-5-1 and Codex, both of which recommended them.

TEST

Tested commit: e783222 (rebased onto a57325b, after #1044).

• just lint: passed.
• Live Athena, including a managed query result storage work group: uv run --env-file .env pytest -n 4 tests/pyathena/pandas tests/pyathena/polars tests/pyathena/arrow tests/pyathena/test_result_set.py: 522 passed. This includes the AsyncCursor tests and unit tests of these packages.
• With Keep integer and JSON values exact in PandasCursor results with NULL #1044: the label-keyed converters use its _get_csv_converter(), and the fallback DataFrame keeps its integer and json-NULL dtypes per position. Same-named json columns with NULL and integer columns with NULL return the same rows and dtypes (object, Int64) from the S3 file and from managed storage (live).
• New tests:
  • Duplicate columns, S3 and managed: integer/varchar and time/integer duplicates for pandas and Polars; json/varchar duplicates for Arrow.
  • pandas names= / usecols= (by position and by label) with duplicates.
  • Polars new_columns= with duplicates, a partial new_columns (eager and chunked), and new_columns with user schema_overrides (live, and offline for .txt).
  • pandas engine selection with repeated names and with names=.
  • Offline: the later-page labels row and the Arrow fetch after close().
  • Each fails on the code before its fix. The partial new_columns case fails on f0a0ba2 for eager reads; chunked reads already passed there.
• Manual live checks covered the cases above with chunksize=1, binary NULL with usecols, pandas quoting=3 (values stay text, as on master), and the PandasCursor and ArrowCursor fail on columns with the same name and different types in the S3 result file #1051 query.
• Not run locally: the S3FS, aio, and SQLAlchemy suites. The AWS CI runs them when the PR is marked Ready.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one: implementation behavior: CLEAN

Scope: git diff 28ec68d9f70b5bc2fb447f6a4fb745ee0a467683..8e5dda66ad09576e7f19aa941c50651ab72e38ee, all 9 files.

Checked:

• Callers. The pandas, Arrow, and Polars result sets are shared by the sync, AsyncCursor, and aio cursors (pyathena/{pandas,arrow,polars}/async_cursor.py, pyathena/aio/{pandas,arrow,polars}/cursor.py). _fetch_all_rows() is also used by AthenaS3FSResultSet (pyathena/s3fs/result_set.py:125), which reads positionally and gets the first-page labels fix. Nothing else referenced _rows_to_columnar(). Only Polars passes column_names to _json_converters(); Arrow keeps the description names, where strict=True holds.
• Name resolution. _get_column_names() returns the description names unchanged unless a name repeats. In that case it uses each library's header parser, so a collision such as x, x, x.1 gets pandas' own x.2. Read paths keyed by these names: dtypes, converters, parse_dates, _time_columns, fetchone(), the .txt names= (pandas) / new_columns (Polars), the chunked readers, and the GetQueryResults fallback. The binary-NULL path (_configure_binary_csv_read) already resolves duplicates with the same parser, so its converter keys match. Measured live: duplicate json/time/date/varbinary/decimal columns convert correctly on the S3 and managed paths, and with chunksize=2.
• pandas engine="pyarrow". It does not rename duplicates (pandas 3.0.6: columns x, x, and a dtype keyed by x.1 raises AttributeError), so it falls back to c only when names repeat.
• Fetch/close. Arrow close() now leaves an exhausted iterator, so fetchone()/fetchmany()/fetchall() return None/[], as pandas (enumerate([])) and Polars (iter([])) do.
• Fallback pages. first_page is computed before the request, so only the request without a NextToken skips the labels row, matching _pre_fetch().
• Tests. Both offline tests fail on the merge-base source (checked by reverting pyathena/ only). The live duplicate tests include a non-duplicate column (y), so they hit the managed-path length mismatch, and they include json and time, which exercise converters and parse_dates.

Limitations, recorded rather than fixed:

• Duplicates with different types remain out of scope by agreement (to be filed separately).
• A Polars name collision such as x, x, x_duplicated_0 raises Polars' DuplicateError. The S3 path raised it before this PR (wrapped in OperationalError), and the managed path raised ShapeError before. Both still fail.
• No live test covers the .txt paths with duplicates; Athena writes .txt only for DDL-style output.
• Keep integer and JSON values exact in PandasCursor results with NULL #1044 also edits pandas' CSV converters, so whichever PR merges second needs a rebase.

[laughingman7743]

Rebase repair, round one (implementation): CLEAN. The branch was rebased from 8e5dda6 (merge-base 28ec68d) to e87840a (merge-base ec5323e) to resolve conflicts with #1033. Reviewed with git range-diff 28ec68d9f70b5bc2fb447f6a4fb745ee0a467683..8e5dda66ad09576e7f19aa941c50651ab72e38ee ec5323ea30e3fc2da1aca536d9cbdf8b51c7b3e2..e87840ac4fe02887f43981d8a4009a0797a14513; both old objects exist.

The resolution keeps both changes:

• _json_converters became Convert time zone values to aware times and empty TIME/JSON text to NULL #1033's _text_value_converters (types in _TEXT_VALUE_TYPES), still with the optional column_names.
• Polars passes the renamed names to it.
• pandas _time_columns uses Convert time zone values to aware times and empty TIME/JSON text to NULL #1033's d[1] == "time" over the renamed names.
• The Arrow _fetch auto-merge reads by position and calls _text_value_converters with the description names, which equal the table names.

Upstream contracts checked: #1033 adds no other name-keyed maps in the result set files. Its new pandas/Polars/Arrow converters are reached through the converters/dtypes properties, which are keyed by the renamed names.

Validation on e87840a: just lint; offline tests 5 passed; live -k "duplicate_column_names or fetch_all_rows or time" 18 passed. Live duplicate time with time zone/json columns return aware times and decoded JSON for all three cursors, on S3 and managed.

[laughingman7743]

CI repair (6d073ae), round one: CLEAN. AWS CI on e87840a failed only tests/pyathena/pandas/test_cursor.py::TestPandasCursor::test_get_csv_engine_explicit_specification (AttributeError: ... no attribute '_metadata'; 2798 passed). The test builds AthenaPandasResultSet.__new__ without __init__, and the new duplicate-name condition in _get_csv_engine() reads description. That was my local miss, because I selected tests with -k.

• Production callers are unaffected: _get_csv_engine() is called only from _read_csv(), after __init__ has set _metadata.
• The repair sets result_set._metadata = None, which gives the same description as a real result set without metadata.
• It adds a case where description repeats a name with the pyarrow engine requested and available, and asserts the c engine. Before this, the new branch had no test.
• The new case fails with pyathena/ reverted to the merge-base (assert 'pyarrow' == 'c') and passes on 6d073ae. Lint passed.

[laughingman7743]

Self-review round two: claims and operational behavior: CLEAN after PR-body corrections

Scope: git diff 28ec68d9f70b5bc2fb447f6a4fb745ee0a467683..8e5dda66ad09576e7f19aa941c50651ab72e38ee, plus the PR body, commit message, and changed comments and docstrings.

Claims checked:

• Before-change table and the pandas time failure. Measured live on 28ec68d with SELECT 1 AS x, 2 AS x, 'a' AS y and with the duplicate json/time/date/varbinary/decimal query. Matches the body.
• Reader behavior. pandas converters and parse_dates, and Polars schema_overrides, hit only the exact (renamed) name; pandas dtype and pyarrow column_types hit every column with the name. pandas' engine="pyarrow" keeps x, x and raises AttributeError for a dtype keyed by x.1. All measured locally with pandas 3.0.6, polars and pyarrow from uv.lock. pandas 3.x is the only supported major on master (Raise the pandas and pyarrow minimum versions to tested versions #852).
• "to_pydict() keeps one column per name" (Arrow comment). Shown by the pre-change Arrow S3 result [(2, 'a')].
• "Only the first page can start with the column labels" (_fetch_all_rows comment). Matches _pre_fetch(). The Result set defects: duplicate column names, header-like rows on fallback pages, and Arrow fetch after close #1032 1500-row managed query now returns 1500 rows for pandas, Arrow, and Polars (live).
• "also applies to S3FSCursor's fallback". Structural: AthenaS3FSResultSet calls _fetch_all_rows() (pyathena/s3fs/result_set.py:125). No S3FS-specific live run with a header-like row.
• Async/aio coverage claim. AthenaPandasResultSet, AthenaArrowResultSet, and AthenaPolarsResultSet are constructed by the async_cursor.py modules and pyathena/aio/*/cursor.py. Those suites ran only in CI, not locally.

Caller compatibility:

• _json_converters() gains an optional argument; existing calls are unchanged.
• The removed private _rows_to_columnar() existed since v3.28.0 (Fetching results does not work with Athena managed query result storage #664). A GitHub code search finds no external caller, only vendored copies and unrelated functions with the same name.
• Property keys change only for duplicate names, as stated.

AWS operator: no added API calls. Name resolution parses one header line locally, and only when names repeat. The fallback page count is unchanged.

Docs: docs/usage.md (managed storage, index-based type hints for duplicate names) and docs/aio.md:223 make no statement that this change invalidates.

Corrections made to the PR body:

• The offline-test line now says only the two new tests fail on the merge-base source.
• The tested-commit line notes the tests ran on b7f4ad3, which has the same tree.
• The unrun coverage now names the async and aio suites.

[laughingman7743]

Follow-up claim correction: a re-measurement with this head (SELECT 1 AS x, 'a1' AS x, CAST('12:34:56' AS TIME) AS x) showed that the different-type case already works on the managed path for all three cursors and on the S3 path for PolarsCursor. Only the pandas and Arrow S3 paths still fail, because pandas dtype and pyarrow column_types apply a name's entry to every column with that name. The PR body's out-of-scope note claimed all three CSV paths needed rework; it now states the measured scope and links the filed follow-up #1051. No code change.

[laughingman7743]

Rebase repair, round two (claims): CLEAN after PR-body update.

• The tested-commit line now names e87840a and says which checks ran there (lint, offline, the pytest live run, the time with time zone check) and which manual checks ran on 8e5dda6.
• The live pytest line now records 18 passed with the widened -k.
• No PR-body claim refers to _json_converters.
• The behavior-change note (property keys only for duplicate names) and the PandasCursor and ArrowCursor fail on columns with the same name and different types in the S3 result file #1051 scope note are unchanged and still hold after Convert time zone values to aware times and empty TIME/JSON text to NULL #1033: parse_dates no longer includes the time zone types, which does not affect them.

[laughingman7743]

CI repair (6d073ae), round two: CLEAN.

• The commit message matches the test-only change.
• The PR body's TEST section now records the e87840a CI result (test-sqla and test-sqla-async passed; one test failure and its cause) and the repair. Earlier claims are unchanged.
• The AWS suite runs again when the PR is marked Ready.

[laughingman7743]

Independent review (relayed): Codex CLI 0.160.0, model gpt-6-astra, reasoning effort high, codex exec -s read-only, session 01a102b5-1d67-73b0-aab0-058e416cd6eb

Scope: git diff 28ec68d9f70b5bc2fb447f6a4fb745ee0a467683..8e5dda66ad09576e7f19aa941c50651ab72e38ee. The snapshot was a detached worktree at the head, without .env. The prompt contained the diff and the three required behaviors from #1032, with no PR number, PR text, commit messages, or self-review findings. Constraints: no edits, builds, tests, git writes, GitHub, or network. Static review. Afterwards the snapshot was clean and at the reviewed head.

Reviewer coverage: all nine files; sync, AsyncCursor, and aio callers; chunk iterators; CSV/.txt and UNLOAD/Parquet readers; binary NULL, JSON, and time conversion; fetch/close; the S3FS fallback; tests. It read the pandas 3.0.6, Polars 1.44.2, and PyArrow 25.0.1 sources. Pagination and closed-Arrow fixes: no defect. Tests: they assert observable behavior and fail on the base.

Verdict: FINDINGS (2).

Finding 1 (P2, reported as introduced): with S3 CSV results, execute("SELECT 1 AS x, 2 AS x, 'text' AS y", engine="python", names=["a", "b", "x.1"]) applies the generated x.1: Int64 dtype to the user's third column and raises OperationalError. The reviewer said the base reads it successfully.

Author verification: rejected as not introduced. Measured live on the base (master abc99b0, whose pyathena/pandas, pyathena/polars, and pyathena/result_set.py equal 28ec68d): the same call already raises OperationalError: Unable to parse string "text" at position 0. The same happens without duplicates, execute("SELECT 1 AS x, 'text' AS y", engine="python", names=["y", "x"]), on both base and head. The type maps are keyed by result column names and do not follow a user's names= rename. That is pre-existing; this diff only adds the renamed keys of duplicate columns.

[laughingman7743]

Independent follow-up (relayed): Codex CLI 0.160.0, model gpt-6-astra, reasoning effort high, codex exec -s read-only, session 01a102c5-1fe2-7f00-8b46-b1c95d92edbd. Static review. Verdict: CLEAN.

Scope: the rebase over #1033. The reviewer had git range-diff 28ec68d9f70b5bc2fb447f6a4fb745ee0a467683..8e5dda66ad09576e7f19aa941c50651ab72e38ee ec5323ea30e3fc2da1aca536d9cbdf8b51c7b3e2..e87840ac4fe02887f43981d8a4009a0797a14513, a detached snapshot at e87840a, and a factual note on the upstream renames. It had no PR text and no prior findings, and the constraints were unchanged.

Reviewer coverage:

• pandas: renamed names stay aligned across CSV parsing, fallback construction, converters, parse_dates, and fetch. _time_columns covers only plain time, and the zone types keep Convert time zone values to aware times and empty TIME/JSON text to NULL #1033's conversion.
• Arrow: positional batches and fallback table. _text_value_converters applies to each matching position without reconverting API-derived plain times.
• Polars: renamed names stay aligned with dtypes, iterators, and the explicit column_names passed to _text_value_converters, for chunked CSV and the fallback.
• Pagination and closed Arrow: no defect.

The snapshot stayed clean at e87840a. The two earlier findings remain as verified above (finding 1 pre-existing; finding 2 is the pre-existing rename limitation, deferred).

[laughingman7743]

Independent follow-up on the CI repair (relayed): Codex CLI 0.160.0, model gpt-6-astra, reasoning effort high, codex exec -s read-only, session 01a10440-11ef-7e20-bd0a-bdece1a8603d. Static review. Verdict: CLEAN.

Scope: git diff e87840ac4fe02887f43981d8a4009a0797a14513..6d073ae139c731fa9bef8d6252c4b56f580861a5 (test-only), snapshot detached at 6d073ae, same constraints as before.

Reviewer coverage:

• The new case supplies repeated names while every other compatibility condition holds and pyarrow availability is mocked, so without the condition it returns pyarrow and fails.
• It traced the sync, threaded-async, and aio paths through construction, _as_pandas(), _read_csv(), and _get_csv_engine(). AthenaResultSet.__init__ sets _metadata before CSV reading, so _metadata = None repairs only the __new__ fixture and does not hide a production defect.
• The prior assertions are unchanged, and the property mocks are scoped.

The snapshot stayed clean at 6d073ae.

[laughingman7743]

Independent review (relayed, continued): Codex gpt-6-astra, session 01a102b5-1d67-73b0-aab0-058e416cd6eb

Finding 2 (P2, reported as introduced): with a non-chunked S3 CSV read, execute("SELECT 1 AS x, 2 AS x, 'text' AS y", new_columns=["y", "x_duplicated_0", "x"]) makes Polars 1.44.2 turn the schema overrides into positional types in new_columns order, so "text" is parsed as an integer and OperationalError is raised.

Author verification: confirmed behavior change, deferred as the same pre-existing limitation; no code change. Measured live:

• Base: that call returns [('text', 'text', 1)]. That is already wrong, because the duplicate x values were merged.
• Head: it raises could not parse "text" as dtype i32 at column 'y'.
• Without duplicates, execute("SELECT 1 AS x, 'text' AS y", new_columns=["y", "x"]) raises the same error on both base and head.

The cause is that name-keyed schema_overrides do not follow a user's new_columns rename. That is independent of this fix, which only adds keys for the renamed duplicate columns. This contrived case changes from silently wrong values to an error. Making the type maps follow user reader renames (pandas names=, Polars new_columns=) is a separate design question, left to the maintainer rather than folded into #1032.

[laughingman7743]

Self-review round one, redesign (implementation): CLEAN

Scope: full pass over git diff 72c95e321d13b93c3a263e65239ef22f5d41fb87..f0a0ba2861418c5a11ce5349c41b800d68a5e482, all 9 files. The patch was rebased onto 72c95e3, over #1054 (pandas time NULLs) and #1045 (Arrow CSV blocks); neither changes name keying.

Checked:

• pandas key equivalence. Without renaming options or duplicates, the labels are the description names, so the rebuilt dtype, converters, and parse_dates equal master's, minus unselected columns. Options given to execute() stay (not in self._kwargs, so explicit {} and None are kept too).
• Non-standard parsing. labels is None under custom quoting, dialect, skiprows, or header, under the pyarrow engine, for .txt, and when the labels are not one-to-one with the columns. In all of these the master keys and master time columns stay. The 5 test_binary_custom_* / test_binary_dataframe_column_names cases that the positional attempt broke pass.
• Binary NULLs. _can_preserve_binary_csv_nulls is master's predicate with the parsing part extracted into _is_standard_csv_parsing. The converters it wraps are the label-keyed ones, which match master's rebuilt dict for renamed or selected columns.
• Fetch. pandas reads rows by position in the DataFrame order. With usecols/index_col that returns the read columns, where master raised KeyError. Polars uses _get_frame_column_names() for eager reads, chunked reads (scan_csv), iter_chunks(), and fetchone(). The managed path uses _get_column_names() without new_columns, so CSV options stay out of the fallback naming. UNLOAD ignores new_columns.
• Public properties. pandas and Polars dtypes/converters/parse_dates return master's description-name keys.
• Tests. The pandas and Polars duplicate tests now mix types (integer/varchar, time/integer), so they exercise the per-label dtype and the time truncation. The read-option tests fail on master (measured: OperationalError, KeyError, wrong values).

Limitations, recorded:

• The pandas dtype inheritance for duplicates without their own entry, and Arrow on the S3 path, remain in PandasCursor and ArrowCursor fail on columns with the same name and different types in the S3 result file #1051.
• Polars columns= with new_columns= is not handled.
• .txt results with duplicate names convert only the first of them.
• Keep integer and JSON values exact in PandasCursor results with NULL #1044 overlaps in pandas' CSV options.

[laughingman7743]

Repair 4585ef7, round one (implementation): CLEAN. Reviewed git range-diff 72c95e32..f0a0ba28 72c95e32..4585ef74; one added commit.

• Polars. _csv_dtypes (header names, duplicates renamed) is the only reader schema. The user's new_columns is popped from the read kwargs and applied after reading to the eager DataFrame and to each scan_csv batch, with Polars' own _update_columns rule. .txt keeps applying the full frame names. Fetch and converters keep using _get_frame_column_names(), which equals the names after renaming. UNLOAD and the managed path are unaffected.
• Arrow. Converters are taken by description position: every column for result files, _TEXT_VALUE_TYPES only for GetQueryResults, as before. The description is aligned with the table for CSV, Parquet (metadata replaced from the schema), and the fallback. The fast path without conversion is kept.
• pandas engine. pyarrow is disallowed exactly when label resolution would be needed, so _get_csv_column_labels() returning None for pyarrow now only covers cases where the labels equal the description names.
• Validation. pytest -n 4 tests/pyathena/pandas tests/pyathena/polars tests/pyathena/arrow tests/pyathena/test_result_set.py: 511 passed. Reverting the source to f0a0ba2 fails the new Arrow tests ([default] and [managed]), the pandas engine test, and the eager partial new_columns test.

[laughingman7743]

Repair a21bb92, round one: CLEAN. In Polars, _get_csv_params() returns the user's new_columns for .csv only when execute() did not give schema_overrides. In that case the readers pop it and rename after reading. Otherwise both options reach Polars unchanged, as on master, and the result set's types are not used. _get_frame_column_names() still applies new_columns, which matches the DataFrame either way. .txt is unchanged. Tests: tests/pyathena/polars 113 passed, and the new test fails on 4585ef7.

[laughingman7743]

Repair f66d765, rounds one and two: CLEAN. Both Polars readers pop new_columns only when execute() gave no schema_overrides. .txt still sets the full frame names after reading, so the final names are the same either way. The docstring and the commit message match. Tests on f66d765: tests/pyathena/polars 115 passed. The new offline test fails on a21bb92.

[laughingman7743]

Rebase over #1044 (e783222), rounds one and two: CLEAN. Reviewed git range-diff 72c95e32..f66d765a a57325b1..e7832221; both old objects exist. Conflicts were only in pandas:

• GetQueryResults fallback. Keep integer and JSON values exact in PandasCursor results with NULL #1044's integer and json-NULL dtypes are now applied per position, with _get_column_names() names, instead of per description name.
• Binary conversion. _configure_binary_csv_read(read_csv_kwargs, labels) is kept. Keep integer and JSON values exact in PandasCursor results with NULL #1044's self._csv_converters = read_csv_kwargs.get("converters") follows it, so _finish_csv_frame() restores json NULLs under the same labels that the converters are keyed by.
• Converters. _key_csv_columns_by_labels() now builds them with Keep integer and JSON values exact in PandasCursor results with NULL #1044's _get_csv_converter() (_JSONConverter for json), matching _get_csv_read_options().

Validation on e783222:

• pytest -n 4 tests/pyathena/pandas tests/pyathena/polars tests/pyathena/arrow tests/pyathena/test_result_set.py: 522 passed, including Keep integer and JSON values exact in PandasCursor results with NULL #1044's tests.
• Live: same-named json and integer columns with NULL give identical rows and dtypes (object, Int64) on the S3 and managed paths.

PR body updated with the tested commit and the #1044 interaction.

[laughingman7743]

Self-review round two, redesign (claims): CLEAN

Claims in the rewritten PR body and the f0a0ba2 commit message, checked against evidence:

• "Each failed on master". Measured on master:
  • pandas names=["y","x"]: OperationalError.
  • usecols=[1,2] on duplicates: KeyError.
  • usecols=["x","t"]: Missing column provided to 'parse_dates': 'd', on 72c95e3.
  • Polars new_columns=["y","x"]: OperationalError.
  • The duplicate type-mismatch queries fail for pandas and Polars.
• Remaining PandasCursor and ArrowCursor fail on columns with the same name and different types in the S3 result file #1051 scope. Measured on f0a0ba2: pandas SELECT 1 AS x, 'a1' AS x, CAST(... AS TIME) AS x raises Unable to parse string "12:34:56.000", because the time column inherits x's Int64. Arrow S3 raises time data '1' does not match. The claim of a pandas fix for integer/varchar duplicates is backed by the new live test.
• "Custom parsing keeps master's keys". quoting=3 returns '"00 ff"' as on master (live), and the pinned tests pass.
• Polars columns= with new_columns=. Stated as not handled; a local check showed Polars applying a dict inconsistently.
• Design comparison. Both failure modes of the position keys were reproduced: the 5 test failures, and the python-engine IndexError. The consultation outputs are kept locally.
• Tested commit and suites. 251 passed and 4 passed ran on f0a0ba2. Arrow, S3FS, async, and aio suites are left to CI and stated as such.

No AWS API calls are added. The header is parsed only when names repeat or renaming or selecting options are given, and the fallback page count is unchanged.

[laughingman7743]

Repair 4585ef7, round two (claims): CLEAN after PR-body update.

The PR body now states:

• The pyarrow engine is avoided when labels need resolving.
• Polars renames after reading, and why: read_csv and scan_csv interpret a dict differently, as measured locally and read in the Polars 1.44.2 source.
• Arrow converts by position.
• Arrow on the S3 path handles different types only when they share an Arrow type. PandasCursor and ArrowCursor fail on columns with the same name and different types in the S3 result file #1051 keeps only Arrow S3 with different Arrow types.

The TEST section names 4585ef7 and the 511-passed run, which includes the async tests and the Polars unit tests omitted before. The commit message matches the change. The #1051 scope comment said Arrow fetch converters were still keyed by name; that is no longer true and will be corrected there.

[laughingman7743]

Repair a21bb92, round two: CLEAN. The commit message and the _get_csv_params() docstring state the schema_overrides exception. The PR body will list the new test and the .txt + new_columns limitation (pre-existing on master).

[laughingman7743]

Independent review (relayed): Codex CLI 0.160.0, model gpt-6-astra, reasoning effort high, codex exec -s read-only, session 01a1049e-7f59-7702-8c1f-f382b9b50975. Static review of git diff 72c95e321d13b93c3a263e65239ef22f5d41fb87..f0a0ba2861418c5a11ce5349c41b800d68a5e482.

The snapshot was a detached worktree at f0a0ba2, without .env. The prompt contained the diff, the required behaviors, and the accepted out-of-scope items, with no PR text or prior findings. Constraints: no edits, builds, tests, git writes, or network. Afterwards the snapshot was clean at f0a0ba2.

Reviewer coverage: all nine files; sync, AsyncCursor, and aio callers; S3 CSV and managed fallbacks; chunking, UNLOAD/Parquet, .txt, binary NULLs, JSON/time, reader overrides, empty results, pagination, close, and public property keys. It read the pandas 3.0.6, Polars 1.44.2, and PyArrow 25.0.1 sources. Verdict: FINDINGS (4).

Finding 1 (P2, introduced): a partial Polars new_columns loses column types. With SELECT '001' AS x, 2 AS y and new_columns=["z"], the overrides {"z": String, "y": Int32} let the eager reader infer x as an integer, so the value becomes 1.

Verified: confirmed. Local check with Polars 1.44.2: read_csv(new_columns=["z"], schema_overrides={"z": String, "y": Int32}) returns (1, 2); scan_csv returns ('001', 2) with the same overrides, but (1, 2) with {x, y}. Polars' read_csv converts a dict into a positional list only when its keys are a prefix of new_columns; otherwise it reads the keys as header names. scan_csv reads them as the new names.

Repair (4585ef7): key schema_overrides by the CSV header (_csv_dtypes) and apply the user's new_columns after reading, as Polars' own _update_columns does: [*new_columns, *df.columns[len(new_columns):]]. Added test_new_columns_renaming_first_columns[None|1]; the eager case fails on f0a0ba2.

[laughingman7743]

Finding 2 (P2, reported as pre-existing): Arrow fetch converters are keyed by name. With managed storage, SELECT json_parse('[1]') AS x, 'text' AS x returns ('[1]', 'text'), and reversing the columns decodes "text" as JSON.

Verified: confirmed. It also occurs on the S3 path whenever same-named columns share an Arrow type: the new Arrow test with json/varchar duplicates fails on f0a0ba2 for both [default] and [managed]. This is within #1032's same-named values.

Repair (4585ef7): _fetch picks each converter by the description position. For managed results, only _TEXT_VALUE_TYPES are converted, as before. Arrow on the S3 path with different Arrow types still fails at read time (pyarrow.csv column_types by name), which stays in #1051.

[laughingman7743]

Finding 3 (P2, pre-existing): the pandas pyarrow engine bypasses label alignment. With SELECT 1 AS a, 'c' AS b, repeat('x', 200) AS padding, engine="pyarrow", and names=["b", "a", "padding"], the engine qualifies, and pandas applies a: Int64 after renaming, so "c" fails to convert.

Verified: confirmed from source. _get_csv_column_labels() returns None for the pyarrow engine, so the master keys stay, and the PR body's claim that types follow names= did not hold for that engine.

Repair (4585ef7): _get_csv_engine() no longer chooses pyarrow when _needs_csv_column_name_resolution() is true (repeated names, names, usecols, separator options). This is the existing fallback to the C engine for incompatible options. test_get_csv_engine_explicit_specification gains a names= case, which fails on f0a0ba2.

[laughingman7743]

(Anchored here because the readers' schema_overrides change caused it; the failing tests are in tests/pyathena/polars/test_result_set.py:32.)

Finding 4 (P2, introduced): existing Polars unit tests break. tests/pyathena/polars/test_result_set.py builds result sets with __new__ and patches dtypes, but the readers no longer read it.

Verified: confirmed. 5 tests failed locally. My earlier local runs had omitted that file.

Repair (4585ef7): the readers take their types from a private _csv_dtypes property, and the tests patch it instead of dtypes. The file passes (included in the 511 passed).

[laughingman7743]

Independent follow-up (relayed): Codex CLI 0.160.0, gpt-6-astra, effort high, codex exec -s read-only, session 01a104b5-f648-70c2-9337-acdd87d29090 (repair review of git diff f0a0ba28..4585ef74). Static review; the snapshot stayed clean at 4585ef7.

It confirmed the four earlier repairs: the Polars partial rename, the Arrow positional converters (exercised on S3 and managed), the pandas engine fallback (tested directly), and the Polars test patches. Verdict: FINDINGS (2), both in Polars new_columns handling.

Finding A (P2, introduced by 4585ef7): user schema_overrides keyed by the new names stop applying. With execute("SELECT '001' AS x", new_columns=["z"], schema_overrides={"z": pl.String}), popping new_columns leaves the key z unmatched, so "001" becomes 1.

Verified: confirmed. The new test test_new_columns_with_schema_overrides fails on 4585ef7 with [(1, 2)] == [('001', 2)].

Repair (a21bb92): when execute() gives schema_overrides, which replace the result set's types, new_columns is left to Polars as on master. The result set renames only when its own types are in use. tests/pyathena/polars: 113 passed.

Finding B (P2, reported as introduced for .txt): for a headerless .txt result with new_columns, the default types target the header names, not Polars' generated column_N.

Verified: pre-existing relative to master; deferred. On master, .txt defaults are keyed by description names, which never match column_N, and the user's new_columns was overwritten by df.columns = description names. Only the intermediate f0a0ba2 happened to translate them. Athena writes .txt only for DDL-style output. No change in this PR; it is recorded as a limitation.

[laughingman7743]

Independent follow-ups (relayed): Codex CLI 0.160.0, gpt-6-astra, effort high, codex exec -s read-only. Static reviews; the snapshots stayed clean.

1. Repair review of git diff 4585ef74..a21bb92d (session 01a104c2-6397-7300-916c-0bebf04116d7). FINDINGS (1): for a headerless .txt result, new_columns=["z"] with schema_overrides={"z": pl.String} still had new_columns popped, so the override could not match column_1 and "001" became 1. On master, both reached Polars.
   • Verified: confirmed by a new offline test, test_txt_new_columns_with_schema_overrides[_read_csv|_iter_csv_chunks], which fails on a21bb92 with 'z': [1].
   • Repair (f66d765): the readers pop new_columns only when execute() does not give schema_overrides, for .csv and .txt alike.
2. Repair review of git diff a21bb92d..f66d765a (session 01a104c9-6a0a-76c1-b703-c240e85ace61). CLEAN: the repair covers eager and chunked reads, with no defect or regression against the merge-base in fetch names or converters.

Validation on f66d765: tests/pyathena/polars 115 passed. The source changes since the 511-passed run on 4585ef7 touch only pyathena/polars/result_set.py.

[laughingman7743]

Independent follow-up on the rebase over #1044 (relayed): Codex CLI 0.160.0, gpt-6-astra, effort high, codex exec -s read-only, session 01a104d9-1fce-7ac2-9058-68a8d98cf2a0. Static review. Verdict: CLEAN.

Scope: git range-diff 72c95e32..f66d765a a57325b1..e7832221, with a detached snapshot at e783222 and a factual note on #1044's changes. The reviewer found no regression in:

• json NULL restoration by label;
• binary converter wrapping alongside _JSONConverter;
• the per-position integer and json-NULL dtypes in the fallback;
• chunked reads.

The snapshot stayed clean.