fix(spanner): process metadata and stats for PartialResultSets with empty values#428
Open
apstndb wants to merge 1 commit intoyoshidan:mainfrom
Open
fix(spanner): process metadata and stats for PartialResultSets with empty values#428apstndb wants to merge 1 commit intoyoshidan:mainfrom
apstndb wants to merge 1 commit intoyoshidan:mainfrom
Conversation
…mpty values Previously, try_recv() returned early when values were empty, skipping metadata, resume_token, and stats processing. This caused: 1. QueryMode::Plan always returned empty columns_metadata() and lost query_plan, because Plan mode responses contain only metadata and stats with no values. 2. QueryMode::Profile with 0 result rows lost query_stats. 3. Normal queries returning 0 rows also lost column metadata. Move the empty-values check after resume_token/stats processing and call self.rs.add() to ensure metadata is always captured. Add three integration tests: - test_query_plan_mode_metadata - test_query_empty_result_metadata - test_query_profile_mode_empty_result_stats Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5a46899 to
9871a10
Compare
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.
Problem
RowIterator::try_recv()returns early whenresult_set.values.is_empty(), skipping all subsequent processing — metadata, resume_token, and stats are silently discarded.For reference, the Go client's
RowIteratordocuments the expected availability of these fields:The Rust
RowIteratorprovides equivalent accessors —columns_metadata(),stats()— but they were broken for empty result sets becausetry_recv()discarded the underlying data before it could be stored.Affected scenarios
When values are empty (either because the mode returns no rows, or because the query matches 0 rows), all information in the
PartialResultSetwas discarded. The following table shows what each mode is expected to return and what was lost:Plancolumns_metadata(),stats().query_planProfilecolumns_metadata(),stats().query_plan,stats().query_statsWithPlanAndStatscolumns_metadata(),stats().query_plan,stats().query_statsWithStatscolumns_metadata(),stats().query_statsNormalcolumns_metadata()Fix
Move the
values.is_empty()check after resume_token and stats processing, and callself.rs.add()before returningOk(false)so that metadata is always captured.Tests
Added three integration tests in
spanner/tests/transaction_ro_test.rs:test_query_plan_mode_metadata—QueryMode::Planpopulatescolumns_metadata()despite returning no data rows. On real Spanner, Plan mode also returnsstats()withquery_plan(plan_nodes), but the emulator does not, soquery_planis not asserted here.test_query_empty_result_metadata—QueryMode::NormalwithWHERE FALSEstill populatescolumns_metadata().test_query_profile_mode_empty_result_stats—QueryMode::Profilewith 0 rows populates bothcolumns_metadata()andstats()(query_stats).All three tests fail without the fix and pass with it. Existing tests pass with no regressions. Additionally, verified against a real Spanner instance that
QueryMode::Planreturnsstats().query_planwith populatedplan_nodes.