[laughingman7743]

WHAT

• Failed fetches break the result set (Define the behavior of result set fetches after an error #1225 option (c)). After a fetch raises, later fetchone(), fetchmany(), and fetchall() calls raise ProgrammingError("The result set cannot be fetched after a failed fetch.") without reading further rows. This covers request errors, interrupts, values that fail to convert, and asyncio cancellations. Argument validation errors, such as a non-integer fetchmany() size, do not break the result set.
  • The base, asyncio, Arrow, S3FS, pandas, and Polars result sets do this.
  • The pandas and Polars chunk iterators raise for reads after a failed read. As a result, a failed iter_chunks() read also breaks the fetch methods that share the iterator.
• Cancelled fetches of AioPandasCursor, AioArrowCursor, AioPolarsCursor, and AioS3FSCursor. These fetches now run through the new pyathena.aio.util.run_in_thread_or_close().
  • A cancellation skips a fetch that has not started and stops the retries of a running one.
  • The cursor drops the result set, so later fetches raise ProgrammingError("No result set.").
  • The result set is closed once its fetch is no longer running: by the fetch's thread when the fetch ends, otherwise by a new thread. As with create_in_thread(), the close needs neither the event loop nor its executor.
  • create_in_thread() now uses the same logging close.
• Removed cross-thread guards. PyAthena declares threadsafety = 2, so code that served only concurrent use of one cursor or result set is removed:
  • Defer closing a chunk reader while another thread reads it #1182: the deferred chunk-reader close in the pandas and Polars iterators
  • Discard the page of a request that close() interrupts #1197: the is_closed checks after a page request
  • Take buffered rows in bulk in fetchmany() and fetchall() #1187: the buffer replacement and the IndexError handling in the bulk take
  • Stop the retries of closed result sets and cancelled asyncio calls #1223: the stop_when parameter of retry_api_call() and async_retry_api_call(), and its use to stop GetQueryResults retries on close()
  • Give the chunk readers the state that close() clears when they are created #1190: the paths parameter of the pandas Parquet chunk reader and the stated reasons. The Polars chunk readers keep the state they take at creation.
• Docs.
  • docs/usage.md: a failed fetch makes later fetches raise, except for invalid arguments. Calling one cursor or its result set from several threads, including close() and cancel(), is unsupported. The callback text points to stopping a query by its ID. The example that cancelled a query from a timer thread is removed.
  • docs/aio.md, docs/pandas.md, docs/polars.md, docs/arrow.md, and docs/s3fs.md: how the asyncio cursors run fetches in threads and what a cancellation does.

Release note (breaking):

• Fetches after a failed fetch raise ProgrammingError. Before, the pandas and Polars result sets returned no more rows, and the other result sets continued after skipped or dropped rows.
• Calling one cursor or result set from several threads is unsupported, as threadsafety = 2 declares. This includes cancel() from a timer thread, which an earlier documentation example showed, and close() during a fetch.
• A cancelled fetch of the asyncio pandas, Arrow, Polars, and S3FS cursors drops the result from the cursor.
• The unreleased guards from Defer closing a chunk reader while another thread reads it #1182, Take buffered rows in bulk in fetchmany() and fetchall() #1187, Give the chunk readers the state that close() clears when they are created #1190, Discard the page of a request that close() interrupts #1197, and Stop the retries of closed result sets and cancelled asyncio calls #1223 are removed. Their draft release-note entries should be dropped.

WHY

Closes #1206. Closes #1225.

#1225 decided option (c): a failed fetch breaks the result set. #1206 dropped the proposed operation guard because concurrent use of a cursor is unsupported under threadsafety = 2. See #1206 (comment) and the contract update on #1205: #1205 (comment).

The asyncio cursors created the one concurrent close themselves. A fetch that keeps running in its thread after a cancellation was closed when the task left async with or called execute() again (#1172). The cursor now drops that result set instead, and the result set is closed after the fetch ends.

TEST

Tested commit: 06adf23.

• just lint: passed. markdownlint on the changed docs: 0 errors.
• uv run --env-file .env pytest -n 2 tests/pyathena/test_result_set.py tests/pyathena/aio/test_result_set.py tests/pyathena/aio/test_common.py tests/pyathena/pandas/test_result_set.py tests/pyathena/polars/test_result_set.py tests/pyathena/aio/test_util.py tests/pyathena/test_util.py tests/pyathena/arrow/test_result_set.py tests/pyathena/s3fs/test_result_set.py: 550 passed, 10 skipped.
• New unit tests, with no AWS calls:
  • Failed fetches (request error, KeyboardInterrupt, conversion error) for the base and asyncio result sets.
  • A cancelled AthenaAioResultSet fetch.
  • Failed reads of the pandas and Polars chunk iterators.
  • Failed iter_chunks() reads breaking the pandas and Polars fetches.
  • Cancelled WithAsyncFetch fetches: retries stopped, result set dropped and closed off the event loop after the fetch ends.
  • run_in_thread_or_close() cancelled before, during, and after its function.
• Tests of the removed cross-thread behavior and of stop_when were removed.
• Performance, measured locally:
  • Sync fetchone() over 1,000,000 buffered rows: 133 ns per row on both master and this branch.
  • Asyncio fetchone() through the thread helper: about the same as run_in_thread(). An asyncio.wait() or asyncio.shield() variant added about 20 µs per call and was not used.
• Interrupt during row counting, pandas and Polars (test_fetchone_interrupted_while_counting).
• tests/pyathena/pandas/test_cursor.py -k binary_csv_stream_close (AWS): 20 passed. The test now expects ProgrammingError after a failed chunk read; an earlier AWS CI run failed on its old expectation.
• aio cursor integration tests (tests/pyathena/aio/{pandas,polars,arrow,s3fs}/test_cursor.py, tests/pyathena/aio/test_cursor.py): 139 passed.
• The full suite and the SQLAlchemy suites run in AWS CI.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (behavior and implementation)

• Base: 59169c5e137a0b453e8bd441ddbdd4b028df4f94; initial head: d1dcf0ce61a0bd51d41eacfd1464914874844cdc; repairs through ef1b3a094 (94c45e02, 54f9750a, ef1b3a09).
• Covered: base/aio/pandas/Polars/Arrow/S3FS fetch paths (fetchone/fetchmany/fetchall, iteration), pandas/Polars chunk iterators (__next__, get_chunk, itertuples, iter_chunks), WithAsyncFetch fetches and cancellation, retry_api_call/async_retry_api_call, create_in_thread, removed guards, tests. Ran /code-review (high) and /simplify in addition to the manual pass.
• Result: FINDINGS, repaired.
  1. retry_api_call(stop_when=...) had no caller after the guards were removed (pyathena/util.py), so it was removed. It is unreleased: it is not in v4.1.0.
  2. asyncio.shield() let a cancelled fetch that was still queued run in full (pyathena/aio/common.py). The close of the detached result set relied on the event loop: inline on the loop thread after an executor shutdown, and never after the loop closed. The fix is the new run_in_thread_or_close(), which skips a queued function and closes in the fetch's thread or a new thread, as create_in_thread() does. It awaits the future directly, because asyncio.wait()/shield() cost about 20 µs per call (measured).
  3. The pandas/Polars _take_rows stopped counting the rows taken before an exception, unlike the base. Restored.
  4. Docstrings: the Polars __next__ section order, AioS3FSCursor, and WithAsyncFetch. Docs: the invalid-argument exception and the release note for cross-thread cancel().
  5. Simplify: one failure message, a shared assert_fetches_fail, a single running flag, create_in_thread using the logging close, the pandas Parquet chunk reader reading self._data_manifest again, and _count_rows.
• Measured: sync fetchone over 1M buffered rows takes 133 ns/row on master and on this branch. The asyncio fetch helper costs about the same as run_in_thread().
• Deferred:
  • The guard is inlined in 8 fetch methods. The base fetchone is a per-row path that the code deliberately inlines.
  • A shared iterator mixin would restructure classes, which needs approval.
  • Reverting the Polars reader split from Give the chunk readers the state that close() clears when they are created #1190 would change when the CSV size check runs. It is a follow-up candidate.
  • A busy-executor test fixture would refactor an existing test pattern.

[laughingman7743]

Self-review round two (claims, compatibility, operations)

• Base: 59169c5e137a0b453e8bd441ddbdd4b028df4f94; head: a2d2fc005789ff771f0e84a4e0154dd45a861520.
• Claims checked:
  • threadsafety = 2 (pyathena/__init__.py:30). PEP 249 level 2 lets threads share connections, not cursors.
  • Validation errors do not break the result set: operator.index(size) runs before the check, in sync and aio.
  • AsyncCursor.cancel(query_id) exists (pyathena/async_cursor.py:304).
  • The removed guards and stop_when (Defer closing a chunk reader while another thread reads it #1182, Take buffered rows in bulk in fetchmany() and fetchall() #1187, Give the chunk readers the state that close() clears when they are created #1190, Discard the page of a request that close() interrupts #1197, Stop the retries of closed result sets and cancelled asyncio calls #1223) are in no tag, and git show v4.1.0:pyathena/util.py has no stop_when. The public signatures therefore match v4.1.0.
• Corrections:
  • docs/pandas.md, docs/polars.md, docs/arrow.md, and docs/s3fs.md said the asyncio cursors use asyncio.to_thread() for creation and fetches. That was already inaccurate for creation (create_in_thread) and is now inaccurate for fetches.
  • docs/aio.md now says what a cancellation does to a fetch that has not started.
  • The PR body now covers the stop_when removal, the partial Give the chunk readers the state that close() clears when they are created #1190 revert, the cross-thread cancel() release note, and dropping the earlier draft release-note entries.
• Existing callers:
  • SQLAlchemy async adapters close the cursor after a failed fetch. After a cancelled fetch the cursor has no result set, so that close does nothing, and the result set is closed by the helper.
  • No constructor or keyword changes.
• AWS: no new requests.
  • A cancellation still stops the retries of a running fetch.
  • A close() from another thread no longer stops GetQueryResults retries. That use is unsupported.
• Evidence at ef1b3a09 (code); a2d2fc00 changes only docs:
  • Affected unit/integration files: 548 passed, 10 skipped.
  • aio cursor integration tests (tests/pyathena/aio/{pandas,polars,arrow,s3fs}/test_cursor.py, tests/pyathena/aio/test_cursor.py): 139 passed.
  • just lint passed. markdownlint: 0 errors. just docs build succeeded.
  • The full suite and SQLAlchemy suites run in AWS CI at Ready.
• Result: FINDINGS (docs/PR claims), corrected.
• Open: docs/spark.md:280 still shows threading.Timer(60, cursor.cancel) on the same cursor. Spark has no start callback, so this waits for the maintainer's decision.

[laughingman7743]

Independent review (relayed), round one

• Reviewer: Codex CLI (codex exec -s read-only), model gpt-6-astra, reasoning effort max, session 01a14923-03ff-7cf3-8386-30021435220b. A static review: no tests, builds, edits, or network access.
• Scope: 59169c5e137a0b453e8bd441ddbdd4b028df4f94..a2d2fc005789ff771f0e84a4e0154dd45a861520, on a detached snapshot that stayed unchanged. The prompt held the diff and the intended contract, with no PR text or prior findings.
• Covered: base/dict, asyncio, Arrow, S3FS, pandas, and Polars fetch paths, argument validation, chunk iteration, and cleanup; run_in_thread_or_close, create_in_thread, cancellation handoffs, executor shutdown, and retry propagation; sync cursors, AsyncCursor futures, SQLAlchemy adapters, retry-helper callers, tests, and docs.
• Result: FINDINGS (4).
  1. P2: after iter_chunks() fails reading the next chunk, rows already buffered in the row iterator from an earlier chunk are still returned before the fetch raises (pandas/result_set.py:977, polars/result_set.py:626). Rejected as intended. Those rows come from a chunk that was read successfully, in order, with nothing skipped or duplicated. This matches the existing contract that test_fetch_after_chunks_run_out tests: the rows of a chunk that a fetch started stay. The next fetch that needs a new chunk raises ProgrammingError.
  2. P2: the ParquetChunkReader prefetch executor (arrow/reader.py:197) does not copy context, so a cancellation does not stop the retries of a prefetch GetObject, and the cleanup waits for them. Deferred as pre-existing. The read-ahead/prefetch pools were deliberately left out of the stop scopes in Cancelled AioS3FileSystem operations keep retrying in executor threads #1203/Stop the S3 retries of cancelled calls and of failed requests' siblings #1231. Before this PR, a cancelled fetch stopped no retries at all.
  3. P2: the pandas/Polars fetchone() called _count_rows(1) outside the exception handler, so an interrupt there left fetching enabled. Fixed in b8b5ebad: the count moved inside the handler, and test_fetchone_interrupted_while_counting was added for both.
  4. P3: test_run_in_thread_or_close_cancelled_after_function relied on a 100 ms sleep. Fixed in b8b5ebad: it now waits for the call to return through the next job of a single-worker executor. 5 consecutive runs passed.
• Validation after repair: just lint passed. Affected test files: 550 passed, 10 skipped.

[laughingman7743]

Repair follow-up for b8b5ebad

• Self-review of the repair:
  • Round one (behavior): the pandas/Polars fetchone() now count inside the handler, as the base does. The new test_fetchone_interrupted_while_counting covers both backends.
  • Round two (claims): no docs, PR text, or public contracts change.
• Independent follow-up (relayed):
  • Reviewer: Codex CLI codex exec -s read-only, model gpt-6-astra, reasoning effort max, session 01a1492e-b9ac-76b2-ba82-1d59cdb53539. A static review.
  • Scope: a2d2fc005789ff771f0e84a4e0154dd45a861520..b8b5ebad56a1161f80886fe7822372640678ffa6 on the detached snapshot.
  • Result: CLEAN. The interrupt tests distinguish base from head. The cancellation test reaches the new-thread close deterministically, with no deadlock or executor leak.
  • Codex noted that the cancellation test passes on both revisions, since its repair only removes the timing dependency.

[laughingman7743]

AWS CI failure and repair (06adf235)

• Failure: AWS CI run 39021324039, job test / run (3.15) at b8b5ebad. 4 failed, 4367 passed. All 4 failures were TestPandasCursor.test_binary_csv_stream_close_on_read_error[{next,get_chunk}-{c,python}], which expected StopIteration and fetchone() is None after a failed chunk read. That is the behavior Define the behavior of result set fetches after an error #1225 replaced; the new ProgrammingError was raised as designed. test-sqla and test-sqla-async passed.
• Repair: the test now expects ProgrammingError from next(), get_chunk(), and fetchone() after the failed read. Locally uv run --env-file .env pytest -n 2 tests/pyathena/pandas/test_cursor.py -k binary_csv_stream_close gave 20 passed. just lint passed.
• Self-review: round one, the assertion matches the iterator message and the shared-iterator path of fetchone(). Round two, it is a test-only change with no claim changes, and no other cursor test still asserts the old post-failure behavior; the full CI run showed no other failures.
• Independent follow-up (relayed): Codex CLI codex exec -s read-only, model gpt-6-astra, effort max, session 01a14940-30c3-7a62-8288-2ee4b7d4edec, range b8b5ebad56a1161f80886fe7822372640678ffa6..06adf235f6341a33104c439debfeef619442b5f9. A static review. Result: CLEAN. The remaining StopIteration/None/[] expectations in the file cover normal closure or exhaustion only.