Skip to content

Follow-ups of #1208: duplicated blocking close paths, AsyncCursor work after close, and description() cost #1281

Closed

Description

@laughingman7743

Follow-up of #1208 (PR #1279). Part of #1205.

These three items were found during the reviews of #1279 and deferred there.

1. Duplicated blocking and asyncio close paths

PR #1279 added blocking releases for objects whose close() is a coroutine. Each one repeats its asyncio counterpart:

  • AioSparkCursor._close_blocking() (pyathena/aio/spark/cursor.py) repeats SparkBaseCursor.close() (pyathena/spark/common.py) and AioSparkCursor.close(). Only the termination call differs: _terminate_session_by_id() instead of the coroutine _terminate_session(). SparkBaseCursor.close() goes through the _terminate_session() hook, which tests/pyathena/spark/test_async_cursor.py patches. That is why the release could not simply be SparkBaseCursor.close.
  • AioS3StreamedFile._close_without_commit_blocking() (pyathena/filesystem/s3_async.py) repeats _close_without_commit(): the closed flag, dropping the buffer, begin_abort(), _discard(), and the pool shutdown. The asyncio version begins the abort on the event loop's thread and runs _discard() shielded in the pool, while the blocking version runs it inline.

A later fix to one copy can miss the other. Possible fixes:

  • Have SparkBaseCursor.close() call _terminate_session_by_id(), so that release=SparkBaseCursor.close works, and update the tests that patch _terminate_session().
  • Share the synchronous part of the file close, for example a _begin_close_without_commit() that both versions call.

2. AsyncCursor futures still poll and build result sets after the cursor closes

AsyncCursor.close() shuts down its executor without cancel_futures, so every execute() future still runs to its end. The future polls the query until it reaches a terminal state, however long the query runs, and then builds the whole result set. For pandas, Arrow, and Polars, that means downloading and parsing the whole CSV or UNLOAD output; for S3FS, it means opening the file. Since #1279, the worker closes that result set right away. AthenaResultSet.close() also discards the query execution and metadata, so the caller gets an object with nothing left in it. The cost is worker threads, Athena polling requests, S3 reads, and memory, all for nothing.

Facts that limit the options:

  • shutdown(cancel_futures=True) would also cancel the queued futures of cancel(query_id), poll(), query_execution(), and description(). For example, cursor.cancel(query_id); cursor.close() would then never send the cancel request.
  • The sync _poll() loop has no stop condition other than a terminal state.

Options to decide:

  • (a) Check _closed before polling, between polls, and after polling. If the cursor has closed, end the future with ProgrammingError without building the result set. Keep the check after the build for a close during the build. This is breaking: after a close, future.result() raises instead of returning a closed result set.
  • (b) Cancel only the result-set futures that have not started, which raise CancelledError. Running ones still poll and build.
  • (c) Keep the current behavior and document the cost.

3. AsyncCursor.description() builds the whole result set

AsyncCursor._description() calls the variant's _collect_result_set() only to read description, and closes the result set afterward (since #1279). For AsyncPandasCursor, AsyncArrowCursor, and AsyncPolarsCursor, that downloads and converts the whole result; for AsyncS3FSCursor, it opens the CSV file.

A cheaper path has to keep the same descriptions:

  • For CSV results, the description comes from the GetQueryResults metadata, which the first page has.
  • With unload=True, the pandas, Arrow, and Polars result sets derive it from the Parquet schema, read from the footer (parquet.read_metadata), not from GetQueryResults.

So the fix needs a metadata-only path for each variant, not just the base AthenaResultSet. Check that the descriptions match the current ones for every variant, with and without UNLOAD.

Environment

Decisions (2026-10-19)

  • Item 1: share the close paths as proposed (SparkBaseCursor.close() calls _terminate_session_by_id(); a shared synchronous part of the file close).
  • Item 2: option (a), after consulting Codex and Claude Fable 5.1. After AsyncCursor closes, every future that waits for a query (execute(), description(), poll()) ends with ProgrammingError at its next status check, including the one after the terminal state, so that no result set is built. cancel() and query_execution() futures still run. AsyncCursor overrides _poll_until_terminal(), and close() sets an event that ends the wait between polls. A close during the build still closes the built result set. Breaking; needs a release note. AsyncSparkCursor is not an AsyncCursor and is out of scope.
  • Item 3: in the same PR. Two designs were rejected: a _metadata_only constructor mode, and a metadata phase on the result sets with AthenaResultSet.describe() creating a result set that reads nothing. After a third consultation with Codex and Claude Fable 5.1 (2026-10-19), the result set metadata is read by separate classes. Each class sits in the module of the result set that it serves.
    • AthenaResultSetMetadataReader (pyathena/result_set.py) reads the result set metadata as the Athena API gives it. It builds and sends GetQueryResults requests, reads a page of one row for the metadata and update count, parses ResultSetMetadata, and formats the DB API descriptions (with the DML rule). describe() returns the descriptions without rows. The result sets page through it.
    • AthenaUnloadResultSetMetadataReader (same module) is shared by pandas, Arrow, and Polars. It decides whether the result is UNLOAD output, reads the data manifest, and derives the unload location. A result without data files has no columns; otherwise it reads the Parquet metadata of its variant.
    • AthenaPandasResultSetMetadataReader: the PyAthena filesystem, the normalized read options, the Parquet engine, the chunked-read decision, the footers (first footer, NULL dtypes).
    • AthenaArrowResultSetMetadataReader: the pyarrow filesystem and the ParquetDataset.
    • AthenaPolarsResultSetMetadataReader: the normalized read options, the storage options, and the scan_parquet() schema.
    • The result sets create their reader, take its metadata, and hand what it prepared (first footer, dataset, schema) to their data phase. They keep reading, converting, and fetching rows.
    • Each result set class declares its reader class as the class variable _metadata_reader_class; the pairing is fixed, so it is not configurable. The async cursors have _metadata_reader_arguments(), which _result_set_arguments() includes with the row options. description() creates only self._result_set_class._metadata_reader_class with those arguments, never the result set itself.
    • Removed: AthenaResultSet.describe(), _read_metadata(), _process_result_metadata(), and the variants' _pre_fetch parameters.
    • Decided: an UNLOAD result has no columns only when it has no data files. pandas and Polars read whole no longer return [] for a result whose rows the read options drop (breaking, release note).

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions