From 5c5fd3ff0a03b6725097666d574a3784ea355765 Mon Sep 17 00:00:00 2001 From: Sean Aye Date: Thu, 9 Jan 2025 22:47:36 -0400 Subject: [PATCH 1/6] add json optional attribute parser and expansion --- sqlx-macros-core/src/derives/attributes.rs | 21 ++++++++++++---- sqlx-macros-core/src/derives/row.rs | 29 +++++++++++++++------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/sqlx-macros-core/src/derives/attributes.rs b/sqlx-macros-core/src/derives/attributes.rs index cf18cffca4..fcb70bf58c 100644 --- a/sqlx-macros-core/src/derives/attributes.rs +++ b/sqlx-macros-core/src/derives/attributes.rs @@ -1,8 +1,7 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::quote_spanned; use syn::{ - punctuated::Punctuated, token::Comma, Attribute, DeriveInput, Field, LitStr, Meta, Token, Type, - Variant, + parenthesized, parse::discouraged::AnyDelimiter, punctuated::Punctuated, token::{self, Comma}, Attribute, DeriveInput, Field, LitStr, Meta, Token, Type, Variant }; macro_rules! assert_attribute { @@ -61,13 +60,18 @@ pub struct SqlxContainerAttributes { pub default: bool, } +pub enum JsonAttribute { + Mandatory, + Optional +} + pub struct SqlxChildAttributes { pub rename: Option, pub default: bool, pub flatten: bool, pub try_from: Option, pub skip: bool, - pub json: bool, + pub json: Option, } pub fn parse_container_attributes(input: &[Attribute]) -> syn::Result { @@ -144,7 +148,7 @@ pub fn parse_child_attributes(input: &[Attribute]) -> syn::Result syn::Result - (false, None, false) => { + (false, None, None) => { predicates .push(parse_quote!(#ty: ::sqlx::decode::Decode<#lifetime, R::Database>)); predicates.push(parse_quote!(#ty: ::sqlx::types::Type)); @@ -107,12 +107,12 @@ fn expand_derive_from_row_struct( parse_quote!(__row.try_get(#id_s)) } // Flatten - (true, None, false) => { + (true, None, None) => { predicates.push(parse_quote!(#ty: ::sqlx::FromRow<#lifetime, R>)); parse_quote!(<#ty as ::sqlx::FromRow<#lifetime, R>>::from_row(__row)) } // Flatten + Try from - (true, Some(try_from), false) => { + (true, Some(try_from), None) => { predicates.push(parse_quote!(#try_from: ::sqlx::FromRow<#lifetime, R>)); parse_quote!( <#try_from as ::sqlx::FromRow<#lifetime, R>>::from_row(__row) @@ -130,11 +130,11 @@ fn expand_derive_from_row_struct( ) } // Flatten + Json - (true, _, true) => { + (true, _, Some(_)) => { panic!("Cannot use both flatten and json") } // Try from - (false, Some(try_from), false) => { + (false, Some(try_from), None) => { predicates .push(parse_quote!(#try_from: ::sqlx::decode::Decode<#lifetime, R::Database>)); predicates.push(parse_quote!(#try_from: ::sqlx::types::Type)); @@ -154,8 +154,8 @@ fn expand_derive_from_row_struct( }) ) } - // Try from + Json - (false, Some(try_from), true) => { + // Try from + Json mandatory + (false, Some(try_from), Some(JsonAttribute::Mandatory)) => { predicates .push(parse_quote!(::sqlx::types::Json<#try_from>: ::sqlx::decode::Decode<#lifetime, R::Database>)); predicates.push(parse_quote!(::sqlx::types::Json<#try_from>: ::sqlx::types::Type)); @@ -175,14 +175,25 @@ fn expand_derive_from_row_struct( }) ) }, + // Try from + Json optional + (false, Some(try_from), Some(JsonAttribute::Optional)) => { + panic!("Cannot use both try from and json optional") + }, // Json - (false, None, true) => { + (false, None, Some(JsonAttribute::Mandatory)) => { predicates .push(parse_quote!(::sqlx::types::Json<#ty>: ::sqlx::decode::Decode<#lifetime, R::Database>)); predicates.push(parse_quote!(::sqlx::types::Json<#ty>: ::sqlx::types::Type)); parse_quote!(__row.try_get::<::sqlx::types::Json<_>, _>(#id_s).map(|x| x.0)) }, + (false, None, Some(JsonAttribute::Optional)) => { + predicates + .push(parse_quote!(::core::Option<::sqlx::types::Json<#ty>>: ::sqlx::decode::Decode<#lifetime, R::Database>)); + predicates.push(parse_quote!(::core::Option<::sqlx::types::Json<#ty>>: ::sqlx::types::Type)); + + parse_quote!(__row.try_get::<::core::Option<::sqlx::types::Json<_>>, _>(#id_s).map(|x| x.map(|y| y.0))) + }, }; if attributes.default { From 39e8be3eac66666643639ccec87bf1a3df3d3cd8 Mon Sep 17 00:00:00 2001 From: Sean Aye Date: Thu, 9 Jan 2025 23:04:00 -0400 Subject: [PATCH 2/6] rename attribute --- sqlx-macros-core/src/derives/attributes.rs | 10 +++++----- sqlx-macros-core/src/derives/row.rs | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/sqlx-macros-core/src/derives/attributes.rs b/sqlx-macros-core/src/derives/attributes.rs index fcb70bf58c..000e88462b 100644 --- a/sqlx-macros-core/src/derives/attributes.rs +++ b/sqlx-macros-core/src/derives/attributes.rs @@ -61,8 +61,8 @@ pub struct SqlxContainerAttributes { } pub enum JsonAttribute { - Mandatory, - Optional + NonNullable, + Nullable } pub struct SqlxChildAttributes { @@ -170,10 +170,10 @@ pub fn parse_child_attributes(input: &[Attribute]) -> syn::Result { + (false, Some(try_from), Some(JsonAttribute::NonNullable)) => { predicates .push(parse_quote!(::sqlx::types::Json<#try_from>: ::sqlx::decode::Decode<#lifetime, R::Database>)); predicates.push(parse_quote!(::sqlx::types::Json<#try_from>: ::sqlx::types::Type)); @@ -175,24 +175,24 @@ fn expand_derive_from_row_struct( }) ) }, - // Try from + Json optional - (false, Some(try_from), Some(JsonAttribute::Optional)) => { - panic!("Cannot use both try from and json optional") + // Try from + Json nullable + (false, Some(try_from), Some(JsonAttribute::Nullable)) => { + panic!("Cannot use both try from and json nullable") }, // Json - (false, None, Some(JsonAttribute::Mandatory)) => { + (false, None, Some(JsonAttribute::NonNullable)) => { predicates .push(parse_quote!(::sqlx::types::Json<#ty>: ::sqlx::decode::Decode<#lifetime, R::Database>)); predicates.push(parse_quote!(::sqlx::types::Json<#ty>: ::sqlx::types::Type)); parse_quote!(__row.try_get::<::sqlx::types::Json<_>, _>(#id_s).map(|x| x.0)) }, - (false, None, Some(JsonAttribute::Optional)) => { + (false, None, Some(JsonAttribute::Nullable)) => { predicates .push(parse_quote!(::core::Option<::sqlx::types::Json<#ty>>: ::sqlx::decode::Decode<#lifetime, R::Database>)); predicates.push(parse_quote!(::core::Option<::sqlx::types::Json<#ty>>: ::sqlx::types::Type)); - parse_quote!(__row.try_get::<::core::Option<::sqlx::types::Json<_>>, _>(#id_s).map(|x| x.map(|y| y.0))) + parse_quote!(__row.try_get::<::core::Option<::sqlx::types::Json<_>>, _>(#id_s).map(|x| x.flatten().map(|y| y.0))) }, }; From e3175ac48fd5f7c16e32fcdd8588922b997c23c7 Mon Sep 17 00:00:00 2001 From: Sean Aye Date: Thu, 9 Jan 2025 23:32:42 -0400 Subject: [PATCH 3/6] add test --- sqlx-macros-core/src/derives/attributes.rs | 7 ++++--- sqlx-macros-core/src/derives/row.rs | 2 +- tests/mysql/macros.rs | 24 ++++++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/sqlx-macros-core/src/derives/attributes.rs b/sqlx-macros-core/src/derives/attributes.rs index 000e88462b..7b541f22b4 100644 --- a/sqlx-macros-core/src/derives/attributes.rs +++ b/sqlx-macros-core/src/derives/attributes.rs @@ -1,7 +1,7 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::quote_spanned; use syn::{ - parenthesized, parse::discouraged::AnyDelimiter, punctuated::Punctuated, token::{self, Comma}, Attribute, DeriveInput, Field, LitStr, Meta, Token, Type, Variant + parenthesized, punctuated::Punctuated, token::Comma, Attribute, DeriveInput, Field, LitStr, Meta, Token, Type, Variant }; macro_rules! assert_attribute { @@ -170,7 +170,8 @@ pub fn parse_child_attributes(input: &[Attribute]) -> syn::Result syn::Result { + (false, Some(_), Some(JsonAttribute::Nullable)) => { panic!("Cannot use both try from and json nullable") }, // Json diff --git a/tests/mysql/macros.rs b/tests/mysql/macros.rs index f6bc75955a..4dbdf07841 100644 --- a/tests/mysql/macros.rs +++ b/tests/mysql/macros.rs @@ -494,6 +494,30 @@ async fn test_from_row_json_attr() -> anyhow::Result<()> { Ok(()) } +#[sqlx_macros::test] +async fn test_from_row_json_attr_nullable() -> anyhow::Result<()> { + #[derive(serde::Deserialize)] + struct J { + a: u32, + b: u32, + } + + #[derive(sqlx::FromRow)] + struct Record { + #[sqlx(json(nullable))] + j: Option, + } + + let mut conn = new::().await?; + + let record = sqlx::query_as::<_, Record>("select null as j") + .fetch_one(&mut conn) + .await?; + + assert_eq!(record.j, None); + Ok(()) +} + #[sqlx_macros::test] async fn test_from_row_json_try_from_attr() -> anyhow::Result<()> { #[derive(serde::Deserialize)] From c38248962354509352b15f54220a2eb7e9cb271b Mon Sep 17 00:00:00 2001 From: Sean Aye Date: Thu, 9 Jan 2025 23:49:31 -0400 Subject: [PATCH 4/6] fix tests --- sqlx-macros-core/src/derives/row.rs | 6 +++--- tests/mysql/macros.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sqlx-macros-core/src/derives/row.rs b/sqlx-macros-core/src/derives/row.rs index 83c4659c19..604986f90b 100644 --- a/sqlx-macros-core/src/derives/row.rs +++ b/sqlx-macros-core/src/derives/row.rs @@ -189,10 +189,10 @@ fn expand_derive_from_row_struct( }, (false, None, Some(JsonAttribute::Nullable)) => { predicates - .push(parse_quote!(::core::Option<::sqlx::types::Json<#ty>>: ::sqlx::decode::Decode<#lifetime, R::Database>)); - predicates.push(parse_quote!(::core::Option<::sqlx::types::Json<#ty>>: ::sqlx::types::Type)); + .push(parse_quote!(::core::option::Option<::sqlx::types::Json<#ty>>: ::sqlx::decode::Decode<#lifetime, R::Database>)); + predicates.push(parse_quote!(::core::option::Option<::sqlx::types::Json<#ty>>: ::sqlx::types::Type)); - parse_quote!(__row.try_get::<::core::Option<::sqlx::types::Json<_>>, _>(#id_s).map(|x| x.flatten().map(|y| y.0))) + parse_quote!(__row.try_get::<::core::option::Option<::sqlx::types::Json<_>>, _>(#id_s).map(|x| x.and_then(|y| y.0))) }, }; diff --git a/tests/mysql/macros.rs b/tests/mysql/macros.rs index 4dbdf07841..9c524c2a46 100644 --- a/tests/mysql/macros.rs +++ b/tests/mysql/macros.rs @@ -510,11 +510,11 @@ async fn test_from_row_json_attr_nullable() -> anyhow::Result<()> { let mut conn = new::().await?; - let record = sqlx::query_as::<_, Record>("select null as j") + let record = sqlx::query_as::<_, Record>("select NULL as j") .fetch_one(&mut conn) .await?; - assert_eq!(record.j, None); + assert!(record.j.is_none()); Ok(()) } From 259a08bbf9fa6c9246f9e17e28239cd87ac3dbfe Mon Sep 17 00:00:00 2001 From: Sean Aye Date: Thu, 23 Jan 2025 21:31:32 -0500 Subject: [PATCH 5/6] fix lints --- sqlx-macros-core/src/derives/attributes.rs | 5 +++-- tests/mysql/macros.rs | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sqlx-macros-core/src/derives/attributes.rs b/sqlx-macros-core/src/derives/attributes.rs index 7b541f22b4..c69687908b 100644 --- a/sqlx-macros-core/src/derives/attributes.rs +++ b/sqlx-macros-core/src/derives/attributes.rs @@ -1,7 +1,8 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::quote_spanned; use syn::{ - parenthesized, punctuated::Punctuated, token::Comma, Attribute, DeriveInput, Field, LitStr, Meta, Token, Type, Variant + parenthesized, punctuated::Punctuated, token::Comma, Attribute, DeriveInput, Field, LitStr, + Meta, Token, Type, Variant, }; macro_rules! assert_attribute { @@ -62,7 +63,7 @@ pub struct SqlxContainerAttributes { pub enum JsonAttribute { NonNullable, - Nullable + Nullable, } pub struct SqlxChildAttributes { diff --git a/tests/mysql/macros.rs b/tests/mysql/macros.rs index 9c524c2a46..8187f6d8d8 100644 --- a/tests/mysql/macros.rs +++ b/tests/mysql/macros.rs @@ -497,6 +497,7 @@ async fn test_from_row_json_attr() -> anyhow::Result<()> { #[sqlx_macros::test] async fn test_from_row_json_attr_nullable() -> anyhow::Result<()> { #[derive(serde::Deserialize)] + #[allow(dead_code)] struct J { a: u32, b: u32, From db09397710c59e75314dc97ddf6a0be159372c58 Mon Sep 17 00:00:00 2001 From: Sean Aye Date: Sun, 26 Jan 2025 17:06:32 -0500 Subject: [PATCH 6/6] Add docs --- sqlx-core/src/from_row.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/sqlx-core/src/from_row.rs b/sqlx-core/src/from_row.rs index 9c647d370a..8776855d92 100644 --- a/sqlx-core/src/from_row.rs +++ b/sqlx-core/src/from_row.rs @@ -271,6 +271,32 @@ use crate::{error::Error, row::Row}; /// } /// } /// ``` +/// +/// By default the `#[sqlx(json)]` attribute will assume that the underlying database row is +/// _not_ NULL. This can cause issues when your field type is an `Option` because this would be +/// represented as the _not_ NULL (in terms of DB) JSON value of `null`. +/// +/// If you wish to describe a database row which _is_ NULLable but _cannot_ contain the JSON value `null`, +/// use the `#[sqlx(json(nullable))]` attrubute. +/// +/// For example +/// ```rust,ignore +/// #[derive(serde::Deserialize)] +/// struct Data { +/// field1: String, +/// field2: u64 +/// } +/// +/// #[derive(sqlx::FromRow)] +/// struct User { +/// id: i32, +/// name: String, +/// #[sqlx(json(nullable))] +/// metadata: Option +/// } +/// ``` +/// Would describe a database field which _is_ NULLable but if it exists it must be the JSON representation of `Data` +/// and cannot be the JSON value `null` pub trait FromRow<'r, R: Row>: Sized { fn from_row(row: &'r R) -> Result; }