Keep NULL rows of single-column CSV results in ArrowCursor - #1031
Conversation
Athena writes a single-column row with a NULL value as an empty line, and the pyarrow CSV reader skipped empty lines unless the result had a varbinary column. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| quote_char='"', | ||
| ignore_empty_lines=not binary_columns, | ||
| # Athena writes a single-column row with a NULL value as an empty line. | ||
| ignore_empty_lines=False, |
There was a problem hiding this comment.
Self-review round one (implementation behavior)
Base de8cc52ac2a43cdba72bb4571c185883287741fc, head 6b924fe7161e6d0e4b61bc601beeadb78f075793.
Covered: AthenaArrowResultSet._read_csv() CSV branch and its only caller (__init__ → as_arrow()/fetch*()), shared by ArrowCursor, AsyncArrowCursor, and AioArrowCursor; the .txt branch is unchanged; pandas and Polars have their own _read_csv().
Result: CLEAN.
- Binary results already used
ignore_empty_lines=False, so their path is unchanged. - Multi-column rows always contain a delimiter, so they never form empty lines; a trailing newline does not add a row (checked locally with pyarrow).
- Quoted values containing blank lines (
"a\n\nb") stay one value with either setting (checked locally). - Types without an Arrow dtype in
DefaultArrowTypeConverter(e.g.uuid,timestamp with time zone,unknown) are inferred; 5,000 leading empty lines followed by a value still inferstringacross 1 KiB blocks, so NULLS FIRST results do not hit a null-type conversion error. - NULL in string-typed columns becomes
'', the documented CSV behavior also seen in multi-column results; the test pins it. - The new test fails on the reverted source (both cases) and passes with the change.
| delimiter=",", | ||
| quote_char='"', | ||
| ignore_empty_lines=not binary_columns, | ||
| # Athena writes a single-column row with a NULL value as an empty line. |
There was a problem hiding this comment.
Self-review round two (claims, callers, operations)
Base de8cc52ac2a43cdba72bb4571c185883287741fc, head 6b924fe7161e6d0e4b61bc601beeadb78f075793.
Result: FINDINGS (PR description only; no code change).
Claims checked:
- Affected since v2.7.0 —
git show v2.7.0:pyathena/arrow/result_set.pybuilds the CSVParseOptionswithoutignore_empty_lines(pyarrow defaultTrue). Holds. - Except for
varbinarycolumns — incorrect as written:ignore_empty_linesis absent in v3.34.0–v3.36.0 and present from v3.37.0 (and on3.x). The release note now says v3.37.0 kept the rows only forvarbinarycolumns and that3.xhas the same code. - Async/aio share the result set —
pyathena/arrow/async_cursor.py:166andpyathena/aio/arrow/cursor.py:177constructAthenaArrowResultSet. Holds; their tests ran in the 106-test local run, but the new regression test is sync only. - NULL string →
''— matchesdocs/null_handling.md(ArrowCursor CSV row); no documentation becomes obsolete. - Evidence revision — the full Arrow run used an earlier test draft on the same source; the TEST section now says so and names the run on 6b924fe.
Callers/operators: no API, default, or exception change; no extra AWS requests (one CSV read as before). Callers that relied on dropped rows now see the NULL rows, which is the fix.
| quote_char='"', | ||
| ignore_empty_lines=not binary_columns, | ||
| # Athena writes a single-column row with a NULL value as an empty line. | ||
| ignore_empty_lines=False, |
There was a problem hiding this comment.
Independent review (relayed result; static review)
Reviewer: Codex CLI 0.160.0, model gpt-6-astra, reasoning effort max, --sandbox read-only, session 01a10222-d559-7372-bf39-d49a4c0fc7a7. Prompt contained only the literal diff, repository context, and review instructions (no PR text, commit message, or prior findings); no edits, tests, network, or GitHub access.
Base de8cc52ac2a43cdba72bb4571c185883287741fc, head 6b924fe7161e6d0e4b61bc601beeadb78f075793, detached snapshot unchanged after the review.
Covered: sync, threaded async, and aio Arrow callers; table/fetch conversion; single/multi-column and binary results; explicit/inferred dtypes; all-NULL and header-only CSV; CRLF, quoted newlines, block_size; documented NULL behavior; the test (fails without the change: 2 vs 3 and 0 vs 1 rows).
Result: FINDINGS (2, both pre-existing; no regression in the changed line's own behavior).
- NULL TIME/JSON values raise when fetched.
strings_can_be_null=Falseturns a NULL in a string-typed column into'', and_to_time('')(pyathena/converter.py:85) /_to_json('')(:121) raise. Verified: live on this head,SELECT 1 AS id, CAST(NULL AS TIME) AS traisesValueError: time data '' does not match format '%H:%M:%S.%f'inArrowCursor(the multi-column path is unchanged by this PR), andSELECT CAST(NULL AS TIME) AS tnow raises the same instead of returning no rows. JSON:_csv_to_jsonin Return Athena JSON values from SQLAlchemy JSON columns and the pandas, Arrow, and S3FS cursors #1010 already maps''toNone. Deferred as a separate pre-existing TIME defect (it also touches_to_time, which TIME(0) and TIME(p) with p > 6 values fail to convert #1030 rewrites); not folded here. - Quoted values with newlines crossing a CSV block boundary fail because
newlines_in_valuesstaysFalse(pyathena/arrow/result_set.py:308). Verified offline with pyarrow: 1,200-byte multi-line values withblock_size=1024raiseCSV parser got out of sync with chunker; they parse with the default 1 MiB blocks unless a value straddles a boundary. Unchanged from the merge-base. Deferred as a separate issue.
There was a problem hiding this comment.
Correction to finding 2: AthenaArrowResultSet.DEFAULT_BLOCK_SIZE is 128 MiB (pyathena/arrow/result_set.py:66), not 1 MiB. Multi-line values fail only when one straddles a block boundary, which needs a CSV result over 128 MiB or a smaller block_size; the 1 MiB figure in the record was the size used in the offline check.
There was a problem hiding this comment.
There was a problem hiding this comment.
Disposition update for finding 2: filed as #1040, reproduced on Athena with execute(sql, block_size=1024).
WHAT
ArrowCursorkeeps the rows of a CSV result whose only column is SQL NULL.AthenaArrowResultSet._read_csv()now always passesignore_empty_lines=Falseto the pyarrow CSV reader; it previously did so only when the result had avarbinarycolumn.INTEGER) is returned asNone, as for multi-column results.varcharcolumn is returned as'', the documentedArrowCursorCSV behavior for NULL strings (docs/null_handling.md), as for multi-column results.AsyncArrowCursorandAioArrowCursorshare the result set.Release note (fix):
ArrowCursorno longer drops NULL rows of single-column CSV results fromfetch*()andas_arrow(). Affected sinceArrowCursorwas added in v2.7.0; v3.37.0 kept them only forvarbinarycolumns. The3.xbranch has the same code.WHY
Closes #1026.
Athena writes a single-column row with a NULL value as an empty line, and the pyarrow CSV reader skips empty lines by default.
TEST
Tested commit: 6b924fe
just lint: passed.TestArrowCursor::test_single_column_null(VALUES 1, NULL, 2andCAST(NULL AS VARCHAR)) checksas_arrow().num_rowsandfetchall(). With the source change reverted, both cases fail; with it, both pass.uv run --env-file .env pytest -n 4 tests/pyathena/arrow tests/pyathena/aio/arrowon the same source with an earlier test draft: 106 passed, 1 failed (the newvarcharcase, which expectedNone). The expectation was corrected to''per the documented behavior above; on 6b924fe,-k "single_column_null or binary"intests/pyathena/arrow/test_cursor.py: 4 passed.just test pyathenasuite; it runs in CI when the PR is marked Ready.🤖 Generated with Claude Code