Repository navigation
Backport bug fixes to 3.x for v3.38.0 #1104
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.
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
Changes from all commits
c09ac2f
b18e533
3800144
d81b11c
550503c
b82fdb9
3a14b71
df27eb1
eb74aa8
0d05a7f
0fd65a7
d534549
63eb967
0afb784
3d36fbc
e05d33b
896c287
a3cba8d
af6f8fb
683658f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
| import logging | ||
| import sys | ||
| from datetime import datetime, timedelta, timezone | ||
| from typing import Any, cast | ||
| from typing import Any, NoReturn, cast | ||
|
|
||
| from pyathena.aio.util import async_retry_api_call | ||
| from pyathena.common import BaseCursor, CursorIterator | ||
|
|
@@ -60,12 +60,16 @@ async def _execute( # type: ignore[override] | |
| result_reuse_minutes=options.result_reuse_minutes, | ||
| execution_parameters=execution_parameters, | ||
| ) | ||
| query_id = await self._find_previous_query_id( | ||
| query, | ||
| options.work_group, | ||
| cache_size=options.cache_size, | ||
| cache_expiration_time=options.cache_expiration_time, | ||
| ) | ||
| query_id = None | ||
| # Athena does not return the ExecutionParameters of earlier executions, | ||
| # so the cache cannot tell which parameters an execution ran with (#941). | ||
| if not request.get("ExecutionParameters"): | ||
| query_id = await self._find_previous_query_id( | ||
| query, | ||
| options.work_group, | ||
| cache_size=options.cache_size, | ||
| cache_expiration_time=options.cache_expiration_time, | ||
| ) | ||
| if query_id is None: | ||
| try: | ||
| response = await async_retry_api_call( | ||
|
|
@@ -376,6 +380,7 @@ class WithAsyncFetch(AioBaseCursor, CursorIterator, WithResultSet): | |
| ``rownumber``, ``rowcount``), lifecycle methods (``close``, ``executemany``, | ||
| ``cancel``), default sync fetch (for cursors whose result sets load all | ||
| data eagerly in ``__init__``), and the async iteration protocol. | ||
| Synchronous iteration raises ``TypeError``. | ||
|
|
||
| Subclasses override ``execute()`` and optionally ``__init__`` and | ||
| format-specific helpers. | ||
|
|
@@ -504,6 +509,14 @@ def fetchall( | |
| result_set = cast(AthenaResultSet, self.result_set) | ||
| return result_set.fetchall() | ||
|
|
||
| def __iter__(self) -> NoReturn: | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Round 1 note (#899):
|
||
| """Reject synchronous iteration; use ``async for`` instead. | ||
|
|
||
| Raises: | ||
| TypeError: Always, because the fetch methods are coroutines. | ||
| """ | ||
| raise TypeError(f"'{type(self).__name__}' object is not iterable; use 'async for' instead.") | ||
|
|
||
| def __aiter__(self): | ||
| return self | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,6 +119,9 @@ def __init__( | |
| import pyarrow as pa | ||
|
|
||
| self._table = pa.Table.from_pydict({}) | ||
| # The fetch methods convert only the values read from a result file. | ||
| # GetQueryResults values are already converted. | ||
| self._convert_rows = bool(self.output_location) | ||
| self._batches = iter(self._table.to_batches(arraysize)) | ||
|
|
||
| def __s3_file_system(self): | ||
|
|
@@ -215,11 +218,15 @@ def _fetch(self) -> None: | |
| return | ||
| else: | ||
| dict_rows = rows.to_pydict() | ||
| column_names = dict_rows.keys() | ||
| processed_rows = [ | ||
| tuple(self.converters[k](v) for k, v in zip(column_names, row, strict=False)) | ||
| for row in zip(*dict_rows.values(), strict=False) | ||
| ] | ||
| if self._convert_rows: | ||
| converters = self.converters | ||
| column_names = dict_rows.keys() | ||
| processed_rows = [ | ||
| tuple(converters[k](v) for k, v in zip(column_names, row, strict=False)) | ||
| for row in zip(*dict_rows.values(), strict=False) | ||
| ] | ||
| else: | ||
| processed_rows = list(zip(*dict_rows.values(), strict=False)) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Independent review (relayed): Codex CLI 0.160.0, model
Finding (introduced, P2):
Author verification: confirmed, deferred.
|
||
| self._rows.extend(processed_rows) | ||
|
|
||
| def fetchone( | ||
|
|
@@ -297,9 +304,13 @@ def _read_csv(self) -> Table: | |
| parse_opts = csv.ParseOptions( | ||
| delimiter=",", | ||
| quote_char='"', | ||
| ignore_empty_lines=not binary_columns, | ||
| # Athena writes a single-column row with a NULL value as an empty line. | ||
| 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, | ||
| ) | ||
| else: | ||
| return pa.Table.from_pydict({}) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,7 +94,7 @@ def get_athena_type(type_: DataType) -> tuple[str, int, int]: | |
| return "date", 0, 0 | ||
| if type_.id == types.Type_TIMESTAMP: # 18 | ||
| return "timestamp", 3, 0 | ||
| if type_.id in [types.Type_DECIMAL128, types.Decimal256Type]: # 23, 24 | ||
| if type_.id in [types.Type_DECIMAL128, types.Type_DECIMAL256]: # 23, 24 | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Self-review round 2: claims, callers and operations: Scope: Claims checked against evidence:
|
||
| type_ = cast(types.Decimal128Type, type_) | ||
| return "decimal", type_.precision, type_.scale | ||
| if type_.id in [ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
| from abc import ABCMeta, abstractmethod | ||
| from collections.abc import Callable | ||
| from copy import deepcopy | ||
| from datetime import date, datetime, time | ||
| from datetime import date, datetime, time, timedelta, timezone | ||
| from decimal import Decimal | ||
| from typing import Any, ClassVar | ||
|
|
||
|
|
@@ -40,11 +40,33 @@ def _to_datetime(varchar_value: str | None) -> datetime | None: | |
| return datetime.strptime(varchar_value, "%Y-%m-%d %H:%M:%S.%f") | ||
|
|
||
|
|
||
| _UTC_OFFSET_PATTERN: re.Pattern[str] = re.compile(r"([+-])(\d{2}):(\d{2})") | ||
|
|
||
|
|
||
| def _parse_utc_offset(value: str) -> timezone | None: | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Self-review round 1: implementation behavior: Scope: Covered:
Recorded limitations (3.x consequences of the agreed scope, not defects of this PR) are in the threads below. |
||
| """Parse a ``+HH:MM`` or ``-HH:MM`` UTC offset. | ||
|
|
||
| Args: | ||
| value: The text to parse. | ||
|
|
||
| Returns: | ||
| The fixed-offset time zone, or None if the text is not an offset. | ||
| """ | ||
| match = _UTC_OFFSET_PATTERN.fullmatch(value) | ||
| if not match: | ||
| return None | ||
| sign, hours, minutes = match.groups() | ||
| offset = timedelta(hours=int(hours), minutes=int(minutes)) | ||
| return timezone(-offset if sign == "-" else offset) | ||
|
|
||
|
|
||
| def _to_datetime_with_tz(varchar_value: str | None) -> datetime | None: | ||
| if varchar_value is None: | ||
| return None | ||
| datetime_, _, tz = varchar_value.rpartition(" ") | ||
| return datetime.strptime(datetime_, "%Y-%m-%d %H:%M:%S.%f").replace(tzinfo=gettz(tz)) | ||
| return datetime.strptime(datetime_, "%Y-%m-%d %H:%M:%S.%f").replace( | ||
| tzinfo=_parse_utc_offset(tz) or gettz(tz) | ||
| ) | ||
|
|
||
|
|
||
| def _to_time(varchar_value: str | None) -> time | None: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Round 2 deferral (docs reader):