From 373cb77c8d456ce9711d2904ab8e4e07de542a81 Mon Sep 17 00:00:00 2001 From: Alvaro Viebrantz Date: Tue, 1 Sep 2026 21:08:15 +0000 Subject: [PATCH] refactor(bigquery)!: rename FromSql::from_sql to from_value --- src/bigquery-derive/src/lib.rs | 6 +- src/bigquery/src/datatypes.rs | 12 ++-- src/bigquery/src/query/from_sql.rs | 104 ++++++++++++++++------------- src/bigquery/src/query/row.rs | 2 +- 4 files changed, 69 insertions(+), 55 deletions(-) diff --git a/src/bigquery-derive/src/lib.rs b/src/bigquery-derive/src/lib.rs index fa0a90298c..7b99e91e03 100644 --- a/src/bigquery-derive/src/lib.rs +++ b/src/bigquery-derive/src/lib.rs @@ -119,7 +119,7 @@ pub fn derive_from_sql(input: TokenStream) -> TokenStream { quote! { let #field_name = iter.next() .ok_or_else(|| google_cloud_bigquery::error::ConvertError::MissingField(#db_column_name.to_string()))?; - let #field_name = google_cloud_bigquery::query::FromSql::from_sql(#field_name)?; + let #field_name = google_cloud_bigquery::query::FromSql::from_value(#field_name)?; } }); @@ -129,13 +129,13 @@ pub fn derive_from_sql(input: TokenStream) -> TokenStream { quote! { let #field_name = obj.remove(#db_column_name) .ok_or_else(|| google_cloud_bigquery::error::ConvertError::MissingField(#db_column_name.to_string()))?; - let #field_name = google_cloud_bigquery::query::FromSql::from_sql(#field_name)?; + let #field_name = google_cloud_bigquery::query::FromSql::from_value(#field_name)?; } }); let expanded = quote! { impl google_cloud_bigquery::query::FromSql for #name { - fn from_sql(value: wkt::Value) -> std::result::Result { + fn from_value(value: wkt::Value) -> std::result::Result { match value { wkt::Value::Array(arr) => { let mut iter = arr.into_iter(); diff --git a/src/bigquery/src/datatypes.rs b/src/bigquery/src/datatypes.rs index d546d06374..07a59815a5 100644 --- a/src/bigquery/src/datatypes.rs +++ b/src/bigquery/src/datatypes.rs @@ -68,7 +68,7 @@ pub struct Interval { } impl FromSql for Interval { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => { let mut parts = s.split_whitespace(); @@ -184,7 +184,7 @@ pub struct Range { } impl FromSql for Range { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => { let trimmed = s.trim(); @@ -216,13 +216,13 @@ impl FromSql for Range { let start = if start_str.is_empty() || start_str == "UNBOUNDED" { None } else { - Some(T::from_sql(wkt::Value::String(start_str.to_string()))?) + Some(T::from_value(wkt::Value::String(start_str.to_string()))?) }; let end = if end_str.is_empty() || end_str == "UNBOUNDED" { None } else { - Some(T::from_sql(wkt::Value::String(end_str.to_string()))?) + Some(T::from_value(wkt::Value::String(end_str.to_string()))?) }; Ok(Range { start, end }) @@ -275,7 +275,7 @@ mod tests { #[test_case(wkt::Value::String("1 3 4:05:06".to_string()) => Err(TestConvertError::Convert("invalid interval year-month format".to_string())) ; "invalid year-month format")] #[test_case(wkt::Value::String("1-2 3 4:05".to_string()) => Err(TestConvertError::Convert("a character literal was not valid".to_string())) ; "invalid time format")] fn test_from_sql_interval(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("[2026-05-28, 2026-05-29)".to_string()) => Ok(Range { start: Some(google_cloud_type::model::Date::new().set_year(2026).set_month(5).set_day(28)), end: Some(google_cloud_type::model::Date::new().set_year(2026).set_month(5).set_day(29)) }) ; "date range bounded")] @@ -293,6 +293,6 @@ mod tests { fn test_from_sql_range( value: wkt::Value, ) -> Result, TestConvertError> { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } } diff --git a/src/bigquery/src/query/from_sql.rs b/src/bigquery/src/query/from_sql.rs index fedcf9854c..480f3b8f1f 100644 --- a/src/bigquery/src/query/from_sql.rs +++ b/src/bigquery/src/query/from_sql.rs @@ -33,9 +33,23 @@ pub(crate) const BIGQUERY_DATETIME_SUBSEC_FORMAT: &[time::format_description::Fo 'static, >] = time::macros::format_description!("[year]-[month]-[day]T[hour]:[minute]:[second].[subsecond]"); -/// A trait for converting BigQuery [`wkt::Value`] representations into Rust +/// A trait for converting BigQuery value representations into Rust /// types. /// +/// The BigQuery client uses [`wkt::Value`] by default, but will support +/// Arrow record batches via the Storage Read API integration in the future. +/// +///
+/// +/// **Do not implement this trait directly.** +/// +/// This trait is not intended for manual implementation. New methods (such as +/// Arrow conversion methods) may be added in future versions, which would break +/// manual implementations. To deserialize custom structs, use [`#[derive(FromSql)]`](derive@crate::query::FromSql) +/// or [`#[derive(FromRow)]`](crate::query::FromRow) instead. +/// +///
+/// /// [`Row::get()`](crate::query::Row::get) or [`Row::take()`](crate::query::Row::take) /// use this trait to convert cell values, and the [`FromRow`](crate::query::FromRow) /// derive macro uses it for field deserialization. @@ -71,17 +85,17 @@ pub(crate) const BIGQUERY_DATETIME_SUBSEC_FORMAT: &[time::format_description::Fo /// ``` pub trait FromSql: Sized { /// Converts a BigQuery `wkt::Value` into the implementing type. - fn from_sql(value: wkt::Value) -> Result; + fn from_value(value: wkt::Value) -> Result; } impl FromSql for wkt::Value { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { Ok(value) } } impl FromSql for String { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => Ok(s), wkt::Value::Null => Err(ConvertError::NotNull), @@ -94,7 +108,7 @@ impl FromSql for String { } impl FromSql for i32 { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::Number(n) => n .as_i64() @@ -113,7 +127,7 @@ impl FromSql for i32 { } impl FromSql for i64 { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::Number(n) => n .as_i64() @@ -131,7 +145,7 @@ impl FromSql for i64 { } impl FromSql for f32 { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::Number(n) => n .as_f64() @@ -150,7 +164,7 @@ impl FromSql for f32 { } impl FromSql for f64 { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::Number(n) => n .as_f64() @@ -168,7 +182,7 @@ impl FromSql for f64 { } impl FromSql for bool { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::Bool(b) => Ok(b), wkt::Value::String(s) => s @@ -184,18 +198,18 @@ impl FromSql for bool { } impl FromSql for Option { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::Null => Ok(None), - other => T::from_sql(other).map(Some), + other => T::from_value(other).map(Some), } } } impl FromSql for Vec { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { - wkt::Value::Array(arr) => arr.into_iter().map(T::from_sql).collect(), + wkt::Value::Array(arr) => arr.into_iter().map(T::from_value).collect(), wkt::Value::Null => Err(ConvertError::NotNull), other => Err(ConvertError::TypeMismatch { expected: "array", @@ -206,7 +220,7 @@ impl FromSql for Vec { } impl FromSql for wkt::Struct { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::Object(obj) => Ok(obj), wkt::Value::Null => Err(ConvertError::NotNull), @@ -219,7 +233,7 @@ impl FromSql for wkt::Struct { } impl FromSql for wkt::Timestamp { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => { let micros = s @@ -251,7 +265,7 @@ fn timestamp_from_micros(micros: i64) -> Result { } impl FromSql for google_cloud_type::model::Date { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => { let date = time::Date::parse(s.as_str(), BIGQUERY_DATE_FORMAT) @@ -280,7 +294,7 @@ pub(crate) fn parse_time(s: &str) -> Result { } impl FromSql for google_cloud_type::model::TimeOfDay { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => { let time = parse_time(s.as_str())?; @@ -300,7 +314,7 @@ impl FromSql for google_cloud_type::model::TimeOfDay { } impl FromSql for google_cloud_type::model::DateTime { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => { let format = if s.contains('.') { @@ -329,7 +343,7 @@ impl FromSql for google_cloud_type::model::DateTime { } impl FromSql for google_cloud_type::model::Decimal { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => Ok(google_cloud_type::model::Decimal::new().set_value(s)), wkt::Value::Number(n) => { @@ -345,7 +359,7 @@ impl FromSql for google_cloud_type::model::Decimal { } impl FromSql for rust_decimal::Decimal { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => s .trim() @@ -373,7 +387,7 @@ impl FromSql for rust_decimal::Decimal { } impl FromSql for Vec { - fn from_sql(value: wkt::Value) -> Result { + fn from_value(value: wkt::Value) -> Result { match value { wkt::Value::String(s) => BASE64_STANDARD .decode(s) @@ -388,8 +402,8 @@ impl FromSql for Vec { } impl FromSql for bytes::Bytes { - fn from_sql(value: wkt::Value) -> Result { - Vec::::from_sql(value).map(bytes::Bytes::from) + fn from_value(value: wkt::Value) -> Result { + Vec::::from_value(value).map(bytes::Bytes::from) } } @@ -426,14 +440,14 @@ mod tests { #[test_case(wkt::Value::String("hello".to_string()) => Ok(wkt::Value::String("hello".to_string())) ; "value string")] fn test_from_sql_value(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("hello".to_string()) => Ok("hello".to_string()) ; "string")] #[test_case(wkt::Value::Null => Err(TestConvertError::NotNull) ; "null string")] #[test_case(wkt::Value::Number(123.into()) => Err(TestConvertError::TypeMismatch("string")) ; "type mismatch string")] fn test_from_sql_string(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::Number(123.into()) => Ok(123) ; "i64 from number")] @@ -442,7 +456,7 @@ mod tests { #[test_case(wkt::Value::Bool(true) => Err(TestConvertError::TypeMismatch("number or string")) ; "try bool as i64")] #[test_case(wkt::Value::String("hello".to_string()) => Err(TestConvertError::Convert("invalid digit found in string".to_string())) ; "invalid string as i64")] fn test_from_sql_i64(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::Number(serde_json::Number::from_f64(123.45).unwrap()) => Ok(123.45) ; "f64 from number")] @@ -451,7 +465,7 @@ mod tests { #[test_case(wkt::Value::Bool(true) => Err(TestConvertError::TypeMismatch("number or string")) ; "try bool as f64")] #[test_case(wkt::Value::String("hello".to_string()) => Err(TestConvertError::Convert("invalid float literal".to_string())) ; "invalid string as f64")] fn test_from_sql_f64(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::Bool(true) => Ok(true) ; "bool true")] @@ -462,29 +476,29 @@ mod tests { #[test_case(wkt::Value::Number(1.into()) => Err(TestConvertError::TypeMismatch("bool or string")) ; "try number as bool")] #[test_case(wkt::Value::String("hello".to_string()) => Err(TestConvertError::Convert("provided string was not `true` or `false`".to_string())) ; "invalid string as bool")] fn test_from_sql_bool(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::Null => Ok(None) ; "option null")] #[test_case(wkt::Value::Number(123.into()) => Ok(Some(123)) ; "option some i64")] #[test_case(wkt::Value::String("hello".to_string()) => Err(TestConvertError::Convert("invalid digit found in string".to_string())) ; "option error i64")] fn test_from_sql_option(value: wkt::Value) -> Result, TestConvertError> { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::Array(vec![wkt::Value::Number(1.into()), wkt::Value::Number(2.into())]) => Ok(vec![1, 2]) ; "vec i64")] - #[test_case(wkt::Value::Null => Err(TestConvertError::NotNull) ; "vec null")] + #[test_case(wkt::Value::Null => Err(TestConvertError::NotNull) ; "null vec i64")] #[test_case(wkt::Value::String("hello".to_string()) => Err(TestConvertError::TypeMismatch("array")) ; "vec type mismatch")] #[test_case(wkt::Value::Array(vec![wkt::Value::String("invalid".to_string())]) => Err(TestConvertError::Convert("invalid digit found in string".to_string())) ; "vec element convert error")] fn test_from_sql_vec(value: wkt::Value) -> Result, TestConvertError> { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::Object(wkt::Struct::from_iter([("a".to_string(), wkt::Value::Number(1.into()))])) => Ok(wkt::Struct::from_iter([("a".to_string(), wkt::Value::Number(1.into()))])) ; "struct ok")] #[test_case(wkt::Value::Null => Err(TestConvertError::NotNull) ; "struct null")] #[test_case(wkt::Value::String("hello".to_string()) => Err(TestConvertError::TypeMismatch("object")) ; "struct type mismatch")] fn test_from_sql_struct(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("1779982200000000".to_string()) => Ok(wkt::Timestamp::new(1779982200, 0).unwrap()) ; "timestamp micro integer string")] @@ -494,7 +508,7 @@ mod tests { #[test_case(wkt::Value::Null => Err(TestConvertError::NotNull) ; "timestamp null")] #[test_case(wkt::Value::Bool(true) => Err(TestConvertError::TypeMismatch("string or number")) ; "timestamp type mismatch")] fn test_from_sql_timestamp(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("2026-05-28".to_string()) => Ok(google_cloud_type::model::Date::new().set_year(2026).set_month(5).set_day(28)) ; "date valid")] @@ -505,7 +519,7 @@ mod tests { fn test_from_sql_date( value: wkt::Value, ) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("15:30:00".to_string()) => Ok(google_cloud_type::model::TimeOfDay::new().set_hours(15).set_minutes(30).set_seconds(0).set_nanos(0)) ; "time of day valid")] @@ -515,7 +529,7 @@ mod tests { fn test_from_sql_time_of_day( value: wkt::Value, ) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("2026-05-28T15:30:00".to_string()) => Ok(google_cloud_type::model::DateTime::new().set_year(2026).set_month(5).set_day(28).set_hours(15).set_minutes(30).set_seconds(0).set_nanos(0)) ; "datetime without subseconds")] @@ -525,7 +539,7 @@ mod tests { fn test_from_sql_datetime( value: wkt::Value, ) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::Number(123.into()) => Ok(123) ; "i32 from number")] @@ -535,7 +549,7 @@ mod tests { #[test_case(wkt::Value::Bool(true) => Err(TestConvertError::TypeMismatch("number or string")) ; "try bool as i32")] #[test_case(wkt::Value::String("hello".to_string()) => Err(TestConvertError::Convert("invalid digit found in string".to_string())) ; "invalid string as i32")] fn test_from_sql_i32(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::Number(serde_json::Number::from_f64(123.45).unwrap()) => Ok(123.45) ; "f32 from number")] @@ -544,7 +558,7 @@ mod tests { #[test_case(wkt::Value::Bool(true) => Err(TestConvertError::TypeMismatch("number or string")) ; "try bool as f32")] #[test_case(wkt::Value::String("hello".to_string()) => Err(TestConvertError::Convert("invalid float literal".to_string())) ; "invalid string as f32")] fn test_from_sql_f32(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("123.456".to_string()) => Ok(Decimal::new().set_value("123.456")) ; "decimal from string")] @@ -552,7 +566,7 @@ mod tests { #[test_case(wkt::Value::Null => Err(TestConvertError::NotNull) ; "null decimal")] #[test_case(wkt::Value::Bool(true) => Err(TestConvertError::TypeMismatch("string or number")) ; "try bool as decimal")] fn test_from_sql_decimal(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("123.456".to_string()) => Ok(RustDecimal::from_str_exact("123.456").unwrap()) ; "rust_decimal from string")] @@ -561,7 +575,7 @@ mod tests { #[test_case(wkt::Value::Null => Err(TestConvertError::NotNull) ; "null rust_decimal")] #[test_case(wkt::Value::Bool(true) => Err(TestConvertError::TypeMismatch("string or number")) ; "try bool as rust_decimal")] fn test_from_sql_rust_decimal(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("AQIDBA==".to_string()) => Ok(vec![1, 2, 3, 4]) ; "vec u8 from base64")] @@ -569,7 +583,7 @@ mod tests { #[test_case(wkt::Value::Null => Err(TestConvertError::NotNull) ; "null vec u8")] #[test_case(wkt::Value::Bool(true) => Err(TestConvertError::TypeMismatch("string (base64 encoded)")) ; "try bool as vec u8")] fn test_from_sql_vec_u8(value: wkt::Value) -> Result, TestConvertError> { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case(wkt::Value::String("AQIDBA==".to_string()) => Ok(bytes::Bytes::from_static(&[1, 2, 3, 4])) ; "bytes from base64")] @@ -577,16 +591,16 @@ mod tests { #[test_case(wkt::Value::Null => Err(TestConvertError::NotNull) ; "null bytes")] #[test_case(wkt::Value::Bool(true) => Err(TestConvertError::TypeMismatch("string (base64 encoded)")) ; "try bool as bytes")] fn test_from_sql_bytes(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } #[test_case("AQIDBA" ; "missing padding")] #[test_case("Not a base64 string" ; "words with spaces")] fn test_from_sql_bytes_invalid_base64(input: &str) { - let err = bytes::Bytes::from_sql(wkt::Value::String(input.to_string())).unwrap_err(); + let err = bytes::Bytes::from_value(wkt::Value::String(input.to_string())).unwrap_err(); assert!(matches!(err, ConvertError::Convert(_))); - let err = Vec::::from_sql(wkt::Value::String(input.to_string())).unwrap_err(); + let err = Vec::::from_value(wkt::Value::String(input.to_string())).unwrap_err(); assert!(matches!(err, ConvertError::Convert(_))); } @@ -603,6 +617,6 @@ mod tests { #[test_case(wkt::Value::Object(wkt::Struct::from_iter([("name".to_string(), wkt::Value::String("James".to_string())), ("some_bool".to_string(), wkt::Value::Bool(true))])) => Err(TestConvertError::MissingField("custom_int".to_string())) ; "missing field")] #[test_case(wkt::Value::String("invalid".to_string()) => Err(TestConvertError::TypeMismatch("array or object")) ; "type mismatch")] fn test_derive_from_sql(value: wkt::Value) -> Result { - FromSql::from_sql(value).map_err(TestConvertError::from) + FromSql::from_value(value).map_err(TestConvertError::from) } } diff --git a/src/bigquery/src/query/row.rs b/src/bigquery/src/query/row.rs index a814147e8a..5afa541cce 100644 --- a/src/bigquery/src/query/row.rs +++ b/src/bigquery/src/query/row.rs @@ -116,7 +116,7 @@ impl Row { } fn convert_value_at(&self, idx: usize, val: Value) -> Result { - T::from_sql(val).map_err(|e| { + T::from_value(val).map_err(|e| { let field_name = self .schema .get_field_by_index(idx)