perf(spanner): add by-value parameter and mutation binding methods

[fornwall]

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

Key Changes

• 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 PR supersedes both PR perf(spanner): add ToValue for &[u8] #6070 and PR feat(spanner): implement ToValue for str #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:

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

[gemini-code-assist]

Code Review

This pull request introduces owned-value variants (to_value, add_param_value, and add_typed_param_value) to ValueBinder and StatementBuilder in the Spanner crate, allowing callers to pass owned values and avoid unnecessary deep clones. The existing reference-based methods have been refactored to delegate to these new methods, and corresponding unit tests have been added. The review feedback correctly points out that the newly added tests use .unwrap(), which violates the repository style guide; these should be replaced with .expect() to provide descriptive error messages upon failure.

[olavloite]

Can we solve this in a slightly different way that does not require adding new methods to the public API. Can we instead change the input argument of the existing methods to accept Into<Value>, and just add this blanket implementation:

impl<T: ToValue + ?Sized> From<&T> for Value {
    fn from(t: &T) -> Self {
        t.to_value()
    }
}

So the entire diff would then be something like this:

--- a/src/spanner/src/mutation.rs
+++ b/src/spanner/src/mutation.rs
@@ -298,9 +298,9 @@ pub struct ValueBinder {
 
 impl ValueBinder {
     /// Sets the value for the column.
-    pub fn to<T: ToValue + ?Sized>(mut self, value: &T) -> WriteBuilder {
+    pub fn to<T: Into<Value>>(mut self, value: T) -> WriteBuilder {
         self.builder.columns.push(self.column);
-        self.builder.values.push(value.to_value());
+        self.builder.values.push(value.into());
         self.builder
     }
 }
 
--- a/src/spanner/src/statement.rs
+++ b/src/spanner/src/statement.rs
@@ -69,8 +69,8 @@ impl StatementBuilder {
     /// It is recommended to use untyped parameter values, unless you explicitly want Spanner to
     /// verify that the type of the parameter value is exactly the same as the type that would
     /// otherwise be inferred from the SQL string.
-    pub fn add_param<T: ToValue + ?Sized>(mut self, name: impl Into<String>, value: &T) -> Self {
-        self.params.insert(name.into(), value.to_value());
+    pub fn add_param<T: Into<Value>>(mut self, name: impl Into<String>, value: T) -> Self {
+        self.params.insert(name.into(), value.into());
         self
     }
 
@@ -78,14 +78,14 @@ impl StatementBuilder {
     ///
     /// The parameter value is sent with an explicit type code to Spanner. The type code must
     /// correspond with the expression in the SQL string that the query parameter is bound to.
-    pub fn add_typed_param<T: ToValue + ?Sized>(
+    pub fn add_typed_param<T: Into<Value>>(
         mut self,
         name: impl Into<String>,
-        value: &T,
+        value: T,
         param_type: Type,
     ) -> Self {
         let name = name.into();
-        self.params.insert(name.clone(), value.to_value());
+        self.params.insert(name.clone(), value.into());
         self.param_types.insert(name, param_type);
         self
     }
 
--- a/src/spanner/src/to_value.rs
+++ b/src/spanner/src/to_value.rs
@@ -194,6 +194,13 @@ where
     }
 }
 
+// Blanket implementation to preserve backwards compatibility for callers passing &T
+// where T implements ToValue.
+impl<T: ToValue + ?Sized> From<&T> for Value {
+    fn from(t: &T) -> Self {
+        t.to_value()
+    }
+}
+
 #[cfg(test)]
 mod tests {

[fornwall]

Thanks @olavloite, applied your suggestion. Slight changes compared to your diff:

1. Regression tests for the new construction added.
2. Make the blanket impl comment a doc comment since it shows up in the public API. And reworded it, since it's not just for backward compat AFAIK, but also for caller ergonomy - not requiring .to_value() in stmt.add_param("age", age.to_value().

[olavloite]

/gcbrun

[olavloite]

/gcbrun

[codecov]

Codecov Report

❌ Patch coverage is 97.08223% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.57%. Comparing base (b3a930b) to head (2465719).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/spanner/src/to_value.rs	93.93%	10 Missing ⚠️
src/spanner/src/mutation.rs	98.83%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #6071    +/-   ##
========================================
  Coverage   96.57%   96.57%            
========================================
  Files         264      264            
  Lines       66577    66872   +295     
========================================
+ Hits        64297    64584   +287     
- Misses       2280     2288     +8     

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

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors Spanner's parameter binding APIs to accept types implementing Into<Value> instead of references to ToValue types, enabling owned value passing and adding support for untyped nulls. However, the reviewer points out that because From is not implemented for common owned types (like String, i64, bool) or &str directly, callers will face compilation errors or usability issues. Implementing From for these common types is recommended to fully realize the performance and usability benefits of the API change.

[olavloite]

/gcbrun

[olavloite]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the Spanner client's parameter binding and mutation APIs to accept values by-value using the Into<Value> trait instead of by-reference using ToValue. To support this, numerous From<T> for Value implementations have been added for common types, along with a Value::null() helper and comprehensive unit tests verifying the new conversions. I have no feedback to provide as there are no review comments and the changes are clean and well-tested.

[olavloite]

/gcbrun