Read columns with the same name by their own types from CSV result files - #1066
Conversation
pyarrow applies a column_types entry to every column with its name, so the Arrow cursor reads such columns under their positions and renames them after reading; the binary columns are also matched by position. pandas copies the dtype of a column to the columns it renames, such as x.1, so the pandas cursor skips the header row and reads the columns under their labels when it builds the dtypes. Closes #1051 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ames Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| } | ||
| else: | ||
| column_names = names | ||
| column_types = self.column_types |
There was a problem hiding this comment.
Self-review round one (behavior and implementation) — base 8c9c201ac53a06c2f54d7f282ec4de23b0fe449f, head 6af16a0dfc8c6810e634d8c4b1390435c101c354. Result: FINDINGS (1, repaired in 936a692).
Finding: for results with unique column names, _read_csv() built its own positional column_types dict instead of reading the public column_types property, so a subclass overriding that property would no longer affect reading, a change outside #1051.
Repair: unique names use self.column_types and the master read options exactly; only repeated names take the positional path. Rerun: pytest -n 4 tests/pyathena/arrow tests/pyathena/aio/arrow → 119 passed.
Covered without findings:
- Positional
column_names/column_typesandrename_columns(names);zip(strict=True)lengths always match by construction. skip_rows_after_names=1skips a parsed row (verified locally on pyarrow 25.0.1 with a quoted newline in the header and small block sizes). A header-only file gives an empty table with the renamed columns..txtresults with repeated names, varbinary NULL handling by position, and async (AsyncArrowCursorshares the result set).
| ] | ||
| self._time_columns = [label for label, d in columns if d[1] == "time"] | ||
|
|
||
| def _read_csv_header_as_labels(self, read_csv_kwargs: dict[str, Any], csv_engine: str) -> None: |
There was a problem hiding this comment.
Self-review round one (behavior and implementation) — base 8c9c201ac53a06c2f54d7f282ec4de23b0fe449f, head 6af16a0d…. pandas side: CLEAN.
Covered:
- Inheritance is real in both engines (pandas 3.0.6): the C engine copies the dtype even with
header=0andnames; the python engine copies it to non-date columns, which is why the newline test adds anintervalcolumn. No dtype value stands in for "no dtype" (objectchanges inference and date dtypes;Nonediffers between engines). - Skipping the header: the C engine skips parsed rows, and the python engine skips physical lines. pandas opens text with
newline="", so universal-newline counting matches for\n,\r,\r\n, and consecutive newlines (verified locally, also withchunksize). - Guards: runs only when
labelsis not None (standard parsing, not the pyarrow engine, labels matching the columns) and neitherdtypenornameswas given toexecute(). Userusecols,index_col, andchunksizework with the replaced header (checked on Athena).engineis popped from kwargs by the cursor, socsv_engineis the engine used. - Ordering: called after
_configure_binary_csv_read(), whose_is_standard_csv_parsing()check requiresheader=0.BinaryCSVReaderpasses the header record through unchanged, andtest_binary_dataframe_column_names[duplicate_names-c|python]passes. - Tests: the new and extended tests fail on master (checked by reverting the source diff).
| ] | ||
| self._time_columns = [label for label, d in columns if d[1] == "time"] | ||
|
|
||
| def _read_csv_header_as_labels(self, read_csv_kwargs: dict[str, Any], csv_engine: str) -> None: |
There was a problem hiding this comment.
Self-review round two (claims, callers, operations, evidence): base 8c9c201ac53a06c2f54d7f282ec4de23b0fe449f, head 936a6924722f375c75891302b6d87640ae6bf5be. Result: FINDINGS (PR description only, corrected). No code changes.
Claims checked:
- "pandas copies the dtype ... in both engines, even when
namesis given withheader=0": measured locally on pandas 3.0.6 (C: time after Int64; python: interval after Int64 failed withheader=0+names). The comment and docstring here state the same thing. - "
skip_rowscounts physical lines" (Arrow) and "the python engine skips lines": measured locally with a quoted\nheader (pyarrow 25.0.1, pandas 3.0.6). - Athena allows newlines in column names and writes them in the header: measured on Athena (
"a\nb","a\nb"). - "Results with unique names are read as before": true after the round-one repair (Arrow uses
self.column_typesand the master options; pandas returns early). - "PolarsCursor ... already correct": was based only on the issue's table. Now measured on Athena with the issue's query and the newline-named int/time/interval columns, with and without
chunksize=1. - Varbinary positional claim:
test_binary_null_vs_emptyfails on master and passes here.
Callers and operations: there are no API signature or default changes. Public column_types/dtypes/converters keep their keys. With PyAthena-built dtypes, renamed columns lose only an inherited dtype that was never theirs, and pandas no longer emits a "Both a converter and dtype" ParserWarning for those columns. There are no new S3 or Athena requests; pandas adds one in-memory header parse only when names repeat.
Corrections: the TEST section claimed 6af16a0 for a run that preceded a comment-only edit, and did not cover the repair head. It now lists the run per revision (473 on the pre-comment tree; 119 Arrow sync and async tests on 936a692). "Measured with this branch" became a dated Athena measurement, and the Polars evidence was added.
Deferred: .txt results and custom quoting/dialects keep the existing keys, the same scope as #1050.
…for other delimiters Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| return | ||
| # pandas renames the names in a header row, and copies their dtypes, even with | ||
| # names given, so the header row is skipped instead. | ||
| read_csv_kwargs["names"] = self._resolve_csv_column_names( |
There was a problem hiding this comment.
Independent review (relayed): Codex CLI 0.160.0, model gpt-6-astra, reasoning effort max, -s read-only, session 01a10527-6805-7ce3-a2dd-fe2a929ab285. Base 8c9c201ac53a06c2f54d7f282ec4de23b0fe449f, head 936a6924722f375c75891302b6d87640ae6bf5be, run in a clean detached snapshot that was still unchanged afterwards. The prompt contained the literal diff and the intended behavior, and no PR text or prior findings. This was a static review: no tests, builds, or network.
Coverage: pandas header replacement for the C and python engines, multiline names, chunking, usecols/index_col/other overrides, BinaryCSVReader, storage_options, empty and header-only files; the Arrow positional path, .txt, binary NULL handling, and the unique-name path; tests and conventions. Result: FINDINGS (2). It found no Arrow or unique-name-path defect.
Finding 1 (P2), this line in 936a692: the replacement names came from _get_column_names() with default options, which ignore caller options such as escapechar. With engine="python", escapechar="\\" and SELECT CAST('01:02:03' AS TIME) AS "a\b", 1 AS "a\b", the dtype and date keys use the labels ab/ab.1, but the names were a\b/a\b.1, giving Missing column provided to 'parse_dates': 'ab'.
Verified: reproduced offline with pandas 3.0.6. Repaired in e1567d1: the names are resolved with _resolve_csv_column_names(names, read_csv_kwargs, pd.read_csv), the same resolution _get_csv_column_labels() uses for the keys.
| self._kwargs.keys() & {"dtype", "names"} | ||
| or len(set(names)) == len(names) | ||
| # pandas detects another delimiter, as with sep=None, from the header row. | ||
| or (read_csv_kwargs.get("delimiter") or read_csv_kwargs.get("sep")) != "," |
There was a problem hiding this comment.
Finding 2 (P2, relayed from the same Codex review): skipping the header removes pandas' delimiter-detection sample. With engine="python", sep=None and the header-only result of SELECT 1 AS x, 2 AS x WHERE false, the read raised Could not determine delimiter; master returns an empty DataFrame.
Verified: reproduced offline. Repaired in e1567d1: the header is replaced only when the delimiter is ,, so with sep=None or another delimiter pandas reads the header row as before.
Repair self-review (both perspectives), 936a6924..e1567d15, pandas only:
- Round one: the names now match the keys by construction (same resolver and options).
usecolsstill gives the full list ([0]). The resolver seesnames=Noneand the engine actually used. Both new test cases (test_duplicate_column_names_header_options[escapechar|detected_delimiter]) fail on 936a692 and pass on e1567d1. - Round two: the PR description now states the comma-delimiter condition and the option-aware labels, and lists the runs per revision.
pytest -n 4 tests/pyathena/pandas tests/pyathena/aio/pandas: 355 passed, 1 failed (test_result_set_file_system[pandas_cursor2-None], an UNLOAD/Parquet case outside the changed CSV path). That failure's output was not kept; a-n 1rerun passed, and its cause is unexplained.
…as reads it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| _PYARROW_READ_CSV_OPTIONS: ClassVar[frozenset[str]] = frozenset({"dtype", "parse_dates"}) | ||
| # The pandas.read_csv() options given to execute() that do not change how pandas reads | ||
| # the header row, with which _read_csv_header_as_labels() replaces the header. | ||
| _LABELED_HEADER_READ_CSV_OPTIONS: ClassVar[frozenset[str]] = frozenset( |
There was a problem hiding this comment.
Independent follow-up review (relayed): Codex CLI 0.160.0, model gpt-6-astra, reasoning effort max, -s read-only.
Round 1, session 01a10539-de2c-7ee3-8038-4e4c63e5e6b8, reviewed 936a6924..e1567d15 on a clean snapshot at e1567d1. Result: FINDINGS (3):
- With
sep=Noneand a populated result, the header was kept, so the dtype copy still failed. This is the same behavior as master. commentchanged the header labels pandas reads (e.g.index_col="z"for"z#suffix"), but the replaced names did not.encodingchanged the decoded header labels, but the replaced names did not.
Findings 2 and 3 were regressions from master for those options; I verified both offline. Repaired in af023a2: the header is replaced only when every option given to execute() is in the allow-list _LABELED_HEADER_READ_CSV_OPTIONS, which lists options that do not change how pandas reads the header row. Any other option (dtype, names, sep, comment, encoding, escapechar, ...) keeps master's header reading, including master's limitation for same-named columns with different types. This narrows the change rather than extending it.
Repair self-review (both perspectives):
- Offline:
comment+index_col,usecols+index_col, userconverters, and no options all read as expected. - On Athena:
pytest -n 4 tests/pyathena/pandas/test_cursor.py tests/pyathena/aio/pandas -k "duplicate_column or binary or header_options or read_options"gave 59 passed. - The PR description now states the allow-list condition and the master fallback.
Round 2, session 01a10547-5690-77e0-8675-ca22c9c3b059, reviewed e1567d15..af023a29 on a clean snapshot at af023a2. Covered: all 20 allow-listed options against names + header=None + skiprows for the C and python engines, empty and multiline-header results, guard order, and the fallback for excluded options. Result: CLEAN. This was a static review against pandas 3.0.6 Python sources; the compiled C tokenizer source was unavailable. No tests were run.
1 participant
WHAT
Columns with the same name and different types now convert by their own types when
PandasCursorandArrowCursorread the S3 CSV result file, as they do through GetQueryResults._read_csv()reads the columns under their positions ("0","1", ...), keyscolumn_typesby those positions, and renames the columns back after reading. The header row is skipped withskip_rows_after_names=1, which skips a parsed row;skip_rowscounts physical lines and splits a quoted name that contains a newline. The varbinary NULL handling now matches binary columns by position, so a varchar column named like a varbinary column keeps its""for NULL. Results with unique names are read as before.x→x.1,x.2) that have no dtype of their own, in both the C and python engines, even whennamesis given withheader=0. When the names repeat and everyread_csvoption given toexecute()is one that does not change how pandas reads the header row (_LABELED_HEADER_READ_CSV_OPTIONS: e.g.usecols,index_col,converters,parse_dates,na_values,nrows),_read_csv_header_as_labels()passes the labels asnameswithheader=Noneand skips the header row: one parsed row for the C engine, and the physical lines of the header for the python engine, which skips lines.column_typesanddtypesproperties keep their name keys.WHY
Closes #1051.
SELECT 1 AS x, 'a1' AS x, CAST('12:34:56' AS TIME) AS xfailed on the S3 result file path:PandasCursor:OperationalError: Unable to parse string "12:34:56.000". TheInt64dtype of the firstxwas applied tox.2.ArrowCursor:ValueError: time data '1' does not match format '%H:%M:%S.%f'. Thecolumn_typesentry of the lastxwas applied to all three.Measured on Athena on 2026-10-04: Athena accepts a newline in a column name and writes it in the CSV header (
"a\nb","a\nb"), so the header can span several lines.Not changed:
read_csvoption given toexecute()(e.g.dtype,names,sep,comment,encoding,escapechar), the header row is read as on master, so columns with the same name and different types can still fail there, as before..txtresults and custom quoting or dialects keep the existing keys, as in Keep columns with the same name, and key CSV column types by the reader's column labels #1050.PolarsCursorand the managed (GetQueryResults) path were already correct. Checked on Athena forPolarsCursorwith the issue's query and the newline-named columns, with and withoutchunksize=1; the managed path through the extendedtest_duplicate_column_names[managed]tests.TEST
just lint: passed on af023a2.uv run --env-file .env pytest -n 4 tests/pyathena/pandas tests/pyathena/arrow tests/pyathena/aio/pandas tests/pyathena/aio/arrow: 473 passed on 6af16a0's code before a comment-only edit in that commit.pyathena/arrow/result_set.py:pytest -n 4 tests/pyathena/arrow tests/pyathena/aio/arrow119 passed.pytest -n 4 tests/pyathena/pandas tests/pyathena/aio/pandas355 passed, 1 failed:test_result_set_file_system[pandas_cursor2-None], the UNLOAD (Parquet) case, which does not read through the changed CSV code. Its error output was not kept; it passed on a-n 1rerun (3 passed), so its cause is unexplained.TestPandasCursor::test_duplicate_column_names_header_options[escapechar|detected_delimiter]fail on 936a692 and pass on e1567d1.pytest -n 4 tests/pyathena/pandas/test_cursor.py tests/pyathena/aio/pandas -k "duplicate_column or binary or header_options or read_options"59 passed. The full pandas suites were not rerun on af023a2; the change only narrows when the header is replaced.TestPandasCursor::test_duplicate_column_names[default](the issue's int/varchar/time columns, C engine).TestPandasCursor::test_duplicate_column_names_with_newline[c|python](int/time/interval columns nameda\nb).TestArrowCursor::test_duplicate_column_names[default],test_duplicate_column_names_with_newline.TestArrowCursor::test_binary_null_vs_empty(a varchar column named like the varbinary column).PandasCursoroptions{},engine="python",chunksize=1(both engines),usecols=["x.2", "x"],dtype={"x.1": str}, andindex_col="x.1"all read the expected values.🤖 Generated with Claude Code