feat(spanner): accept T: Into<Value> in parameter and mutation bindings - #6184
feat(spanner): accept T: Into<Value> in parameter and mutation bindings#6184olavloite wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Spanner client's parameter and mutation value binding APIs (ValueBinder::to, StatementBuilder::add_param, and StatementBuilder::add_typed_param) to accept values implementing Into<Value> by value, rather than requiring references to types implementing ToValue. This change significantly simplifies client code by eliminating the need for explicit borrows (e.g., .to(&1) becomes .to(1)). Corresponding updates have been made across examples, integration tests, and unit tests, and new tests have been added to verify various value conversions, including owned, borrowed, optional, and null types. No review comments were provided, so there is no additional feedback to address.
c9bc31a to
7151539
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6184 +/- ##
========================================
Coverage 96.57% 96.58%
========================================
Files 264 264
Lines 66577 66894 +317
========================================
+ Hits 64297 64609 +312
- Misses 2280 2285 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
7151539 to
f078404
Compare
Changes `StatementBuilder::add_param`, `StatementBuilder::add_typed_param`, and `ValueBinder::to` to accept `T: Into<Value>` instead of `&T: ToValue`. This allows callers to pass owned values directly (like `String`, `Value`, or `Option<T>`) zero-copy without unnecessary cloning, while maintaining complete backward compatibility for references (`&str`, `&i64`, `&Option<T>`). - **Direct `From<T> for Value` Implementations**: Added zero-copy `From` conversions for owned types (`String`, `i64`, `i32`, `bool`, `f64`, `f32`, `Decimal`, `Date`, `OffsetDateTime`, `SystemTime`, `wkt::Timestamp`, `Vec<u8>`, `Option<T>`, `Vec<T>`, `ProtoValue`). - **Decoupled `ToValue`**: All `ToValue` trait implementations now delegate directly to `From` / `.into()`, decoupling conversion logic from `ToValue` and paving the way for eventual deprecation of the `ToValue` trait. - **Unsized Slice Support (`[u8]` and `str`)**: Implemented `ToValue` for `[u8]` and `str` (and `&[u8]`, `&str`), enabling zero-copy slice conversions and generic trait bounds `T: ToValue + ?Sized`. This supersedes both PR googleapis#6070 and PR googleapis#6087. - **Untyped NULL Support**: Added `Value::null()`, `From<()>` and `ToValue for ()` to easily support untyped NULL parameters (`Value::null()`, `None::<()>`, `None::<Value>`). - **Cleaned up Examples & Tests**: Updated example scripts and tests to pass owned values directly, eliminating needless reference borrows (`clippy::needless_borrows_for_generic_args`). BREAKING CHANGE: This is a minor breaking change only for code that passes inline `&None` with a method-level turbofish: ```rust // Old (fails to compile): builder.set("Col").to::<Option<bool>>(&None); // Works both before and after this change: builder.set("Col").to(&None::<bool>); // New (remove the &): builder.set("Col").to::<Option<bool>>(None); // Or: builder.set("Col").to(None::<bool>); builder.set("Col").to(Value::null()); ``` Code passing existing `Option` variables (like `.to(&my_var)`) is unaffected and continues to compile unchanged.
f078404 to
b205c13
Compare
Changes
StatementBuilder::add_param,StatementBuilder::add_typed_param, andValueBinder::toto acceptT: Into<Value>instead of&T: ToValue.This allows callers to pass owned values directly (like
String,Value, orOption<T>) zero-copy without unnecessary cloning, while maintaining complete backward compatibility for references (&str,&i64,&Option<T>).Key Changes
From<T> for ValueImplementations: Added zero-copyFromconversions for owned types (String,i64,i32,bool,f64,f32,Decimal,Date,OffsetDateTime,SystemTime,wkt::Timestamp,Vec<u8>,Option<T>,Vec<T>,ProtoValue).ToValue: AllToValuetrait implementations now delegate directly toFrom/.into(), decoupling conversion logic fromToValueand paving the way for eventual deprecation of theToValuetrait.[u8]andstr): ImplementedToValuefor[u8]andstr(and&[u8],&str), enabling zero-copy slice conversions and generic trait boundsT: ToValue + ?Sized. This supersedes both PR perf(spanner): add ToValue for &[u8] #6070 and PR feat(spanner): implement ToValue for str #6087.Value::null(),From<()>andToValue for ()to easily support untyped NULL parameters (Value::null(),None::<()>,None::<Value>).clippy::needless_borrows_for_generic_args).BREAKING CHANGE
BREAKING CHANGE: This is a minor breaking change only for code that passes inline
&Nonewith a method-level turbofish:Code passing existing
Optionvariables (like.to(&my_var)) is unaffected and continues to compile unchanged.This pull request replaces the following pull requests: