feat(bigquery): accelerate row-based query() with Arrow wire format - #14380
jinseopkim0 wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Arrow query results format in the fast query path of BigQuery, including Arrow schema and record batch deserialization, pagination handling via ArrowQueryPageFetcher, and corresponding unit tests. The feedback suggests adding a defensive null check when decoding the Arrow schema to prevent a potential NullPointerException if the serialized schema is missing from the response.
be77a7d to
8cea5f6
Compare
8cea5f6 to
82693bf
Compare
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Arrow results format in BigQuery query execution, allowing the deserialization of Arrow schemas and record batches, as well as pagination via a new Arrow query page fetcher. The changes also include comprehensive unit tests covering fast-path execution, multi-page streaming, limit enforcement, and serialization. The review feedback suggests improving robustness by enforcing maxResults on the first page of results, ensuring hasMorePages is only true when the job is complete, and removing redundant defensive null checks on fields guaranteed to be non-null upon job completion.
… for hasMorePages
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Arrow results format in BigQuery query execution, including schema and record batch deserialization, pagination handling via ArrowQueryPageFetcher, and comprehensive unit tests. The review feedback highlights a potential NullPointerException when processing zero rows with a null Arrow record batch, and suggests removing an unnecessary fully qualified class name in the test suite to improve readability.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for the Arrow results format in the fast query path execution of BigQuery. It introduces deserialization of Arrow schemas and record batches, supports pagination for Arrow queries via a new page fetcher, and adds comprehensive unit tests to validate these changes. There are no review comments, so I have no feedback to provide.
…ssingSerializedSchema
|
@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 of BigQuery, including schema and record batch deserialization, pagination via ArrowQueryPageFetcher, and validation to restrict Arrow format to fast path execution. Comprehensive unit tests are added to cover these scenarios. Feedback on the changes highlights a potential NullPointerException when results.getRows() is null, suggesting a defensive check to use an empty list instead.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Arrow query results format in the fast query path execution within BigQueryImpl. It updates the query execution logic to deserialize Arrow schemas and record batches, handles pagination and maximum result limits for Arrow-formatted responses, and implements the ArrowQueryPageFetcher for retrieving subsequent pages. Additionally, several unit tests have been added to BigQueryImplTest to verify fast path execution, multi-page results, serialization, and error handling for missing schemas. There are no review comments provided, and I have no additional feedback on these changes.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for Arrow-formatted query results in the query method of BigQueryImpl, implementing a private queryRpcArrow helper to deserialize Arrow schemas and record batches, handle pagination, and construct TableResult objects. It also includes comprehensive unit tests in BigQueryImplTest covering various scenarios such as fast-path execution, multi-page results, and error handling. The review feedback highlights two potential NullPointerException issues where results.getJobReference() is accessed without a null check when the job is incomplete or when fetching subsequent pages.
|
@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 query method of BigQueryImpl, introducing the queryRpcArrow helper to deserialize Arrow schemas and record batches, handle pagination, and manage multi-page results. Comprehensive unit tests have been added to validate these changes. The review feedback suggests simplifying an anonymous Callable class into a lambda expression to reduce boilerplate, and removing a redundant results.getJobComplete() check since it is already validated earlier in the execution flow.
|
@gemini-code-assist review |
|
@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 BigQuery fast query path. It introduces the queryRpcArrow method in BigQueryImpl to handle Arrow-serialized schemas and record batches, manage pagination, and build TableResult objects. It also updates the main query method to route Arrow-formatted queries through this new path, throwing an exception if an unsupported configuration is used. Comprehensive unit tests have been added to verify various scenarios, including single-page and multi-page results, serialization, and error handling. There are no review comments to address, and I have no additional feedback to provide.
There was a problem hiding this comment.
Code Review
This pull request adds support for Arrow-formatted query results in BigQueryImpl by introducing the queryRpcArrow method to handle fast-path query execution, deserialization of Arrow schemas and record batches, and pagination via ArrowQueryPageFetcher. It also updates the query method to support this format and adds extensive unit tests in BigQueryImplTest. The feedback suggests using List<FieldValueList> instead of Collection<FieldValueList> for firstPageRows to be more specific and idiomatic, which would also allow removing an unused import.
| results.getSessionInfo() != null ? SessionInfo.fromPb(results.getSessionInfo()) : null; | ||
|
|
||
| // Deserialize first page of rows from the Arrow record batch (if present). | ||
| Collection<FieldValueList> firstPageRows; |
There was a problem hiding this comment.
Using List<FieldValueList> is more specific and idiomatic than Collection<FieldValueList> here, as the rows represent an ordered sequence. This also allows you to remove the unused java.util.Collection import on line 80.
| Collection<FieldValueList> firstPageRows; | |
| List<FieldValueList> firstPageRows; |
Enables Apache Arrow wire acceleration for the traditional
BigQuery.query()API returning row-basedTableResult.Part of the BigQuery Apache Arrow support stack. Based on #14378 (page fetcher).