From 0540dadc17b0ef10fcac7b260d6e10e4a6368711 Mon Sep 17 00:00:00 2001 From: Austin Bonander Date: Fri, 5 Jan 2024 20:56:47 -0800 Subject: [PATCH] refactor: lift type mappings into driver crates Motivated by #2917 --- sqlx-core/src/lib.rs | 1 + sqlx-core/src/type_checking.rs | 188 ++++++++++++++++++ sqlx-macros-core/src/database/impls.rs | 71 +++++++ sqlx-macros-core/src/database/mod.rs | 129 +----------- sqlx-macros-core/src/query/args.rs | 2 +- sqlx-macros-core/src/query/output.rs | 5 +- sqlx-mysql/src/lib.rs | 1 + .../src/type_checking.rs | 16 +- sqlx-postgres/src/lib.rs | 1 + .../src/type_checking.rs | 20 +- sqlx-sqlite/src/lib.rs | 1 + .../src/type_checking.rs | 22 +- 12 files changed, 308 insertions(+), 149 deletions(-) create mode 100644 sqlx-core/src/type_checking.rs create mode 100644 sqlx-macros-core/src/database/impls.rs rename sqlx-macros-core/src/database/mysql.rs => sqlx-mysql/src/type_checking.rs (80%) rename sqlx-macros-core/src/database/postgres.rs => sqlx-postgres/src/type_checking.rs (93%) rename sqlx-macros-core/src/database/sqlite.rs => sqlx-sqlite/src/type_checking.rs (65%) diff --git a/sqlx-core/src/lib.rs b/sqlx-core/src/lib.rs index 468242e9e6..74fa36e574 100644 --- a/sqlx-core/src/lib.rs +++ b/sqlx-core/src/lib.rs @@ -77,6 +77,7 @@ pub mod query_scalar; pub mod row; pub mod rt; pub mod sync; +pub mod type_checking; pub mod type_info; pub mod value; diff --git a/sqlx-core/src/type_checking.rs b/sqlx-core/src/type_checking.rs new file mode 100644 index 0000000000..652139e9bf --- /dev/null +++ b/sqlx-core/src/type_checking.rs @@ -0,0 +1,188 @@ +use crate::database::Database; +use crate::decode::Decode; +use crate::type_info::TypeInfo; +use crate::value::Value; +use std::any::Any; +use std::fmt; +use std::fmt::{Debug, Formatter}; + +/// The type of query parameter checking done by a SQL database. +#[derive(PartialEq, Eq)] +pub enum ParamChecking { + /// Parameter checking is weak or nonexistent (uses coercion or allows mismatches). + Weak, + /// Parameter checking is strong (types must match exactly). + Strong, +} + +/// Type-checking extensions for the `Database` trait. +/// +/// Mostly supporting code for the macros, and for `Debug` impls. +pub trait TypeChecking: Database { + /// Describes how the database in question typechecks query parameters. + const PARAM_CHECKING: ParamChecking; + + /// Get the full path of the Rust type that corresponds to the given `TypeInfo`, if applicable. + /// + /// If the type has a borrowed equivalent suitable for query parameters, + /// this is that borrowed type. + fn param_type_for_id(id: &Self::TypeInfo) -> Option<&'static str>; + + /// Get the full path of the Rust type that corresponds to the given `TypeInfo`, if applicable. + /// + /// Always returns the owned version of the type, suitable for decoding from `Row`. + fn return_type_for_id(id: &Self::TypeInfo) -> Option<&'static str>; + + /// Get the name of the Cargo feature gate that must be enabled to process the given `TypeInfo`, + /// if applicable. + fn get_feature_gate(info: &Self::TypeInfo) -> Option<&'static str>; + + /// If `value` is a well-known type, decode and format it using `Debug`. + /// + /// If `value` is not a well-known type or could not be decoded, the reason is printed instead. + fn fmt_value_debug(value: &::Value) -> FmtValue<'_, Self>; +} + +/// An adapter for [`Value`] which attempts to decode the value and format it when printed using [`Debug`]. +pub struct FmtValue<'v, DB> +where + DB: Database, +{ + value: &'v ::Value, + fmt: fn(&'v ::Value, &mut Formatter<'_>) -> fmt::Result, +} + +impl<'v, DB> FmtValue<'v, DB> +where + DB: Database, +{ + // This API can't take `ValueRef` directly as it would need to pass it to `Decode` by-value, + // which means taking ownership of it. We cannot rely on a `Clone` impl because `SqliteValueRef` doesn't have one. + /// When printed with [`Debug`], attempt to decode `value` as the given type `T` and format it using [`Debug`]. + /// + /// If `value` could not be decoded as `T`, the reason is printed instead. + pub fn debug(value: &'v ::Value) -> Self + where + T: Decode<'v, DB> + Debug + Any, + { + Self { + value, + fmt: |value, f| { + let info = value.type_info(); + + match T::decode(value.as_ref()) { + Ok(value) => Debug::fmt(&value, f), + Err(e) => f.write_fmt(format_args!( + "(error decoding SQL type {} as {}: {e:?})", + info.name(), + std::any::type_name::() + )), + } + }, + } + } + + /// If the type to be decoded is not known or not supported, print the SQL type instead, + /// as well as any applicable SQLx feature that needs to be enabled. + pub fn unknown(value: &'v ::Value) -> Self + where + DB: TypeChecking, + { + Self { + value, + fmt: |value, f| { + let info = value.type_info(); + + if let Some(feature_gate) = ::get_feature_gate(&info) { + return f.write_fmt(format_args!( + "(unknown SQL type {}: SQLx feature {feature_gate} not enabled)", + info.name() + )); + } + + f.write_fmt(format_args!("(unknown SQL type {})", info.name())) + }, + } + } +} + +impl<'v, DB> Debug for FmtValue<'v, DB> +where + DB: Database, +{ + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + (self.fmt)(&self.value, f) + } +} + +#[doc(hidden)] +#[macro_export] +macro_rules! select_input_type { + ($ty:ty, $input:ty) => { + stringify!($input) + }; + ($ty:ty) => { + stringify!($ty) + }; +} + +#[macro_export] +macro_rules! impl_type_checking { + ( + $database:path { + $($(#[$meta:meta])? $ty:ty $(| $input:ty)?),*$(,)? + }, + ParamChecking::$param_checking:ident, + feature-types: $ty_info:ident => $get_gate:expr, + ) => { + impl $crate::type_checking::TypeChecking for $database { + const PARAM_CHECKING: $crate::type_checking::ParamChecking = $crate::type_checking::ParamChecking::$param_checking; + + fn param_type_for_id(info: &Self::TypeInfo) -> Option<&'static str> { + match () { + $( + $(#[$meta])? + _ if <$ty as sqlx_core::types::Type<$database>>::type_info() == *info => Some($crate::select_input_type!($ty $(, $input)?)), + )* + $( + $(#[$meta])? + _ if <$ty as sqlx_core::types::Type<$database>>::compatible(info) => Some(select_input_type!($ty $(, $input)?)), + )* + _ => None + } + } + + fn return_type_for_id(info: &Self::TypeInfo) -> Option<&'static str> { + match () { + $( + $(#[$meta])? + _ if <$ty as sqlx_core::types::Type<$database>>::type_info() == *info => Some(stringify!($ty)), + )* + $( + $(#[$meta])? + _ if <$ty as sqlx_core::types::Type<$database>>::compatible(info) => Some(stringify!($ty)), + )* + _ => None + } + } + + fn get_feature_gate($ty_info: &Self::TypeInfo) -> Option<&'static str> { + $get_gate + } + + fn fmt_value_debug(value: &Self::Value) -> $crate::type_checking::FmtValue { + use $crate::value::Value; + + let info = value.type_info(); + + match () { + $( + $(#[$meta])? + _ if <$ty as sqlx_core::types::Type<$database>>::compatible(&info) => $crate::type_checking::FmtValue::debug::<$ty>(value), + )* + _ => $crate::type_checking::FmtValue::unknown(value), + } + } + } + }; +} diff --git a/sqlx-macros-core/src/database/impls.rs b/sqlx-macros-core/src/database/impls.rs new file mode 100644 index 0000000000..35dc9393bc --- /dev/null +++ b/sqlx-macros-core/src/database/impls.rs @@ -0,0 +1,71 @@ +macro_rules! impl_database_ext { + ( + $database:path, + row: $row:path, + $(describe-blocking: $describe:path,)? + ) => { + impl $crate::database::DatabaseExt for $database { + const DATABASE_PATH: &'static str = stringify!($database); + const ROW_PATH: &'static str = stringify!($row); + impl_describe_blocking!($database, $($describe)?); + } + } +} + +macro_rules! impl_describe_blocking { + ($database:path $(,)?) => { + fn describe_blocking( + query: &str, + database_url: &str, + ) -> sqlx_core::Result> { + use $crate::database::CachingDescribeBlocking; + + // This can't be a provided method because the `static` can't reference `Self`. + static CACHE: CachingDescribeBlocking<$database> = CachingDescribeBlocking::new(); + + CACHE.describe(query, database_url) + } + }; + ($database:path, $describe:path) => { + fn describe_blocking( + query: &str, + database_url: &str, + ) -> sqlx_core::Result> { + $describe(query, database_url) + } + }; +} + +// The paths below will also be emitted from the macros, so they need to match the final facade. +mod sqlx { + #[cfg(feature = "mysql")] + pub use sqlx_mysql as mysql; + + #[cfg(feature = "postgres")] + pub use sqlx_postgres as postgres; + + #[cfg(feature = "sqlite")] + pub use sqlx_sqlite as sqlite; +} + +// NOTE: type mappings have been moved to `src/type_checking.rs` in their respective driver crates. +#[cfg(feature = "mysql")] +impl_database_ext! { + sqlx::mysql::MySql, + row: sqlx::mysql::MySqlRow, +} + +#[cfg(feature = "postgres")] +impl_database_ext! { + sqlx::postgres::Postgres, + row: sqlx::postgres::PgRow, +} + +#[cfg(feature = "sqlite")] +impl_database_ext! { + sqlx::sqlite::Sqlite, + row: sqlx::sqlite::SqliteRow, + // Since proc-macros don't benefit from async, we can make a describe call directly + // which also ensures that the database is closed afterwards, regardless of errors. + describe-blocking: sqlx_sqlite::describe_blocking, +} diff --git a/sqlx-macros-core/src/database/mod.rs b/sqlx-macros-core/src/database/mod.rs index c4fe696700..e5b886be7d 100644 --- a/sqlx-macros-core/src/database/mod.rs +++ b/sqlx-macros-core/src/database/mod.rs @@ -8,20 +8,15 @@ use sqlx_core::connection::Connection; use sqlx_core::database::Database; use sqlx_core::describe::Describe; use sqlx_core::executor::Executor; +use sqlx_core::type_checking::TypeChecking; -#[derive(PartialEq, Eq)] -#[allow(dead_code)] -pub enum ParamChecking { - Strong, - Weak, -} +#[cfg(any(feature = "postgres", feature = "mysql", feature = "sqlite"))] +mod impls; -pub trait DatabaseExt: Database { +pub trait DatabaseExt: Database + TypeChecking { const DATABASE_PATH: &'static str; const ROW_PATH: &'static str; - const PARAM_CHECKING: ParamChecking; - fn db_path() -> syn::Path { syn::parse_str(Self::DATABASE_PATH).unwrap() } @@ -30,12 +25,6 @@ pub trait DatabaseExt: Database { syn::parse_str(Self::ROW_PATH).unwrap() } - fn param_type_for_id(id: &Self::TypeInfo) -> Option<&'static str>; - - fn return_type_for_id(id: &Self::TypeInfo) -> Option<&'static str>; - - fn get_feature_gate(info: &Self::TypeInfo) -> Option<&'static str>; - fn describe_blocking(query: &str, database_url: &str) -> sqlx_core::Result>; } @@ -73,113 +62,3 @@ impl CachingDescribeBlocking { }) } } - -#[cfg(any(feature = "postgres", feature = "mysql", feature = "sqlite"))] -macro_rules! impl_database_ext { - ( - $database:path { - $($(#[$meta:meta])? $ty:ty $(| $input:ty)?),*$(,)? - }, - ParamChecking::$param_checking:ident, - feature-types: $ty_info:ident => $get_gate:expr, - row: $row:path, - $(describe-blocking: $describe:path,)? - ) => { - impl $crate::database::DatabaseExt for $database { - const DATABASE_PATH: &'static str = stringify!($database); - const ROW_PATH: &'static str = stringify!($row); - const PARAM_CHECKING: $crate::database::ParamChecking = $crate::database::ParamChecking::$param_checking; - - fn param_type_for_id(info: &Self::TypeInfo) -> Option<&'static str> { - match () { - $( - $(#[$meta])? - _ if <$ty as sqlx_core::types::Type<$database>>::type_info() == *info => Some(input_ty!($ty $(, $input)?)), - )* - $( - $(#[$meta])? - _ if <$ty as sqlx_core::types::Type<$database>>::compatible(info) => Some(input_ty!($ty $(, $input)?)), - )* - _ => None - } - } - - fn return_type_for_id(info: &Self::TypeInfo) -> Option<&'static str> { - match () { - $( - $(#[$meta])? - _ if <$ty as sqlx_core::types::Type<$database>>::type_info() == *info => return Some(stringify!($ty)), - )* - $( - $(#[$meta])? - _ if <$ty as sqlx_core::types::Type<$database>>::compatible(info) => return Some(stringify!($ty)), - )* - _ => None - } - } - - fn get_feature_gate($ty_info: &Self::TypeInfo) -> Option<&'static str> { - $get_gate - } - - impl_describe_blocking!($database, $($describe)?); - } - } -} - -#[cfg(any(feature = "postgres", feature = "mysql", feature = "sqlite"))] -macro_rules! impl_describe_blocking { - ($database:path $(,)?) => { - fn describe_blocking( - query: &str, - database_url: &str, - ) -> sqlx_core::Result> { - use $crate::database::CachingDescribeBlocking; - - // This can't be a provided method because the `static` can't reference `Self`. - static CACHE: CachingDescribeBlocking<$database> = CachingDescribeBlocking::new(); - - CACHE.describe(query, database_url) - } - }; - ($database:path, $describe:path) => { - fn describe_blocking( - query: &str, - database_url: &str, - ) -> sqlx_core::Result> { - $describe(query, database_url) - } - }; -} - -#[cfg(any(feature = "postgres", feature = "mysql", feature = "sqlite"))] -macro_rules! input_ty { - ($ty:ty, $input:ty) => { - stringify!($input) - }; - ($ty:ty) => { - stringify!($ty) - }; -} - -#[cfg(feature = "postgres")] -mod postgres; - -#[cfg(feature = "mysql")] -mod mysql; - -#[cfg(feature = "sqlite")] -mod sqlite; - -mod fake_sqlx { - pub use sqlx_core::*; - - #[cfg(feature = "mysql")] - pub use sqlx_mysql as mysql; - - #[cfg(feature = "postgres")] - pub use sqlx_postgres as postgres; - - #[cfg(feature = "sqlite")] - pub use sqlx_sqlite as sqlite; -} diff --git a/sqlx-macros-core/src/query/args.rs b/sqlx-macros-core/src/query/args.rs index 3a07b1b303..3b4bb8d6b2 100644 --- a/sqlx-macros-core/src/query/args.rs +++ b/sqlx-macros-core/src/query/args.rs @@ -58,7 +58,7 @@ pub fn quote_args( None => { DB::param_type_for_id(¶m_ty) .ok_or_else(|| { - if let Some(feature_gate) = ::get_feature_gate(¶m_ty) { + if let Some(feature_gate) = DB::get_feature_gate(¶m_ty) { format!( "optional sqlx feature `{}` required for type {} of param #{}", feature_gate, diff --git a/sqlx-macros-core/src/query/output.rs b/sqlx-macros-core/src/query/output.rs index 65f44216ad..905c90306f 100644 --- a/sqlx-macros-core/src/query/output.rs +++ b/sqlx-macros-core/src/query/output.rs @@ -8,6 +8,7 @@ use sqlx_core::describe::Describe; use crate::database::DatabaseExt; use crate::query::QueryMacroInput; +use sqlx_core::type_checking::TypeChecking; use std::fmt::{self, Display, Formatter}; use syn::parse::{Parse, ParseStream}; use syn::Token; @@ -222,10 +223,10 @@ pub fn quote_query_scalar( fn get_column_type(i: usize, column: &DB::Column) -> TokenStream { let type_info = &*column.type_info(); - ::return_type_for_id(&type_info).map_or_else( + ::return_type_for_id(&type_info).map_or_else( || { let message = - if let Some(feature_gate) = ::get_feature_gate(&type_info) { + if let Some(feature_gate) = ::get_feature_gate(&type_info) { format!( "optional sqlx feature `{feat}` required for type {ty} of {col}", ty = &type_info, diff --git a/sqlx-mysql/src/lib.rs b/sqlx-mysql/src/lib.rs index a8582f07eb..d77d0daf3b 100644 --- a/sqlx-mysql/src/lib.rs +++ b/sqlx-mysql/src/lib.rs @@ -23,6 +23,7 @@ mod query_result; mod row; mod statement; mod transaction; +mod type_checking; mod type_info; pub mod types; mod value; diff --git a/sqlx-macros-core/src/database/mysql.rs b/sqlx-mysql/src/type_checking.rs similarity index 80% rename from sqlx-macros-core/src/database/mysql.rs rename to sqlx-mysql/src/type_checking.rs index 3fbd0af495..3f3ce5833e 100644 --- a/sqlx-macros-core/src/database/mysql.rs +++ b/sqlx-mysql/src/type_checking.rs @@ -1,7 +1,12 @@ -use super::fake_sqlx as sqlx; +// Type mappings used by the macros and `Debug` impls. -impl_database_ext! { - sqlx::mysql::MySql { +#[allow(unused_imports)] +use sqlx_core as sqlx; + +use crate::MySql; + +impl_type_checking!( + MySql { u8, u16, u32, @@ -20,6 +25,8 @@ impl_database_ext! { // BINARY, VAR_BINARY, BLOB Vec, + // Types from third-party crates need to be referenced at a known path + // for the macros to work, but we don't want to require the user to add extra dependencies. #[cfg(all(feature = "chrono", not(feature = "time")))] sqlx::types::chrono::NaiveTime, @@ -55,5 +62,4 @@ impl_database_ext! { }, ParamChecking::Weak, feature-types: info => info.__type_feature_gate(), - row: sqlx::mysql::MySqlRow, -} +); diff --git a/sqlx-postgres/src/lib.rs b/sqlx-postgres/src/lib.rs index 5b9d5804b2..abdc064260 100644 --- a/sqlx-postgres/src/lib.rs +++ b/sqlx-postgres/src/lib.rs @@ -20,6 +20,7 @@ mod query_result; mod row; mod statement; mod transaction; +mod type_checking; mod type_info; pub mod types; mod value; diff --git a/sqlx-macros-core/src/database/postgres.rs b/sqlx-postgres/src/type_checking.rs similarity index 93% rename from sqlx-macros-core/src/database/postgres.rs rename to sqlx-postgres/src/type_checking.rs index 0bd6b4c856..6b5b65658d 100644 --- a/sqlx-macros-core/src/database/postgres.rs +++ b/sqlx-postgres/src/type_checking.rs @@ -1,7 +1,14 @@ -use super::fake_sqlx as sqlx; +use crate::Postgres; -impl_database_ext! { - sqlx::postgres::Postgres { +// The paths used below will also be emitted by the macros so they have to match the final facade. +#[allow(unused_imports, dead_code)] +mod sqlx { + pub use crate as postgres; + pub use sqlx_core::*; +} + +impl_type_checking!( + Postgres { (), bool, String | &str, @@ -183,10 +190,10 @@ impl_database_ext! { #[cfg(feature = "chrono")] Vec>> | - Vec>>, + &[sqlx::postgres::types::PgRange>], #[cfg(feature = "chrono")] - &[sqlx::postgres::types::PgRange>] | + Vec>> | &[sqlx::postgres::types::PgRange>], #[cfg(feature = "time")] @@ -203,5 +210,4 @@ impl_database_ext! { }, ParamChecking::Strong, feature-types: info => info.__type_feature_gate(), - row: sqlx::postgres::PgRow, -} +); diff --git a/sqlx-sqlite/src/lib.rs b/sqlx-sqlite/src/lib.rs index 2ddc576301..db09cc2f48 100644 --- a/sqlx-sqlite/src/lib.rs +++ b/sqlx-sqlite/src/lib.rs @@ -65,6 +65,7 @@ mod query_result; mod row; mod statement; mod transaction; +mod type_checking; mod type_info; pub mod types; mod value; diff --git a/sqlx-macros-core/src/database/sqlite.rs b/sqlx-sqlite/src/type_checking.rs similarity index 65% rename from sqlx-macros-core/src/database/sqlite.rs rename to sqlx-sqlite/src/type_checking.rs index 3c4c440219..c823ec795a 100644 --- a/sqlx-macros-core/src/database/sqlite.rs +++ b/sqlx-sqlite/src/type_checking.rs @@ -1,10 +1,13 @@ -use super::fake_sqlx as sqlx; +#[allow(unused_imports)] +use sqlx_core as sqlx; + +use crate::Sqlite; // f32 is not included below as REAL represents a floating point value -// stored as an 8-byte IEEE floating point number +// stored as an 8-byte IEEE floating point number (i.e. an f64) // For more info see: https://www.sqlite.org/datatype3.html#storage_classes_and_datatypes -impl_database_ext! { - sqlx::sqlite::Sqlite { +impl_type_checking!( + Sqlite { bool, i32, i64, @@ -34,9 +37,10 @@ impl_database_ext! { sqlx::types::Uuid, }, ParamChecking::Weak, + // While there are type integrations that must be enabled via Cargo feature, + // SQLite's type system doesn't actually have any type that we cannot decode by default. + // + // The type integrations simply allow the user to skip some intermediate representation, + // which is usually TEXT. feature-types: _info => None, - row: sqlx::sqlite::SqliteRow, - // Since proc-macros don't benefit from async, we can make a describe call directly - // which also ensures that the database is closed afterwards, regardless of errors. - describe-blocking: sqlx_sqlite::describe_blocking, -} +);