fix(bqjdbc): pass rowsInPage with TableResult#13238
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a rowsInPage field to the TableResult class, allowing for more accurate tracking of the number of rows in a specific results page. This field is integrated into the JDBC driver's read ratio calculation and various TableResult construction points. Feedback includes a critical fix for the equals method to use Objects.equals for string comparison of queryId and a suggestion to ensure rowsInPage is populated during pagination for consistency.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| .setSchema(schema) | ||
| .setTotalRows(data.y()) | ||
| .setPageNoSchema(data.x()) | ||
| .setRowsInPage(data.x() != null ? (long) Iterables.size(data.x().getValues()) : 0L) |
There was a problem hiding this comment.
Can we guarantee somehow that it would be using .size() to avoid O(N) here?
There was a problem hiding this comment.
We can guarantee
But if we want we can add an explicit instanceof check here. Should I add it?
b/511229401
This PR introduces the
rowsInPagemetadata toTableResultand ensures it is populated for all pages (both the first page and subsequent pages fetched via pagination).Previously, metadata about the number of rows in the page was not easily accessible or propagated across multiple paginated requests (e.g., when calling
getNextPage()). This change:rowsInPagefield and builder method to the@AutoValueTableResultclass.rowsInPageduring initial page creation inBigQueryImpland empty results inJob.java.TableResult.getNextPage()to calculate the size of values for subsequent pages viaIterables.size(which isTableResultpages.TableResultTest.javaandSerializationTest.javato verify thatrowsInPageis populated and propagated correctly.Key Changes
TableResult.java: AddedgetRowsInPage(), builder methodsetRowsInPage(), and updatedgetNextPage()to pass down the calculated row count for the next page.BigQueryImpl.java: SetrowsInPageon initial query results page creation.TableResultTest.java: Added assertions to verify thatrowsInPageis correctly populated for both the first page and subsequent pages.