Repository navigation
fix(bigquery): send the same Arrow options on jobs.query and ReadRows #18646
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
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 | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,7 +17,7 @@ | |||||||||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import base64 | ||||||||||||||||||||||||||||||||||
| from typing import Iterator, Optional, TYPE_CHECKING | ||||||||||||||||||||||||||||||||||
| from typing import Dict, Iterator, Optional, TYPE_CHECKING | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from google.cloud.bigquery import _versions_helpers | ||||||||||||||||||||||||||||||||||
| from google.cloud.bigquery import enums | ||||||||||||||||||||||||||||||||||
|
|
@@ -84,6 +84,30 @@ def _check_dependencies() -> None: | |||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def serialization_options(compression_codec: Optional[str]) -> Dict[str, str]: | ||||||||||||||||||||||||||||||||||
| """Build the ``arrowSerializationOptions`` sent on ``jobs.query``. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ``ReadRows`` gets the same options. Options the caller didn't set are sent | ||||||||||||||||||||||||||||||||||
| with their defaults, so every batch uses one encoding, whatever each | ||||||||||||||||||||||||||||||||||
| endpoint would default to. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||
| "bufferCompression": getattr(compression_codec, "value", compression_codec) | ||||||||||||||||||||||||||||||||||
| or "COMPRESSION_UNSPECIFIED", | ||||||||||||||||||||||||||||||||||
| "picosTimestampPrecision": "TIMESTAMP_PRECISION_MICROS", | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def _read_rows_options(options: Optional[Dict[str, str]]) -> Dict[str, str]: | ||||||||||||||||||||||||||||||||||
| """Convert ``jobs.query`` ``arrowSerializationOptions`` to the | ||||||||||||||||||||||||||||||||||
| ``ArrowSerializationOptions`` fields that ``ReadRows`` takes.""" | ||||||||||||||||||||||||||||||||||
| options = options or serialization_options(None) | ||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||
| "buffer_compression": options["bufferCompression"], | ||||||||||||||||||||||||||||||||||
| "picos_timestamp_precision": options["picosTimestampPrecision"], | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+101
to
+108
Contributor
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. Using direct dictionary lookups like
Suggested change
Contributor
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. These options always come from serialization_options(), which sets both keys, so a missing key would be a bug I'd want to see as a KeyError rather than paper over with defaults. RowIterator's constructor is internal to query_and_wait() here. |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def download_arrow_from_job( | ||||||||||||||||||||||||||||||||||
| row_iterator: "RowIterator", | ||||||||||||||||||||||||||||||||||
| bqstorage_client: Optional["bigquery_storage.BigQueryReadClient"] = None, | ||||||||||||||||||||||||||||||||||
|
|
@@ -153,7 +177,14 @@ def download_arrow_from_job( | |||||||||||||||||||||||||||||||||
| row_iterator.client.location if row_iterator.client else None | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| stream_name = f"projects/{project}/locations/{location}/jobs/{row_iterator._job_id}/streams/_default" | ||||||||||||||||||||||||||||||||||
| reader = bqstorage_client.read_rows(stream_name, offset=offset, timeout=timeout) | ||||||||||||||||||||||||||||||||||
| reader = bqstorage_client.read_rows( | ||||||||||||||||||||||||||||||||||
| stream_name, | ||||||||||||||||||||||||||||||||||
| offset=offset, | ||||||||||||||||||||||||||||||||||
| timeout=timeout, | ||||||||||||||||||||||||||||||||||
| arrow_serialization_options=_read_rows_options( | ||||||||||||||||||||||||||||||||||
| row_iterator._arrow_serialization_options | ||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| for response in reader: | ||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||
| response.arrow_schema | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -1910,6 +1910,9 @@ class RowIterator(HTTPIterator): | |||||||
| If representing query results, the start time of the associated query. | ||||||||
| ended (Optional[datetime.datetime]): | ||||||||
| If representing query results, the end time of the associated query. | ||||||||
| arrow_serialization_options (Optional[Dict[str, str]]): | ||||||||
| The ``arrowSerializationOptions`` sent on ``jobs.query`` for | ||||||||
| ``ARROW`` results. ``ReadRows`` requests the same options. | ||||||||
| """ | ||||||||
|
|
||||||||
| def __init__( | ||||||||
|
|
@@ -1938,6 +1941,7 @@ def __init__( | |||||||
| started: Optional[datetime.datetime] = None, | ||||||||
| ended: Optional[datetime.datetime] = None, | ||||||||
| query_results_format: Optional[str] = None, | ||||||||
| arrow_serialization_options: Optional[Dict[str, str]] = None, | ||||||||
|
Contributor
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. nit: It would be nice to force this to be a kwarg too. Optional, since the RowIterator constructor isn't expected to be used directly by customers.
Suggested change
|
||||||||
| ): | ||||||||
| super(RowIterator, self).__init__( | ||||||||
| client, | ||||||||
|
|
@@ -1972,6 +1976,7 @@ def __init__( | |||||||
| self._job_started = started | ||||||||
| self._job_ended = ended | ||||||||
| self._query_results_format = query_results_format | ||||||||
| self._arrow_serialization_options = arrow_serialization_options | ||||||||
|
|
||||||||
| @property | ||||||||
| @_disallow_when_arrow | ||||||||
|
|
||||||||
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.
I assume it'll be a separate project to plumb through all of the TIMESTAMP_PRECISION_PICOS features at some point in the future?