From 1d66cdfb8323fc09612a08dca10752b47238181a Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Wed, 16 Aug 2023 16:00:50 +0200 Subject: [PATCH] Fix #3748 We now also generate the necessary `FromSql`/`ToSql` impls for `chrono`/`time` types for `Time`/`Date`/`Timestamp`. This follows the same reasoning as ##3747. --- .github/workflows/ci.yml | 2 +- diesel/Cargo.toml | 2 + diesel/src/internal/derives.rs | 8 + diesel_derives/Cargo.toml | 2 + diesel_derives/src/multiconnection.rs | 72 ++++- diesel_derives/tests/multiconnection.rs | 346 ++++++++++++++++++++++-- 6 files changed, 398 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 322d62d727c7..051f715a1e2e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -221,7 +221,7 @@ jobs: - name: Test diesel-derives (nightly) if: matrix.rust == 'nightly' shell: bash - run: cargo +${{ matrix.rust }} test --manifest-path diesel_derives/Cargo.toml --no-default-features --features "diesel/${{ matrix.backend }} diesel/unstable" + run: cargo +${{ matrix.rust }} test --manifest-path diesel_derives/Cargo.toml --no-default-features --features "diesel/${{ matrix.backend }} diesel/unstable diesel/time time diesel/chrono chrono" - name: Test diesel-derives shell: bash diff --git a/diesel/Cargo.toml b/diesel/Cargo.toml index ab0104486f3f..c66d2229e2aa 100644 --- a/diesel/Cargo.toml +++ b/diesel/Cargo.toml @@ -68,6 +68,8 @@ returning_clauses_for_sqlite_3_35 = [] i-implement-a-third-party-backend-and-opt-into-breaking-changes = [] nightly-error-messages = [] r2d2 = ["diesel_derives/r2d2", "dep:r2d2"] +chrono = ["diesel_derives/chrono", "dep:chrono"] +time = ["diesel_derives/time", "dep:time"] [package.metadata.docs.rs] features = ["postgres", "mysql", "sqlite", "extras"] diff --git a/diesel/src/internal/derives.rs b/diesel/src/internal/derives.rs index be8929c8e486..216936cf6ac0 100644 --- a/diesel/src/internal/derives.rs +++ b/diesel/src/internal/derives.rs @@ -69,4 +69,12 @@ pub mod multiconnection { #[doc(hidden)] pub use crate::query_builder::select_statement::SelectStatementAccessor; + + #[doc(hidden)] + #[cfg(feature = "chrono")] + pub use chrono; + + #[doc(hidden)] + #[cfg(feature = "time")] + pub use time; } diff --git a/diesel_derives/Cargo.toml b/diesel_derives/Cargo.toml index 489592ad7f52..2fcf6f4425f1 100644 --- a/diesel_derives/Cargo.toml +++ b/diesel_derives/Cargo.toml @@ -44,3 +44,5 @@ mysql = [] without-deprecated = [] with-deprecated = [] r2d2 = [] +chrono = [] +time = [] diff --git a/diesel_derives/src/multiconnection.rs b/diesel_derives/src/multiconnection.rs index 1ec5c623c24d..d32c142c53c2 100644 --- a/diesel_derives/src/multiconnection.rs +++ b/diesel_derives/src/multiconnection.rs @@ -565,7 +565,7 @@ fn generate_row(connection_types: &[ConnectionVariant]) -> TokenStream { } fn generate_bind_collector(connection_types: &[ConnectionVariant]) -> TokenStream { - let to_sql_impls = vec![ + let mut to_sql_impls = vec![ ( quote::quote!(diesel::sql_types::SmallInt), quote::quote!(i16), @@ -583,11 +583,40 @@ fn generate_bind_collector(connection_types: &[ConnectionVariant]) -> TokenStrea quote::quote!([u8]), ), (quote::quote!(diesel::sql_types::Bool), quote::quote!(bool)), - ] - .into_iter() - .map(|t| generate_to_sql_impls(t, connection_types)); + ]; + if cfg!(feature = "chrono") { + to_sql_impls.push(( + quote::quote!(diesel::sql_types::Timestamp), + quote::quote!(diesel::internal::derives::multiconnection::chrono::NaiveDateTime), + )); + to_sql_impls.push(( + quote::quote!(diesel::sql_types::Date), + quote::quote!(diesel::internal::derives::multiconnection::chrono::NaiveDate), + )); + to_sql_impls.push(( + quote::quote!(diesel::sql_types::Time), + quote::quote!(diesel::internal::derives::multiconnection::chrono::NaiveTime), + )); + } + if cfg!(feature = "time") { + to_sql_impls.push(( + quote::quote!(diesel::sql_types::Timestamp), + quote::quote!(diesel::internal::derives::multiconnection::time::PrimitiveDateTime), + )); + to_sql_impls.push(( + quote::quote!(diesel::sql_types::Time), + quote::quote!(diesel::internal::derives::multiconnection::time::Time), + )); + to_sql_impls.push(( + quote::quote!(diesel::sql_types::Date), + quote::quote!(diesel::internal::derives::multiconnection::time::Date), + )); + } + let to_sql_impls = to_sql_impls + .into_iter() + .map(|t| generate_to_sql_impls(t, connection_types)); - let from_sql_impls = vec![ + let mut from_sql_impls = vec![ ( quote::quote!(diesel::sql_types::SmallInt), quote::quote!(i16), @@ -608,9 +637,36 @@ fn generate_bind_collector(connection_types: &[ConnectionVariant]) -> TokenStrea quote::quote!(Vec), ), (quote::quote!(diesel::sql_types::Bool), quote::quote!(bool)), - ] - .into_iter() - .map(generate_from_sql_impls); + ]; + if cfg!(feature = "chrono") { + from_sql_impls.push(( + quote::quote!(diesel::sql_types::Timestamp), + quote::quote!(diesel::internal::derives::multiconnection::chrono::NaiveDateTime), + )); + from_sql_impls.push(( + quote::quote!(diesel::sql_types::Date), + quote::quote!(diesel::internal::derives::multiconnection::chrono::NaiveDate), + )); + from_sql_impls.push(( + quote::quote!(diesel::sql_types::Time), + quote::quote!(diesel::internal::derives::multiconnection::chrono::NaiveTime), + )); + } + if cfg!(feature = "time") { + from_sql_impls.push(( + quote::quote!(diesel::sql_types::Timestamp), + quote::quote!(diesel::internal::derives::multiconnection::time::PrimitiveDateTime), + )); + from_sql_impls.push(( + quote::quote!(diesel::sql_types::Time), + quote::quote!(diesel::internal::derives::multiconnection::time::Time), + )); + from_sql_impls.push(( + quote::quote!(diesel::sql_types::Date), + quote::quote!(diesel::internal::derives::multiconnection::time::Date), + )); + } + let from_sql_impls = from_sql_impls.into_iter().map(generate_from_sql_impls); let into_bind_value_bounds = connection_types.iter().map(|c| { let ty = c.ty; diff --git a/diesel_derives/tests/multiconnection.rs b/diesel_derives/tests/multiconnection.rs index 87537d039c13..51c93836d116 100644 --- a/diesel_derives/tests/multiconnection.rs +++ b/diesel_derives/tests/multiconnection.rs @@ -1,6 +1,5 @@ use crate::schema::users; use diesel::prelude::*; -use diesel::sql_types::{Binary, Float, Integer, Nullable, Text}; #[derive(diesel::MultiConnection)] pub enum InferConnection { @@ -20,16 +19,7 @@ pub struct User { #[test] fn check_queries_work() { - let database_url = if cfg!(feature = "mysql") { - dotenvy::var("MYSQL_UNIT_TEST_DATABASE_URL").or_else(|_| dotenvy::var("DATABASE_URL")) - } else if cfg!(feature = "postgres") { - dotenvy::var("PG_DATABASE_URL").or_else(|_| dotenvy::var("DATABASE_URL")) - } else { - Ok(dotenvy::var("DATABASE_URL").unwrap_or_else(|_| ":memory:".to_owned())) - }; - let database_url = database_url.expect("DATABASE_URL must be set in order to run tests"); - - let mut conn = InferConnection::establish(&database_url).unwrap(); + let mut conn = establish_connection(); diesel::sql_query( "CREATE TEMPORARY TABLE users(\ @@ -106,19 +96,325 @@ fn check_queries_work() { // delete diesel::delete(users::table).execute(&mut conn).unwrap(); +} - // more binds - // mostly nullable types - let (string, int, blob, float) = diesel::select(( - None::.into_sql::>(), - None::.into_sql::>(), - None::.into_sql::>(), - None::>.into_sql::>(), - )) - .get_result::<(Option, Option, Option, Option>)>(&mut conn) - .unwrap(); - assert!(string.is_none()); - assert!(int.is_none()); - assert!(float.is_none()); - assert!(blob.is_none()); +fn establish_connection() -> InferConnection { + let database_url = if cfg!(feature = "mysql") { + dotenvy::var("MYSQL_UNIT_TEST_DATABASE_URL").or_else(|_| dotenvy::var("DATABASE_URL")) + } else if cfg!(feature = "postgres") { + dotenvy::var("PG_DATABASE_URL").or_else(|_| dotenvy::var("DATABASE_URL")) + } else { + Ok(dotenvy::var("DATABASE_URL").unwrap_or_else(|_| ":memory:".to_owned())) + }; + let database_url = database_url.expect("DATABASE_URL must be set in order to run tests"); + + InferConnection::establish(&database_url).unwrap() +} + +#[cfg(all(feature = "chrono", feature = "time"))] +fn make_test_table(conn: &mut InferConnection) { + match conn { + #[cfg(feature = "postgres")] + InferConnection::Pg(ref mut conn) => { + diesel::sql_query( + "CREATE TEMPORARY TABLE type_test( \ + small_int SMALLINT,\ + integer INTEGER,\ + big_int BIGINT,\ + float FLOAT4,\ + double FLOAT8,\ + string TEXT,\ + blob BYTEA,\ + timestamp1 TIMESTAMP,\ + date1 DATE,\ + time1 TIME,\ + timestamp2 TIMESTAMP,\ + date2 DATE,\ + time2 TIME + )", + ) + .execute(conn) + .unwrap(); + } + #[cfg(feature = "sqlite")] + InferConnection::Sqlite(ref mut conn) => { + diesel::sql_query( + "CREATE TEMPORARY TABLE type_test( \ + small_int SMALLINT,\ + integer INTEGER,\ + big_int BIGINT,\ + float FLOAT4,\ + double FLOAT8,\ + string TEXT,\ + blob BLOB,\ + timestamp1 TIMESTAMP,\ + date1 DATE,\ + time1 TIME,\ + timestamp2 TIMESTAMP,\ + date2 DATE,\ + time2 TIME + )", + ) + .execute(conn) + .unwrap(); + } + #[cfg(feature = "mysql")] + InferConnection::Mysql(ref mut conn) => { + diesel::sql_query( + "CREATE TEMPORARY TABLE type_test( \ + `small_int` SMALLINT,\ + `integer` INT,\ + `big_int` BIGINT,\ + `float` FLOAT,\ + `double` DOUBLE,\ + `string` TEXT,\ + `blob` BLOB,\ + `timestamp1` DATETIME, + `date1` DATE,\ + `time1` TIME,\ + `timestamp2` DATETIME, + `date2` DATE,\ + `time2` TIME + )", + ) + .execute(conn) + .unwrap(); + } + } +} + +#[cfg(all(feature = "chrono", feature = "time"))] +#[test] +fn type_checks() { + use diesel::internal::derives::multiconnection::{chrono, time}; + + table! { + type_test(integer) { + small_int -> SmallInt, + integer -> Integer, + big_int -> BigInt, + float -> Float, + double -> Double, + string -> Text, + blob -> Blob, + timestamp1 -> Timestamp, + time1 -> Time, + date1 -> Date, + timestamp2 -> Timestamp, + time2 -> Time, + date2 -> Date, + } + } + + let mut conn = establish_connection(); + make_test_table(&mut conn); + conn.begin_test_transaction().unwrap(); + let small_int = 1_i16; + let integer = 2_i32; + let big_int = 3_i64; + let float = 4.0_f32; + let double = 5.0_f64; + let string = String::from("bar"); + let blob = vec![1_u8, 2, 3, 4]; + let date1 = chrono::NaiveDate::from_ymd_opt(2023, 08, 17).unwrap(); + let time1 = chrono::NaiveTime::from_hms_opt(07, 50, 12).unwrap(); + let timestamp1 = chrono::NaiveDateTime::new(date1, time1); + let time2 = time::Time::from_hms(12, 22, 23).unwrap(); + let date2 = time::Date::from_calendar_date(2023, time::Month::August, 26).unwrap(); + let timestamp2 = time::PrimitiveDateTime::new(date2, time2); + + diesel::insert_into(type_test::table) + .values(( + type_test::small_int.eq(small_int), + type_test::integer.eq(integer), + type_test::big_int.eq(big_int), + type_test::float.eq(float), + type_test::double.eq(double), + type_test::string.eq(&string), + type_test::blob.eq(&blob), + type_test::timestamp1.eq(timestamp1), + type_test::time1.eq(time1), + type_test::date1.eq(date1), + type_test::timestamp2.eq(timestamp2), + type_test::time2.eq(time2), + type_test::date2.eq(date2), + )) + .execute(&mut conn) + .unwrap(); + + let result = type_test::table + .get_result::<( + i16, //0 + i32, //1 + i64, //2 + f32, //3 + f64, //4 + String, //5 + Vec, //6 + chrono::NaiveDateTime, //7 + chrono::NaiveTime, //8 + chrono::NaiveDate, //9 + time::PrimitiveDateTime, //10 + time::Time, //11 + time::Date, //12 + )>(&mut conn) + .unwrap(); + + assert_eq!(small_int, result.0); + assert_eq!(integer, result.1); + assert_eq!(big_int, result.2); + assert_eq!(float, result.3); + assert_eq!(double, result.4); + assert_eq!(string, result.5); + assert_eq!(blob, result.6); + assert_eq!(timestamp1, result.7); + assert_eq!(time1, result.8); + assert_eq!(date1, result.9); + assert_eq!(timestamp2, result.10); + assert_eq!(time2, result.11); + assert_eq!(date2, result.12); +} + +#[cfg(all(feature = "chrono", feature = "time"))] +#[test] +fn nullable_type_checks() { + use diesel::internal::derives::multiconnection::{chrono, time}; + + table! { + type_test(integer) { + small_int -> Nullable, + integer -> Nullable, + big_int -> Nullable, + float -> Nullable, + double -> Nullable, + string -> Nullable, + blob -> Nullable, + timestamp1 -> Nullable, + time1 -> Nullable