Skip to content

chore(spanner): directly execute queries and wait for first result#5698

Open
olavloite wants to merge 5 commits into
googleapis:mainfrom
olavloite:spanner-direct-execute-query
Open

chore(spanner): directly execute queries and wait for first result#5698
olavloite wants to merge 5 commits into
googleapis:mainfrom
olavloite:spanner-direct-execute-query

Conversation

@olavloite
Copy link
Copy Markdown
Contributor

This changes the way that queries are executed on Spanner from a model that uses a background task to read from the query stream, to a model that directly calls ExecuteStreamingSql when execute_query is called, and then waits for the first PartialResultSet or error to be received. This ensures that execute_query really executes the query and returns any errors that happens during query execution.

Closes #5685
Fixes #5673

This changes the way that queries are executed on Spanner from a model that
uses a background task to read from the query stream, to a model that directly
calls ExecuteStreamingSql when execute_query is called, and then waits for the
first PartialResultSet or error to be received. This ensures that execute_query
really executes the query and returns any errors that happens during query
execution.

Closes googleapis#5685
Fixes googleapis#5673
@olavloite olavloite requested a review from sakthivelmanii May 20, 2026 09:31
@olavloite olavloite requested review from a team as code owners May 20, 2026 09:31
@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label May 20, 2026
Copy link
Copy Markdown

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

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 refactors the ResultSet implementation to handle metadata initialization more robustly, replacing the background worker pattern with an explicit init_stream method. It also introduces ExplicitBeginParams to streamline transaction initialization and adds rustls as a dependency for secure connections. The reviewer suggests explicitly handling the Result from the rustls provider installation and refactoring redundant metadata logic in ResultSet to improve maintainability.

Comment thread tests/spanner/src/client.rs Outdated
Comment thread src/spanner/src/result_set.rs Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 96.22642% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.88%. Comparing base (23f3c97) to head (e45381f).

Files with missing lines Patch % Lines
src/spanner/src/result_set.rs 95.26% 8 Missing ⚠️
src/spanner/src/read_write_transaction.rs 91.30% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5698      +/-   ##
==========================================
- Coverage   97.89%   97.88%   -0.02%     
==========================================
  Files         226      226              
  Lines       55536    55552      +16     
==========================================
+ Hits        54368    54378      +10     
- Misses       1168     1174       +6     

☔ View full report in Codecov by Sentry.
📢 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.

Comment thread tests/spanner/Cargo.toml Outdated
@olavloite olavloite force-pushed the spanner-direct-execute-query branch from 9d3aad9 to 5b2d174 Compare May 21, 2026 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the Spanner API.

Projects

None yet

2 participants