You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Decide whether the Polars chunk readers keep the state they take at creation #1249
#1190 (for #1188) split the chunk readers of AthenaPolarsResultSet into a factory that takes the result set's state when the reader is created, plus a generator that only uses that state. The pairs are _iter_csv_chunks() / _scan_csv_chunks() and _iter_parquet_chunks() / _read_parquet_chunks() / _scan_parquet_chunks(), and _get_parquet_storage_options() became a static method that takes the connection. The stated reason was that a close() from another thread could clear that state before the first read. That case arose with the deferred close of #1182.
#1206 (PR #1238) declared using one cursor or result set from several threads, including close(), unsupported under threadsafety = 2. It removed the deferred close and the stated reasons, and reverted the same change in the pandas Parquet chunk reader. The Polars split was kept because merging it back changes behavior:
_iter_csv_chunks() checks the CSV file size (_is_csv_readable(), a HeadObject request) and builds the read options when the iterator is created, during execute(). A single generator would make that request at the first read instead, so its errors would move from execute() to the first fetch.
Read UNLOAD timestamps at microseconds #1233 has since passed the Parquet schema from _read_parquet_schema() into _iter_parquet_chunks(), so the Parquet side has another reason to take state at creation.
Proposed change
Decide whether the Polars chunk readers should go back to single generators now that the cross-thread case is unsupported:
Keep the split and only state its current reasons in the docstrings: errors are reported at execute(), and the schema is read once.
Merge the CSV pair back into one generator, which moves the HeadObject check and its errors to the first read. Keep the Parquet factory for the schema.
Merge both where the schema can be passed differently.
Option 1 changes no behavior. Options 2 and 3 simplify the code at the cost of when errors surface, which is a user-visible change and needs a release note.
Validation plan (if implementing)
For options 2 and 3: unit tests for when the CSV size check and its errors happen, the existing Polars chunked CSV and UNLOAD tests in tests/pyathena/polars/, and just test pyathena for the Polars cursor tests against Athena.
Decision
Option 1: keep the split and state its current reasons in the docstrings, with no behavior change. PR #1271 closes this issue.
Activity