Skip to content

Read multi-line CSV values that cross a block in ArrowCursor #1045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laughingman7743 merged 2 commits into from
Oct 4, 2026
Merged

Read multi-line CSV values that cross a block in ArrowCursor #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aef05e4
Read multi-line CSV values that cross a block in ArrowCursor
laughingman7743 Oct 3, 2026
ca69d5e
Merge branch 'master' into fix/1040-arrow-csv-multiline-blocks
laughingman7743 Oct 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pyathena/arrow/result_set.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,9 @@ def _read_csv(self) -> Table:
ignore_empty_lines=False,
double_quote=True,
escape_char=False,
# A quoted value can contain a newline, so the reader must not split
# blocks inside quotes.
newlines_in_values=True,

@laughingman7743 laughingman7743 Oct 3, 2026

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 28ec68d9f70b5bc2fb447f6a4fb745ee0a467683, head aef05e4a36ec49af94cf685c1e51b638a2b6a597.

Covered: the .csv branch of AthenaArrowResultSet._read_csv() and every Arrow reader of it (ArrowCursor, AsyncArrowCursor, AioArrowCursor); the .txt branch (tab-separated, quote_char=False, unchanged); UNLOAD/Parquet and managed (GetQueryResults) paths, which do not use these options; the interaction with ignore_empty_lines=False from #1031 (empty lines are single-column NULL rows); block_size plumbing from execute(); other CSV readers in the package (only this module uses pyarrow.csv.ParseOptions, while pandas, Polars, and S3FS use their own quote-aware readers); newlines_in_values availability (present well before the pyarrow>=22 floor); the new test.

Result: CLEAN.

  • Offline, with these options and block_size=64, 200 empty lines interleaved with 200 two-line values read as 400 rows, so Keep NULL rows of single-column CSV results in ArrowCursor #1031's NULL rows survive the quote-aware chunker.
  • The test reads 50 two-line 301-byte values at block_size=1024 (about 15 blocks) and asserts every row. With the fix reverted, it fails with CSV parser got out of sync with chunker.
  • Live: tests/pyathena/arrow/test_cursor.py 56 passed.

@laughingman7743 laughingman7743 Oct 3, 2026

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): Codex CLI 0.160.0, model gpt-6-astra, codex exec -s read-only --ephemeral, session 01a1029d-81d3-7f82-ac4e-5526ad97f04a. Base 28ec68d9f70b5bc2fb447f6a4fb745ee0a467683, head aef05e4a36ec49af94cf685c1e51b638a2b6a597, detached snapshot (unchanged afterwards). Prompt had the literal diff and repository conventions only, with no PR text or prior findings. This was a static review: no builds, tests, or network access.

Reviewer coverage: Arrow result-set init, _as_arrow(), _read_csv(), conversion and fetching; sync, threaded-async, asyncio, and SQLAlchemy callers; the new test and fixture; the CSV/TSV/Parquet/API branches; the pandas/Polars/S3FS sibling readers; dependency sources matching uv.lock.

Verdict: FINDINGS, none of them in this diff. No regression was found in the Arrow change, and the new test reaches the parser and crosses block boundaries.

  1. P2, pre-existing sibling defect (pyathena/pandas/result_set.py:594, pyathena/polars/result_set.py:498): PandasCursor with engine="pyarrow" and PolarsCursor with use_pyarrow=True let pandas or Polars build pyarrow ParseOptions without newlines_in_values, so multi-line values crossing their 1 MiB blocks raise the same out-of-sync error.

Author verification: confirmed offline. On a 9.6 MB CSV of 8,000 two-line values, pd.read_csv(engine="pyarrow") and pl.read_csv(use_pyarrow=True) raise the error, while engine="c" and the native Polars reader read 8,000 rows. Deferred to #1048: neither wrapper exposes newlines_in_values, so a fix needs a PyAthena-controlled read path, which goes beyond this PR. The default engines are unaffected. The pandas file is also being edited for #1027.

No change to this PR from the independent review.

)
else:
return pa.Table.from_pydict({})
Expand Down
12 changes: 12 additions & 0 deletions tests/pyathena/arrow/test_cursor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@ def test_binary_null_vs_empty(self, arrow_cursor):
]
assert [row[3] for row in rows] == ["", "", "NULL"]

def test_multiline_values_across_blocks(self, arrow_cursor):

@laughingman7743 laughingman7743 Oct 3, 2026

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, compatibility, operations): base 28ec68d9f70b5bc2fb447f6a4fb745ee0a467683, head aef05e4a36ec49af94cf685c1e51b638a2b6a597.

Claims checked:

  • 128 MiB default: DEFAULT_BLOCK_SIZE = 1024 * 1024 * 128 (result_set.py:66).
  • "A single value larger than block_size still fails": reproduced offline with pyarrow 25.0.1, where 2001-byte values at block_size=1024 raise straddling object straddles two block boundaries with the option on, and the out-of-sync error with it off.
  • Parse cost: measured locally on a 250 MiB CSV, adding 0.033–0.043 s (best of 3). The PR text originally said "about 0.04 s" and now says 0.03–0.04 s. This is local CPU time only; the S3 transfer is not measured.
  • Origin: the options date from e743003 (v2.7.0), so 3.x has the same defect; the PR targets master.

Existing callers: no signature or default changes; results that did not cross a block parse identically (the existing Arrow suite passes); a result that used to raise OperationalError now returns rows. No extra AWS requests.

Result: CLEAN after the wording correction. Async and aio Arrow tests were not run locally; AWS CI runs them when the PR is Ready.

# The 50 two-line values of 301 bytes span several 1024-byte blocks.
arrow_cursor.execute(
"""
SELECT array_join(repeat('x', 150), '') || chr(10) || array_join(repeat('y', 150), '')
AS v
FROM UNNEST(sequence(1, 50)) AS t(i)
""",
block_size=1024,
)
assert arrow_cursor.fetchall() == [("x" * 150 + "\n" + "y" * 150,)] * 50

def test_binary_single_null(self, arrow_cursor):
arrow_cursor.execute("SELECT CAST(NULL AS VARBINARY) AS value")
assert arrow_cursor.fetchall() == [(None,)]
Expand Down
Loading