feat(bigquery): integrate Arrow query response processing and stream pagination - #13944
feat(bigquery): integrate Arrow query response processing and stream pagination#13944jinseopkim0 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for Arrow-formatted query results in the BigQuery client, adding ArrowQueryPageFetcher to fetch Arrow pages and updating response parsing to handle Arrow schemas and record batches. The review feedback highlights critical issues that need to be addressed: a potential NotSerializableException in ArrowQueryPageFetcher due to a non-serializable schema field, memory leaks from unclosed Arrow vectors and record batches, and overly restrictive type checks (instanceof List instead of instanceof Collection) when determining page row counts.
2f4435e to
76600f0
Compare
76600f0 to
ec26676
Compare
ec26676 to
68798eb
Compare
bb63672 to
88530f7
Compare
88530f7 to
80ae0af
Compare
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Apache Arrow query results format in the BigQuery client. It refactors Arrow schema conversion and vector creation logic into a new helper class ArrowPojoUtils, adds an ArrowQueryPageFetcher to stream Arrow-formatted query results, and updates BigQueryImpl and QueryRequestInfo to handle Arrow serialization options and deserialize Arrow record batches. The review feedback highlights several improvement opportunities: preventing a potential resource leak in ArrowPojoUtils.createVectors by closing already allocated vectors if an exception is thrown, avoiding a potential NullPointerException in BigQueryImpl by using serviceOptions.getCredentialsProvider(), and extracting duplicate logic for computing firstPageRows to improve maintainability.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Apache Arrow query results format in the BigQuery client. It extracts Arrow conversion utilities into a new ArrowPojoUtils helper class, implements ArrowQueryPageFetcher to stream results via the BigQuery Storage Read API, and updates query execution paths to handle Arrow schemas and record batches. The review feedback focuses on critical resource management and performance optimizations, specifically ensuring exception-safe LIFO cleanup of Arrow vectors, allocators, and schema roots to prevent memory leaks, reusing allocator and root instances across page fetches, enforcing the maxResults limit, and properly handling credentials and universe domains.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for retrieving query results in Apache Arrow format using the fast query path and the BigQuery Storage API. It adds a new ArrowQueryPageFetcher to handle paginated Arrow record batches, introduces ArrowPojoUtils for Arrow schema and vector utilities, and updates BigQueryImpl to deserialize Arrow schemas and record batches. The review feedback highlights a critical resource leak in ArrowQueryPageFetcher due to its stateful nature across pages, suggesting a stateless implementation using try-with-resources to ensure gRPC streams, clients, and off-heap memory allocators are safely closed.
8f1d641 to
101d2d0
Compare
|
@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 BigQuery, extracting Arrow conversion utilities into ArrowPojoUtils and implementing ArrowQueryPageFetcher in BigQueryImpl to read Arrow rows via the Storage Read API. The reviewer identifies a critical issue where direct imports and references to Apache Arrow classes in BigQueryImpl.java will cause NoClassDefFoundError at runtime for users who do not have Arrow on their classpath. To resolve this, the reviewer recommends encapsulating all Arrow-specific logic and deserialization within ArrowDeserializer and refactoring BigQueryImpl to avoid direct bytecode dependencies on optional Arrow classes.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Apache Arrow query results format in the BigQuery client, enabling faster query path execution. Key changes include the addition of ArrowPojoUtils for schema conversions, updates to ArrowDeserializer to handle row loading and deserialization, and the implementation of ArrowQueryPageFetcher in BigQueryImpl to fetch pages of Arrow-formatted results. The review feedback highlights three improvement opportunities: caching the BigQueryReadClient and stream iterator in transient fields within ArrowQueryPageFetcher to prevent significant gRPC overhead on pagination, removing an unused BufferAllocator in ArrowDeserializer.deserializeSchema, and restructuring vector resource management in loadArrowRows to avoid a potential double-closing issue.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for retrieving query results in Apache Arrow format from BigQuery. It adds the ArrowDeserializer and ArrowPojoUtils helper classes to convert Arrow schemas and deserialize record batches, implements ArrowQueryPageFetcher to fetch pages of Arrow rows, and integrates these changes into BigQueryImpl and QueryRequestInfo. Feedback on these changes highlights several critical issues: potential resource leaks if the stream iterator is not fully exhausted, a risk of generating invalid stream names if the job location is null, high memory consumption due to a hardcoded page size of 100,000 rows, performance overhead from repeatedly instantiating RootAllocator, and reduced type safety from using Object instead of concrete Arrow types in method signatures.
There was a problem hiding this comment.
Code Review
This pull request introduces support for retrieving query results in Apache Arrow format using the BigQuery Storage Read API, adding utility classes for Arrow deserialization and a new ArrowQueryPageFetcher for paginating results. The review feedback highlights critical issues regarding thread safety and resource management: first, ArrowQueryPageFetcher is stateful and mutates its row offset, which violates the immutability contract of the Page interface and should be refactored to be immutable; second, the lazily instantiated BigQueryReadClient in BigQueryImpl is never closed, potentially leaking gRPC channels and threads, and should be cleaned up by implementing AutoCloseable.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for the Apache Arrow query results format in the BigQuery client, including deserializing Arrow schemas and record batches, and fetching Arrow rows from storage streams. The reviewer feedback highlights a potential race condition in BigQueryImpl.close() where it can race with getBigQueryReadClient(), and suggests using explicit locks. Additionally, the reviewer points out duplicated code in ArrowDeserializer for creating VectorSchemaRoot and recommends extracting it into a shared helper method to improve maintainability.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Apache Arrow query results format in the BigQuery client, enabling faster query path execution by deserializing Arrow schemas and record batches directly. Key changes include the addition of ArrowDeserializer and ArrowPojoUtils, extending the BigQuery interface with AutoCloseable to manage BigQueryReadClient lifecycle, and implementing ArrowQueryPageFetcher for paginated Arrow results. The review feedback highlights several important improvements: avoiding checked exceptions on the new close() methods, fixing a potential bug where standard schema checks could override Arrow schema processing, lazily initializing the resource-intensive RootAllocator to prevent overhead on non-Arrow paths, and simplifying try-with-resources blocks in the deserializer.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for fetching query results in Apache Arrow format. It adds deserialization utilities (ArrowDeserializer and ArrowPojoUtils), integrates them into the query execution path in BigQueryImpl, and updates the BigQuery interface to extend AutoCloseable for proper cleanup of the newly introduced BigQueryReadClient. The reviewer feedback focuses on improving resource management and robustness, specifically recommending: using child allocators instead of a global static allocator for better memory tracking; ensuring AutoCloseable streams and channels are properly managed in try-with-resources blocks; strictly respecting the requested page size during row loading; and avoiding wrapping existing BigQueryException instances to preserve original error details.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for fetching query results in Apache Arrow format. It refactors Arrow deserialization logic, extracts helper methods into a new ArrowPojoUtils class, and implements an ArrowQueryPageFetcher in BigQueryImpl to read Arrow rows from storage streams using BigQueryReadClient. Feedback on the changes points out that adding a non-default close() method to the public BigQuery interface is a breaking change, and suggests making it a default method to preserve backward compatibility.
|
@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 BigQuery client. It refactors ArrowDeserializer and extracts schema conversion helpers into a new ArrowPojoUtils class, makes BigQuery extend AutoCloseable to manage the lifecycle of a BigQueryReadClient, and implements ArrowQueryPageFetcher to handle paginated retrieval of Arrow-formatted query results. The review feedback correctly identifies a critical bug in loadArrowRows where pagination can lead to silent data truncation if a response contains more rows than the page size and no subsequent responses exist in the stream. A code suggestion is provided to track unconsumed rows and resolve this issue.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request adds support for retrieving BigQuery query results in Apache Arrow format using the BigQuery Storage API. It introduces Arrow deserialization helpers, a new page fetcher for Arrow rows, and updates the query execution path to handle Arrow schemas and record batches. The review feedback correctly points out a compilation error in BigQueryImpl.java where HostAndPort.getHost() is called instead of HostAndPort.getHostText().
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for fetching query results in Apache Arrow format from BigQuery. It implements deserialization of Arrow schemas and record batches, adds a page fetcher (ArrowQueryPageFetcher) to stream Arrow rows using the BigQuery Storage Read API, and integrates these changes into the BigQueryImpl query execution path. Additionally, it refactors Arrow conversion utilities into a new ArrowPojoUtils class and updates relevant tests. I have no feedback to provide as there are no review comments.
21f9657 to
e64fffb
Compare
e64fffb to
6713822
Compare
6713822 to
e196675
Compare
Stacked PR 3 of 3: Integrates the Arrow API surface (#13942) and the ArrowDeserializer (#13943) into the Veneer client's query execution path, implementing fallback validations, first-page parsing, and stateless gRPC stream pagination.
b/540476814