Repository navigation
Conversation
RowIterator only removed startIndex from follow-up page requests when page_size was set. With only max_results and start_index, the server can still split the result into several pages, and the next request then sent both pageToken and startIndex, which the API rejects with 400 'When using a page token, you cannot specify an arbitrary startIndex'. Fixes googleapis#18601
abhishekacharya200
requested review from
tswast
and removed request for
a team
October 8, 2026 09:57
1 task done
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the logic for removing startIndex from query parameters during multi-page results in BigQuery. Instead of checking for page_size, it now removes startIndex when a page token is present, preventing conflicts when the server splits large results. A unit test was also added to verify this behavior. Feedback suggests improving readability by checking self.next_token is not None instead of checking if self._next_token is in params, as _next_token is a class attribute representing the parameter name string.
parthea
approved these changes
Oct 8, 2026
ohmayr
approved these changes
Oct 8, 2026
Contributor
|
Thanks @abhishekacharya200! |
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.
Fixes #18601
RowIterator._get_next_page_responseonly removedstartIndexfrom follow-up page requests whenpage_sizewas set. With onlymax_resultsandstart_index, the server can still split the result into several pages (for example when a page would exceed the response size limit). The next request then sent bothpageTokenandstartIndex, which the API rejects:This drops
startIndexwhenever a page token is sent, regardless ofpage_size. The first request still carriesstartIndex, and the original params are not modified, so iterating the results again keeps working as before.Added
test_result_with_start_index_and_max_results_multi_page, which fails onmainwith the samepageToken+startIndexrequest from the issue and passes with this change. The fulltests/unitsuite passes locally (2396 passed), andblack==23.7.0reports no changes.