Rename the opt-in S3FS CSV reader and define its protocol

[laughingman7743]

WHAT

Rename DefaultCSVReader to EmptyStringAsNullCSVReader for PyAthena 4.0, removing the old name while retaining the opt-in empty-string-as-NULL behavior.
Define the public CSVReader protocol and use each reader instance's empty_strings_as_null flag instead of a class-identity check, including for subclasses and independent implementations.
Update all three S3FS cursor families, their tests, and the CSV/NULL handling and API documentation with migration guidance, including the flag required by custom readers.
Direct reader iteration and managed/API fallback retain their existing values.

WHY

AthenaCSVReader is the default, so the old reader name misrepresents its role.
A reader's declared conversion behavior should apply regardless of its class identity.

Closes #1097.

TEST

Validated head: 774219e113d55e6bc4e54c290e7351e7dcfac433.

• just format and just lint: passed.
• uv run --env-file .env pytest -n 1 tests/pyathena/s3fs tests/pyathena/aio/s3fs -q: 133 passed, 1 skipped, including live Athena/S3 coverage for synchronous, threaded asynchronous, and native asyncio cursors. The existing skip requires the insert_test table.
• Offline reader/result-set tests: 57 passed; three cursor-family read_options tests: 6 passed. These are also included in the final targeted suites above.
• Restoring only the original _fetch() class-identity conversion in an isolated Python process (adapting the renamed import) makes 9 regression cases fail.
• Temporary mypy probes: both built-in readers and an independent tuple reader accepted; an incomplete reader rejected as expected.
• just docs lint, just docs build, direct current-tree Sphinx build, and rendered migration/API checks passed. Existing unrelated Sphinx warnings remain.
• Both self-review rounds and the independent Claude Code claude-opus-5-5 review/follow-up (first-party Max profile, effort high) are recorded inline. The fixture-option finding was fixed and the follow-up is CLEAN; independent review was static only.
• All applicable PR checks passed on this head. Ready-triggered Test workflow run 37554213131: Python 3.14, 2767 passed, 10 skipped. The workflow filters excluded the SQLAlchemy and Spark packages and compliance suites for this S3FS-only code/test change. The PR is Ready and mergeable.

[laughingman7743]

Self-review round one — CLEAN (static implementation review).

Frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..146455fe6c59b4e0b87153b426b527d3afd10fd4. Initial pass covered all 13 changed files, including both reader implementations, the structural Protocol and alias, result-set conversion and closure, the sync/threaded/native-asyncio callers, regression tests, and changed docs/API references.

Traced CSV header skipping and tab-separated files, default NULL/empty-string preservation, opt-in normalization before hinted and unhinted conversion, inherited and independently implemented readers, batches/EOF/rownumber, stream closure, managed-results fallback, and pandas' use of the unchanged raw Athena reader. No actionable defects found.

Author validation: required lint passed; reader/result-set offline tests 57 passed; three cursor-family forwarding tests 6 passed. Reinstating the original class-identity _fetch() in an isolated process (renamed import adapted) produced 9 failing regression cases. AWS coverage is pending and is not established by this static review.

[laughingman7743]

Round-one repair follow-up — CLEAN for the corrected scope.

The additional fixture-path audit found the async/aio test-option defect in the original head; the independent reviewer separately confirmed it. This supersedes the original CLEAN conclusion for those test parametrizations. Repair: 774219e113d55e6bc4e54c290e7351e7dcfac433; complete frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..774219e113d55e6bc4e54c290e7351e7dcfac433. Both old objects were verified and the bounded range-diff 971dd12dd1a638429a5691d3be0e300555edb15e..146455fe6c59b4e0b87153b426b527d3afd10fd4 versus 971dd12dd1a638429a5691d3be0e300555edb15e..774219e113d55e6bc4e54c290e7351e7dcfac433 contains only the two fixture corrections and the custom-reader migration clarification.

Traced both indirect fixtures through connect/aio_connect, Connection.cursor_kwargs, cursor creation, and the shared result set: default cases retain AthenaCSVReader; explicit cases now deliver the selected reader. A mocked-session check proved that the former top-level option was ignored and the repaired cursor_kwargs option is applied, with no AWS calls. Existing fixture teardown and expectations are retained. Protocol/reader/result-set code is unchanged. Required format/lint and documentation lint/render checks passed. Live AWS validation remains pending.

[laughingman7743]

Final author runtime validation for 774219e113d55e6bc4e54c290e7351e7dcfac433:

• Local required format/lint and documentation checks passed. Targeted live AWS S3FS suites across all three cursor families: 133 passed, 1 pre-existing skip requiring insert_test.
• Ready-triggered Test workflow 37554213131, Python 3.14: 2767 passed, 10 skipped; all applicable PR checks completed successfully. The job log confirms skip-spark=true and skip-sqla=true, so those packages and compliance suites are outside this workflow selection.
• The published head still matches the self-reviewed and independently reviewed repaired revision. PR state is Ready, MERGEABLE, and CLEAN. The dedicated worktree and main checkout are clean.

These are author/CI execution results; the independent Claude reviews remain source-only. No merge was performed.

[laughingman7743]

Self-review round two — CLEAN (compatibility, claims, and operational review).

Frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..146455fe6c59b4e0b87153b426b527d3afd10fd4. Initial pass covered all 13 changed files and the PR description, with a separate audit of defaults, caller compatibility, public typing, migration examples, NULL handling guarantees, and validation claims.

Confirmed that the old public name is removed for 4.0 and appears only in migration prose; AthenaCSVReader remains the default; direct reader iteration retains its existing values; the instance flag governs CSV/TSV results across all three cursor families, including the type-hint branch. Managed/API fallback retains its existing values and is explicitly excluded from the flag's documented scope. Positive and expected-negative mypy probes support the structural typing claim; documentation lint, full multiversion build, direct current-tree build, and rendered migration/API checks passed. Source inspection does not establish AWS execution.

No additional Athena/S3 requests, retry changes, dependency changes, or unrelated dialect changes are introduced. Existing Sphinx warnings are outside this patch. Shared AWS validation is serialized with other runs; live coverage remains pending. No actionable defects found.

[laughingman7743]

Round-two repair follow-up — CLEAN for the corrected scope.

Frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..774219e113d55e6bc4e54c290e7351e7dcfac433; reviewed the bounded patch-series comparison from 146455fe6c59b4e0b87153b426b527d3afd10fd4 and traced both corrected fixture mappings to their existing connection/cursor contracts. The updated migration note explicitly states that custom readers need empty_strings_as_null, with False preserving empty strings and True normalizing them before result-file type conversion. The new wording rendered successfully and matches the Protocol and actual converter path. Direct iteration and API fallback remain distinct in the docs.

Clarification to the initial operational record: library execution adds no Athena/S3 requests; the integration coverage adds six query cases across the two async cursor families. No retry, dependency, or dialect change is introduced. Repaired test expectations remain based on literal observable values. Static/source checks and mocked forwarding do not establish live AWS results. No further actionable defects found within the repaired scope.

[laughingman7743]

Relayed independent review — FINDINGS (source inspection only).

Reviewer: Claude Code claude-opus-5-5, first-party Max subscription/profile, effort high; session ee9a320b-7504-4422-be17-6da5f2619913. Frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..146455fe6c59b4e0b87153b426b527d3afd10fd4.

The complete diff and all 13 changed files were reviewed with directly related public source/tests and repository conventions. Read/Glob/Grep only; no edits, commands, builds, tests, PR discussion, credentials, personal memory, or GitHub writes. The frozen snapshot was verified unchanged. Actual model usage identified claude-opus-5-5.

[Medium] tests/pyathena/s3fs/test_async_cursor.py:32-33 and tests/pyathena/aio/s3fs/test_cursor.py:27-28 pass csv_reader as a connection setting through their fixtures. Connection.cursor() uses cursor_kwargs, so these settings are dropped and the opt-in cases use AthenaCSVReader: the query returns (None, '', 'text') instead of the expected (None, None, 'text'). Wrap reader options inside cursor_kwargs. Verified by tracing both fixtures and Connection and by a mocked-session forwarding check with no AWS calls.

The reviewer found no defect in the library change, raw reader behavior, hinted/unhinted conversion, managed/API fallback, closure, three cursor callers, the offline result-set tests, or the API documentation. A non-blocking observation recommends mentioning the newly required custom-reader flag in the 4.0 migration note. We retain the explicit Protocol requirement and will document it; no fallback behavior is added.

The author also found the fixture issue during additional source checks while this frozen review was running. The author's worktree has a pending correction, so this review applies to the original published head only; the corrected head requires both self-review follow-ups and an independent follow-up before Ready. Runtime AWS coverage is not established by this review.

[laughingman7743]

Fixed in 774219e113d55e6bc4e54c290e7351e7dcfac433: both indirect fixture parameters now nest csv_reader under cursor_kwargs. Verified the connection/cursor forwarding with a mocked session and no AWS calls. Added the custom-reader flag requirement to the 4.0 migration note rather than introducing an implicit fallback.

Both self-review perspectives rechecked the narrow repair; format/lint and documentation lint/render checks passed. A fresh bounded Claude Max follow-up on this published head is still required before Ready. Live AWS validation remains pending.

[laughingman7743]

Relayed independent repair follow-up — CLEAN (static review only).

Reviewer: Claude Code claude-opus-5-5, verified first-party Max subscription/profile, effort high; session e94b8e0d-4197-4098-b30a-926f44509588. New frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..774219e113d55e6bc4e54c290e7351e7dcfac433; old range ended at 146455fe6c59b4e0b87153b426b527d3afd10fd4.

The reviewer inspected the verified patch-series comparison and all three repaired files, tracing both indirect fixtures through connect/aio_connect, Connection's cursor_kwargs merge, both async cursor constructors, and the shared result set. The corrected reader selections and literal NULL/empty-string expectations are consistent. The new migration sentence accurately requires the custom-reader flag and scopes it to result files rather than managed/API fallback. No further actionable defects found within the repaired scope.

Read/Glob/Grep only, no edits, builds, tests, commands, GitHub writes, credentials, PR discussion, personal memory, or delegation. Model usage confirmed claude-opus-5-5; permission denials were empty. The final review snapshot and author worktree were verified unchanged. Together with the completed initial full review, this resolves the fixture finding. Runtime Athena/S3 validation is still separate and running; it was not performed by the independent reviewer.