Skip to content

impl(bigquery): fetch more pages and limit buffered results#6032

Open
alvarowolfx wants to merge 2 commits into
googleapis:mainfrom
alvarowolfx:impl-bq-page-fetch
Open

impl(bigquery): fetch more pages and limit buffered results#6032
alvarowolfx wants to merge 2 commits into
googleapis:mainfrom
alvarowolfx:impl-bq-page-fetch

Conversation

@alvarowolfx

Copy link
Copy Markdown
Contributor

Towards #5844

@product-auto-label product-auto-label Bot added the api: bigquery Issues related to the BigQuery API. label Jul 10, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements pagination support for the BigQuery RowIterator by implementing the fetch_page method using GetQueryResultsRequest, adding a set_max_rows_buffered configuration method, and adding corresponding unit and integration tests. The review feedback suggests avoiding the use of .expect() in fetch_page and instead returning a Result to prevent potential panics, aligning with the repository's style guide.

Comment on lines +84 to +86
let job_ref = self.job_ref.as_ref().expect(
"only queries with a job reference should have page tokens and can fetch more pages",
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to the Repository Style Guide (lines 50-59), unwrap() and expect() should typically be avoided in production code, and the library should almost always prefer returning a Result over panicking. Since fetch_page returns a Result, we can safely return a RowError::InvalidRowFormat if job_ref is missing instead of panicking.

Suggested change
let job_ref = self.job_ref.as_ref().expect(
"only queries with a job reference should have page tokens and can fetch more pages",
);
let job_ref = self.job_ref.as_ref().ok_or_else(|| {
RowError::InvalidRowFormat("missing job reference for pagination".to_string())
})?;
References
  1. unwrap() and expect() should typically be avoided in production code and examples (use ? or handle errors). The library should almost always prefer to return a Result<> over an internal panic!. (link)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an unreachable state. Only happens if the backend runs a query as a stateless job (no job reference) and provide a page token (which is not possible)

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.98%. Comparing base (0ea7ea9) to head (511c27f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6032      +/-   ##
==========================================
+ Coverage   96.95%   96.98%   +0.02%     
==========================================
  Files         248      248              
  Lines       62379    62500     +121     
==========================================
+ Hits        60482    60616     +134     
+ Misses       1897     1884      -13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alvarowolfx alvarowolfx marked this pull request as ready for review July 10, 2026 20:58
@alvarowolfx alvarowolfx requested a review from a team as a code owner July 10, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the BigQuery API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant