Skip to content

Keep NULL rows of single-column CSV results in ArrowCursor - #1031

Merged
laughingman7743 merged 1 commit into
masterfrom
fix/1026-arrow-csv-null-rows
Oct 3, 2026
Merged

laughingman7743 merged 1 commit into
masterfrom
fix/1026-arrow-csv-null-rows

Conversation

@laughingman7743

@laughingman7743 laughingman7743 commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

WHAT

ArrowCursor keeps the rows of a CSV result whose only column is SQL NULL.
AthenaArrowResultSet._read_csv() now always passes ignore_empty_lines=False to the pyarrow CSV reader; it previously did so only when the result had a varbinary column.

  • A NULL in a non-string column (e.g. INTEGER) is returned as None, as for multi-column results.
  • A NULL in a varchar column is returned as '', the documented ArrowCursor CSV behavior for NULL strings (docs/null_handling.md), as for multi-column results.
  • Multi-column results never contain empty lines, so they are unchanged; a trailing newline does not add a row.

AsyncArrowCursor and AioArrowCursor share the result set.

Release note (fix): ArrowCursor no longer drops NULL rows of single-column CSV results from fetch*() and as_arrow(). Affected since ArrowCursor was added in v2.7.0; v3.37.0 kept them only for varbinary columns. The 3.x branch 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.
  • New TestArrowCursor::test_single_column_null (VALUES 1, NULL, 2 and CAST(NULL AS VARCHAR)) checks as_arrow().num_rows and fetchall(). 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/arrow on the same source with an earlier test draft: 106 passed, 1 failed (the new varchar case, which expected None). The expectation was corrected to '' per the documented behavior above; on 6b924fe, -k "single_column_null or binary" in tests/pyathena/arrow/test_cursor.py: 4 passed.
  • Local pyarrow check of the reader options: empty lines in INTEGER, all-NULL, and CRLF single-column CSVs are kept as nulls; header-only and multi-column CSVs give the same tables as before.
  • Not run locally: the full just test pyathena suite; it runs in CI when the PR is marked Ready.

🤖 Generated with Claude Code

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,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 infer string across 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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.py builds the CSV ParseOptions without ignore_empty_lines (pyarrow default True). Holds.
  • Except for varbinary columns — incorrect as written: ignore_empty_lines is absent in v3.34.0–v3.36.0 and present from v3.37.0 (and on 3.x). The release note now says v3.37.0 kept the rows only for varbinary columns and that 3.x has the same code.
  • Async/aio share the result set — pyathena/arrow/async_cursor.py:166 and pyathena/aio/arrow/cursor.py:177 construct AthenaArrowResultSet. Holds; their tests ran in the 106-test local run, but the new regression test is sync only.
  • NULL string → '' — matches docs/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,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

  1. NULL TIME/JSON values raise when fetched. strings_can_be_null=False turns 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 t raises ValueError: time data '' does not match format '%H:%M:%S.%f' in ArrowCursor (the multi-column path is unchanged by this PR), and SELECT CAST(NULL AS TIME) AS t now raises the same instead of returning no rows. JSON: _csv_to_json in Return Athena JSON values from SQLAlchemy JSON columns and the pandas, Arrow, and S3FS cursors #1010 already maps '' to None. 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.
  2. Quoted values with newlines crossing a CSV block boundary fail because newlines_in_values stays False (pyathena/arrow/result_set.py:308). Verified offline with pyarrow: 1,200-byte multi-line values with block_size=1024 raise CSV 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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disposition update for finding 1: the NULL TIME ValueError is fixed in #1033 (commit "Treat an empty TIME value as NULL": _to_time() returns None for '', with an ArrowCursor regression test), which also covers #1030. It stays out of this PR to avoid a duplicate change to _to_time().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disposition update for finding 2: filed as #1040, reproduced on Athena with execute(sql, block_size=1024).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ArrowCursor drops NULL rows of single-column CSV results

1 participant