feat(bigquery): accelerate row-based query() with Arrow wire format - #14355
feat(bigquery): accelerate row-based query() with Arrow wire format#14355jinseopkim0 wants to merge 22 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for the Arrow query results format in the fast query path execution of BigQuery. It introduces ArrowQueryPageFetcher to handle paginated Arrow results, updates BigQueryImpl to deserialize Arrow schemas and record batches, and adds validation to restrict Arrow format usage to fast-path queries. The review feedback highlights a potential bug where declaring firstPageRows as a plain Iterable and using instanceof Collection checks could lead to duplicate rows or incorrect metadata if the deserialized rows do not implement Collection. The reviewer suggests declaring firstPageRows as a Collection and eagerly loading it, which also simplifies several downstream instanceof checks.
…oot.create in ArrowQueryResultImpl
99a1d1f to
88790fa
Compare
88790fa to
2316e7b
Compare
…onal client initialization in Arrow query
2316e7b to
2ae65ec
Compare
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for the Arrow query results format in BigQueryImpl by introducing ArrowQueryPageFetcher to retrieve pages using the BigQuery Storage API and updating fast path query execution. The review feedback focuses on optimizing resource management within ArrowQueryPageFetcher by sharing the BigQueryReadClient from BigQueryImpl instead of instantiating new clients, cancelling active server streams on close, and adding defensive checks to prevent null location errors during stream name formatting.
…check location in ArrowQueryPageFetcher
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for the Arrow results format during fast query path execution in BigQueryImpl. It introduces ArrowQueryPageFetcher to handle paginated Arrow results, deserializes Arrow schemas and record batches, and updates query validation to allow Arrow format only when compatible configurations are used. Feedback on these changes includes addressing a hardcoded page size in ArrowQueryPageFetcher to prevent potential memory issues, removing a redundant type cast when deserializing the Arrow schema, and avoiding the use of a fully qualified class name for BigInteger in the test suite.
…t in ArrowQueryPageFetcher
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request enables support for the Arrow results format in the fast query path of BigQueryImpl.query(). It introduces ArrowQueryPageFetcher to fetch paginated Arrow rows from storage streams and updates query response handling to deserialize Arrow schemas and record batches. Feedback points out a potential resource leak where a newly created BigQueryReadClient is not closed if the user does not fully exhaust the page iterator.
…revent gRPC client leak
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for Arrow query results in BigQueryImpl via a fast query path execution, adding the ArrowQueryPageFetcher class to handle pagination of Arrow rows and updating query execution logic to deserialize Arrow schemas and record batches. The review feedback highlights two main improvement opportunities: preventing a potential ClassCastException by casting the MAX_RESULTS option to Number instead of Long, and simplifying the cleanup logic by removing the redundant isSharedClient field.
…d redundant field
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for Arrow query results in the fast query path of BigQuery, introducing ArrowQueryPageFetcher for paginating Arrow results and decoding Arrow schemas and record batches. Feedback suggests optimizing the rowBatch list initialization by setting its initial capacity to pageSize to avoid resizing overhead, and changing the return type of transformTableData from Collection to List for better type precision.
…transformTableData return type
|
@gemini-code-assist review |
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for Arrow query results in the fast query path execution of BigQuery. It adds ArrowQueryPageFetcher to handle pagination of Arrow query results via gRPC streams, updates schema and record batch deserialization, and restricts Arrow results to fast path queries. The review feedback highlights a potential gRPC stream leak due to the lack of AutoCloseable implementation on TableResult or Page, suggests an optimization to prevent unnecessary extra page fetches when maxResults is reached, and recommends adding a unit test to cover Arrow pagination.
…am lifecycle, and add pagination test
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for the Arrow query results format in the fast query path execution. It introduces the ArrowQueryPageFetcher class to handle paginated results from the gRPC Storage Read API stream, decodes Arrow schemas and record batches, and updates the query execution logic to support this format while restricting it to fast query paths. Unit tests are also added to verify these behaviors. Feedback points out that passing null as maxResults to ArrowQueryPageFetcher will cause the client to ignore the query's maxResults configuration, and suggests passing configuration.getMaxResults() instead to ensure the limit is respected.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for reading query results in Arrow format via a new ArrowQueryPageFetcher that streams data from the default gRPC storage read stream, along with corresponding unit tests. Feedback on these changes highlights a potential gRPC client resource leak when ArrowQueryPageFetcher is serialized and deserialized, as well as an OutOfMemoryError risk if the initial capacity of the row batch list is allocated using an uncapped pageSize.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for queries returning results in Arrow format, introducing the ArrowQueryPageFetcher to fetch subsequent pages from the gRPC storage read stream and updating BigQueryImpl to handle Arrow schemas and record batches. Feedback focuses on a critical bug where the deserialized ArrowQueryPageFetcher incorrectly marks itself as owning the BigQueryReadClient (ownsClient = true), leading to the premature closure of the shared client instance and breaking subsequent operations. It is recommended to remove this ownership logic and update the corresponding unit tests.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for retrieving query results in Arrow format via the fast query path, introducing ArrowQueryPageFetcher to handle paginated streaming from the BigQuery Storage Read API. Review feedback suggests avoiding double-wrapping BigQueryException within the page fetcher's error handling, and defensively checking for null on results.getRows() to prevent potential NullPointerExceptions.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for queries returning results in Arrow format, implementing the ArrowQueryPageFetcher to handle paginated Arrow results and updating BigQueryImpl to deserialize Arrow schemas and record batches. It also adds comprehensive unit tests to cover various Arrow query scenarios. A critical issue was identified in the pagination logic where initialRowOffset is calculated incorrectly when a page token is present, which could lead to duplicate or skipped rows; parsing the page token as a long is suggested to resolve this.
…ion at maxResults
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for Arrow-formatted query results in BigQueryImpl, including schema deserialization, record batch processing, and pagination via a new ArrowQueryPageFetcher. It also adds comprehensive unit tests to verify these changes. The reviewer suggests optimizing the parsing of the page token in BigQueryImpl to avoid redundant parsing operations.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for executing queries that return results in Arrow format via the fast query path. It adds the ArrowQueryPageFetcher class to handle paginated Arrow results using the BigQuery Storage Read API, and updates BigQueryImpl to deserialize Arrow schemas and record batches. It also includes comprehensive unit tests to verify fast-path execution, multi-page streaming, and serialization. Feedback was provided to avoid using exception handling for normal control flow when parsing page tokens in BigQueryImpl.java.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for queries returning results in the Arrow format via the fast query path. It implements ArrowQueryPageFetcher to fetch subsequent pages from the default gRPC storage read stream and updates query response processing to handle Arrow schemas and record batches. Comprehensive unit tests have been added to verify the fast path, multi-page streaming, and serialization of the page fetcher. The review feedback highlights a potential issue with the numeric validation of the page token, where Character.isDigit might match non-ASCII Unicode digits that fail to parse with Long.parseLong, and suggests a more robust try-catch parsing approach.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for the Arrow query results format during fast query path execution in BigQueryImpl. It introduces ArrowQueryPageFetcher to handle paginated results from the gRPC storage read stream, deserializes Arrow schemas and record batches, and updates the query execution path to allow Arrow format when appropriate. Comprehensive unit tests have been added to verify fast-path execution, multi-page streaming, and serialization. There are no review comments, and I have no additional feedback to provide.
Enables Apache Arrow wire acceleration for the traditional
BigQuery.query()API returning row-basedTableResult. WhenQueryResultsFormat.ARROWis configured on fast-path queries, result batches are fetched over the wire in Arrow format and decoded intoFieldValueListrows, backed byArrowQueryPageFetcherfor page iteration.