fix(bqjdbc): pass rowsInPage with TableResult#13238
Open
Neenu1995 wants to merge 4 commits into
Open
Conversation
Contributor
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.