feat(bigquery): add Storage Read API slow-path fallback for row-based query() - #14389
jinseopkim0 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a fallback mechanism (queryFallbackArrow) to support the Arrow query results format for slow-path query executions by executing the query job and streaming the results via the BigQuery Storage Read API. The feedback recommends passing the resolved destinationTable instead of null to TableDataPageFetcher to prevent potential NullPointerExceptions, and optimizing the calculation of rowsInPage to avoid inefficiently copying the entire first page of results into an ImmutableList just to retrieve its size.
| .setTotalRows(numDmlAffectedRows != null ? numDmlAffectedRows : 0L) | ||
| .setPageNoSchema( | ||
| new PageImpl<>( | ||
| new TableDataPageFetcher(null, schema, getOptions(), null, optionMap(options)), |
There was a problem hiding this comment.
Instead of passing null as the first argument (TableId) to TableDataPageFetcher, pass destinationTable. Since destinationTable is guaranteed to be non-null at this point, passing it ensures that the fetcher is correctly initialized and avoids potential NullPointerExceptions if getNextPage() is ever invoked.
| new TableDataPageFetcher(null, schema, getOptions(), null, optionMap(options)), | |
| new TableDataPageFetcher(destinationTable, schema, getOptions(), null, optionMap(options)), |
| List<FieldValueList> firstPageRows = | ||
| firstPage != null ? ImmutableList.copyOf(firstPage.getValues()) : ImmutableList.of(); | ||
| long rowsInPage = (long) firstPageRows.size(); |
There was a problem hiding this comment.
Copying the entire first page of results into a new ImmutableList via ImmutableList.copyOf(firstPage.getValues()) just to get its size is inefficient, especially for large page sizes (e.g., up to 10,000 rows). Instead, we can check if the values are already a Collection to get the size in Iterables.size() to avoid copying.
long rowsInPage = 0;
if (firstPage != null) {
Iterable<FieldValueList> values = firstPage.getValues();
rowsInPage = values instanceof java.util.Collection
? ((java.util.Collection<?>) values).size()
: com.google.common.collect.Iterables.size(values);
}| firstPage != null | ||
| ? firstPage | ||
| : new PageImpl<>( | ||
| new TableDataPageFetcher(null, schema, getOptions(), null, optionMap(options)), |
There was a problem hiding this comment.
Instead of passing null as the first argument (TableId) to TableDataPageFetcher, pass destinationTable. Since destinationTable is guaranteed to be non-null at this point, passing it ensures that the fetcher is correctly initialized and avoids potential NullPointerExceptions if getNextPage() is ever invoked.
| new TableDataPageFetcher(null, schema, getOptions(), null, optionMap(options)), | |
| new TableDataPageFetcher(destinationTable, schema, getOptions(), null, optionMap(options)), |
This PR implements slow-path execution fallback for row-based queries requesting Arrow results format (
QueryResultsFormat.ARROW). When queries cannot be evaluated via the fast query path (such as queries writing to destination tables or exceeding fast-path limits), BigQuery job execution is triggered and table results are streamed via the BigQuery Storage Read API in Arrow format.Follow-up PR stacked on top of #14380.