feat(spanner): implement FromValue for serde_json::Value with dynamic STRUCT/ARRAY deserialization by buu-nguyen · Pull Request #5863 · googleapis/google-cloud-rust

buu-nguyen · 2026-06-08T19:01:10Z

Summary

Implements FromValue for serde_json::Value to enable dynamic deserialization of all Spanner column types — including STRUCT, ARRAY, and nested ARRAY<STRUCT<ARRAY<...>>> — without requiring predefined Rust structs.

This unblocks:

Change stream TVF queries (deeply nested struct results)
Partition reads on tables with STRUCT/ARRAY columns
Any query returning structured data in a schema-agnostic pipeline

Usage:

let value: serde_json::Value = row.try_get("any_struct_or_array_column")?;
// Works for ANY schema — uses runtime Type metadata from result set

Resolves #5860

gemini-code-assist

Code Review

This pull request implements the FromValue trait for serde_json::Value (JsonValue), enabling dynamic deserialization of Spanner values (including nested arrays and structs) into JSON using runtime type metadata. It also adds helper methods to the Type struct and introduces comprehensive unit tests. The review feedback suggests refactoring several if/else blocks to reduce code duplication and avoid unnecessary cloning of field names, as well as extracting repeated array conversion logic into a helper function to adhere to the DRY principle.

… STRUCT/ARRAY deserialization Any query returning STRUCT or ARRAY<STRUCT> columns (partition queries, change stream TVFs, INFORMATION_SCHEMA) could not be deserialized without a predefined Rust type. The existing FromValue trait only supported compile-time-known types, making dynamic data extraction (e.g. RisingWave reading arbitrary Spanner tables) impossible without manual Value tree-walking. Added impl FromValue for serde_json::Value that recursively converts any Spanner Value into serde_json::Value using runtime Type metadata. Key mechanisms: - Positional ListValue + StructType metadata → named JSON object (the core reconstruction). Missing positional fields insert null rather than being silently omitted. - Named StructValue → JSON object via Struct::from_ref. Missing metadata-declared fields also insert null for consistency. - TypeCode::Json columns → parsed from wire StringValue into structured JSON. - NaN/Infinity → JsonValue::Null (matches SDK into_serde_value convention). Callers cannot distinguish these from genuine SQL NULLs. - Duplicate field names (e.g. unnamed SELECT expressions) use last-write-wins via serde_json::Map::insert. - Recursion depth limit (MAX_RECURSION_DEPTH = 64) prevents stack overflow on pathological inputs. - Type::struct_type() accessor for zero-cost metadata access. - Type::from_ref() zero-cost cast (matches Value::from_ref pattern) eliminating per-field Type clones during recursive deserialization.

olavloite · 2026-06-09T16:35:54Z

/gcbrun

olavloite · 2026-06-09T16:43:16Z

/gemini review

gemini-code-assist

Code Review

This pull request implements the FromValue trait for serde_json::Value to enable dynamic deserialization of Spanner values (including STRUCT, ARRAY, and JSON columns) using runtime type metadata, accompanied by comprehensive unit tests. The review feedback suggests key efficiency and maintainability improvements in the recursive conversion logic, such as avoiding redundant allocations of Type::default(), refactoring the signature to use Option<&Type>, and replacing k.to_string() with k.clone() to avoid unnecessary formatting overhead.

codecov · 2026-06-09T16:46:30Z

Codecov Report

❌ Patch coverage is 98.70968% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.91%. Comparing base (fa3df1b) to head (c5c5576).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/spanner/src/from_value.rs	98.69%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5863      +/-   ##
==========================================
+ Coverage   97.89%   97.91%   +0.01%     
==========================================
  Files         226      226              
  Lines       57808    58581     +773     
==========================================
+ Hits        56592    57358     +766     
- Misses       1216     1223       +7

☔ 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.

olavloite · 2026-06-10T09:11:58Z

@buu-nguyen I opened thealtoclef#2 to address the formatting issues + some other minor concerns that I had with this implementation. Would you mind taking a look at that and merge that into your pull request?

- Fixes the formatting errors in the PR - Adds more tests, including an integration test - Splits the method into multiple smaller steps for better readability - Fixes an issue with NaN and Inf values for float64 and float32

buu-nguyen · 2026-06-10T16:35:28Z

@buu-nguyen I opened thealtoclef#2 to address the formatting issues + some other minor concerns that I had with this implementation. Would you mind taking a look at that and merge that into your pull request?

It's merged. Can you re-run the test suite? Thanks

olavloite · 2026-06-10T17:08:23Z

/gcbrun

olavloite · 2026-06-10T17:08:46Z

/gemini review

gemini-code-assist

Code Review

This pull request implements dynamic deserialization of Spanner values into serde_json::Value using runtime type metadata, along with comprehensive unit and integration tests. The code review feedback focuses on aligning the implementation with the repository's style guide. Specifically, it recommends using the // SAFETY: prefix (in all caps) for the unsafe block in types.rs, simplifying a match arm in from_value.rs using Option::map and unwrap_or, and applying the 'Return Early' principle via let else statements to reduce nesting and eliminate unnecessary else blocks.

olavloite · 2026-06-10T18:19:02Z

@buu-nguyen I forgot about these warnings in my changes:

error: approximate value of `f64::consts::PI` found
--
  | --> src/spanner/src/from_value.rs:832:17
  | \|
  | 832 \|         let v = 3.14f64.to_value();
  | \|                 ^^^^^^^
  | \|
  | = help: consider using the constant directly
  | = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#approx_constant
  | = note: `#[deny(clippy::approx_constant)]` on by default
  |  
  | error: approximate value of `f{32, 64}::consts::PI` found
  | --> src/spanner/src/from_value.rs:834:41
  | \|
  | 834 \|         assert_eq!(j, serde_json::json!(3.14));
  | \|                                         ^^^^
  | \|
  | = help: consider using the constant directly
  | = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.96.0/index.html#approx_constant
  |  
  | Checking google-cloud-artifactregistry-v1 v1.10.0 (/workspace/src/generated/devtools/artifactregistry/v1)
  | error: could not compile `google-cloud-spanner` (lib test) due to 2 previous errors
  | warning: build failed, waiting for other jobs to finish...

Would you mind taking a look at those? (Can you view the build log?) And also take a look at the last comments from gemini?

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…let and else block, making the code more concise and idiomatic

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…stant lint The Float64-to-JSON test used 3.14f64, which clippy flags as an approximation of f64::consts::PI when running with --tests and -D clippy::approx_constant. This lint is enforced in CI. Changed the test value to 1.5, which is not an approximation of any well-known mathematical constant.

buu-nguyen · 2026-06-11T05:47:39Z

@olavloite I did apply suggestions from Gemini. Also updated the test to resolve the error: approximate value of `f64::consts::PI` found as well.

olavloite · 2026-06-11T05:49:43Z

/gcbrun

olavloite · 2026-06-11T05:49:49Z

/gemini review

gemini-code-assist

Code Review

This pull request implements the FromValue trait for serde_json::Value, enabling dynamic deserialization of Spanner values (including nested arrays, structs, and JSON columns) using runtime type metadata. It also adds helper methods to Type and comprehensive unit and integration tests. The review feedback suggests that parsing failures for Float64 and Float32 types when represented as StringValue should not be silently ignored, but rather propagated as a ConvertError to adhere to the "Demand Explosive Correctness" principle.

…llowing it

buu-nguyen · 2026-06-11T08:07:58Z

/gcbrun

buu-nguyen · 2026-06-11T08:09:17Z

/gemini review

gemini-code-assist

Code Review

This pull request implements the FromValue trait for serde_json::Value, enabling dynamic deserialization of Spanner values (including STRUCT, ARRAY, and nested types) into JSON values using runtime type metadata. It also adds helper methods to Type for struct metadata and safe reference reinterpretation, along with comprehensive unit and integration tests. The feedback suggests refactoring the float parsing logic in from_value_recursive to return early, thereby avoiding an unnecessary else block and reducing nesting depth in accordance with the repository style guide.

… logic flow linear

buu-nguyen · 2026-06-11T08:41:51Z

/gemini review

gemini-code-assist

Code Review

This pull request implements the FromValue trait for serde_json::Value to enable dynamic deserialization of Spanner types (such as STRUCT, ARRAY, and JSON) into JSON without requiring predefined Rust types. It introduces recursive conversion helpers, handles Spanner-specific type mappings (such as preserving high-precision types as strings and converting non-finite floats to null), and adds comprehensive unit and integration tests. Additionally, helper methods were added to Type to retrieve struct metadata and safely cast references. No review comments were provided, and the implementation is clean, well-tested, and conforms to the repository's style guidelines.

buu-nguyen · 2026-06-11T08:51:47Z

Code Review

This pull request implements the FromValue trait for serde_json::Value to enable dynamic deserialization of Spanner types (such as STRUCT, ARRAY, and JSON) into JSON without requiring predefined Rust types. It introduces recursive conversion helpers, handles Spanner-specific type mappings (such as preserving high-precision types as strings and converting non-finite floats to null), and adds comprehensive unit and integration tests. Additionally, helper methods were added to Type to retrieve struct metadata and safely cast references. No review comments were provided, and the implementation is clean, well-tested, and conforms to the repository's style guidelines.

Gemini finally approved this.
@olavloite Can you re-run the test suite? Thanks

olavloite · 2026-06-11T10:49:19Z

/gcbrun

buu-nguyen requested review from a team as code owners June 8, 2026 19:01

product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Jun 8, 2026

gemini-code-assist Bot reviewed Jun 8, 2026

View reviewed changes

Comment thread src/spanner/src/from_value.rs Outdated

Comment thread src/spanner/src/from_value.rs Outdated

Comment thread src/spanner/src/from_value.rs Outdated

buu-nguyen force-pushed the devin/1780940126-from-value-json branch from b446097 to 4996bf9 Compare June 8, 2026 19:10

olavloite self-assigned this Jun 9, 2026

gemini-code-assist Bot reviewed Jun 9, 2026

View reviewed changes

Comment thread src/spanner/src/from_value.rs

gemini-code-assist Bot reviewed Jun 10, 2026

View reviewed changes

Comment thread src/spanner/src/types.rs Outdated

Comment thread src/spanner/src/from_value.rs

Comment thread src/spanner/src/from_value.rs

Comment thread src/spanner/src/from_value.rs Outdated

Comment thread src/spanner/src/from_value.rs

buu-nguyen and others added 3 commits June 11, 2026 12:14

Update src/spanner/src/types.rs

527d97b

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Simplified by using Option::map and unwrap_or to avoid the nested if …

8f68edd

…let and else block, making the code more concise and idiomatic

Following the "Return Early" principle from the style guide

79c5ef3

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

buu-nguyen force-pushed the devin/1780940126-from-value-json branch from 69c173d to 79c5ef3 Compare June 11, 2026 05:27

gemini-code-assist Bot reviewed Jun 11, 2026

View reviewed changes

Comment thread src/spanner/src/from_value.rs

propagate the parsing error as a ConvertError instead of silently swa…

0b274b1

…llowing it

gemini-code-assist Bot reviewed Jun 11, 2026

View reviewed changes

Comment thread src/spanner/src/from_value.rs

Avoid unnecessary else blocks to reduce indentation and keep the main…

c5c5576

… logic flow linear

gemini-code-assist Bot reviewed Jun 11, 2026

View reviewed changes

Conversation

buu-nguyen commented Jun 8, 2026

Summary

Usage:

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

olavloite commented Jun 9, 2026

Uh oh!

olavloite commented Jun 9, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

codecov Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

olavloite commented Jun 10, 2026

Uh oh!

buu-nguyen commented Jun 10, 2026

Uh oh!

olavloite commented Jun 10, 2026

Uh oh!

olavloite commented Jun 10, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

olavloite commented Jun 10, 2026

Uh oh!

buu-nguyen commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

olavloite commented Jun 11, 2026

Uh oh!

olavloite commented Jun 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

buu-nguyen commented Jun 11, 2026

Uh oh!

buu-nguyen commented Jun 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

buu-nguyen commented Jun 11, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

buu-nguyen commented Jun 11, 2026

Code Review

Uh oh!

olavloite commented Jun 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

codecov Bot commented Jun 9, 2026 •

edited

Loading

buu-nguyen commented Jun 11, 2026 •

edited

Loading