diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2edccbc7a5c1..accf8f4eb483 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -472,12 +472,12 @@ jobs: cargo +stable -Z build-std test --manifest-path diesel/Cargo.toml --no-default-features --features "mysql extras __with_asan_tests" --target x86_64-unknown-linux-gnu minimal_rust_version: - name: Check Minimal supported rust version (1.82.0) + name: Check Minimal supported rust version (1.84.0) runs-on: ubuntu-latest needs: [rustfmt_and_clippy] steps: - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@1.82.0 + - uses: dtolnay/rust-toolchain@1.84.0 - uses: dtolnay/rust-toolchain@nightly - uses: taiki-e/install-action@cargo-hack - uses: taiki-e/install-action@cargo-minimal-versions @@ -490,12 +490,12 @@ jobs: sudo apt-get update sudo apt-get -y install libsqlite3-dev libpq-dev libmysqlclient-dev - name: Check diesel_derives - run: cargo +1.82.0 minimal-versions check -p diesel_derives --features "postgres sqlite mysql 32-column-tables 64-column-tables 128-column-tables without-deprecated with-deprecated r2d2" + run: cargo +1.84.0 minimal-versions check -p diesel_derives --features "postgres sqlite mysql 32-column-tables 64-column-tables 128-column-tables without-deprecated with-deprecated r2d2" - name: Check diesel - run: cargo +1.82.0 minimal-versions check -p diesel --features "postgres mysql sqlite extras" + run: cargo +1.84.0 minimal-versions check -p diesel --features "postgres mysql sqlite extras" - name: Check diesel_dynamic_schema - run: cargo +1.82.0 minimal-versions check -p diesel-dynamic-schema --all-features + run: cargo +1.84.0 minimal-versions check -p diesel-dynamic-schema --all-features - name: Check diesel_migrations - run: cargo +1.82.0 minimal-versions check -p diesel_migrations --all-features + run: cargo +1.84.0 minimal-versions check -p diesel_migrations --all-features - name: Check diesel_cli - run: cargo +1.82.0 minimal-versions check -p diesel_cli --features "default sqlite-bundled" + run: cargo +1.84.0 minimal-versions check -p diesel_cli --features "default sqlite-bundled" diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f8cce9d79da..f44753a61149 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ in a way that makes the pools suitable for use in parallel tests. * Add support for the `CAST` operator * Support `[print_schema] allow_tables_to_appear_in_same_query_config = "none"` to generate no `allow_tables_to_appear_in_same_query!` calls. (Default: `"all_tables"`.). ([#4333](https://github.com/diesel-rs/diesel/issues/4333)) * Add `[print_schema] pg_domains_as_custom_types` parameter to generate custom types for [PostgreSQL domains](https://www.postgresql.org/docs/current/domains.html) that matches any of the regexes in the given list. (Default: `[]`.) This option allows an application to selectively give special meaning for the serialization/deserialization of these types, avoiding the default behavior of treating the domain as the underlying type. ([#4592](https://github.com/diesel-rs/diesel/discussions/4592)) +* Add support for batch insert and upsert statements with returning for SQLite ### Fixed @@ -43,7 +44,7 @@ in a way that makes the pools suitable for use in parallel tests. ### Changed * Use distinct `DIESEL_LOG` logging filter env variable instead of the default `RUST_LOG` one (#4575) -* The minimal supported Rust version is now 1.82.0 +* The minimal supported Rust version is now 1.84.0 ## [2.2.2] 2024-07-19 diff --git a/Cargo.toml b/Cargo.toml index 34beec3fde0e..e2e238d28c1a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ members = [ ] [workspace.package] -rust-version = "1.82.0" +rust-version = "1.84.0" include = ["src/**/*.rs", "tests/**/*.rs", "LICENSE-*", "README.md"] edition = "2021" diff --git a/diesel/src/pg/value.rs b/diesel/src/pg/value.rs index 83c0751430d2..35bc810c4ff8 100644 --- a/diesel/src/pg/value.rs +++ b/diesel/src/pg/value.rs @@ -55,11 +55,7 @@ impl TypeOidLookup for NonZeroU32 { impl<'a> PgValue<'a> { #[cfg(test)] pub(crate) fn for_test(raw_value: &'a [u8]) -> Self { - #[allow(unsafe_code)] // that's actual safe - static FAKE_OID: NonZeroU32 = unsafe { - // 42 != 0, so this is actually safe - NonZeroU32::new_unchecked(42) - }; + static FAKE_OID: NonZeroU32 = NonZeroU32::new(42).unwrap(); Self { raw_value, type_oid_lookup: &FAKE_OID, diff --git a/diesel/src/query_builder/insert_statement/insert_with_default_for_sqlite.rs b/diesel/src/query_builder/insert_statement/insert_with_default_for_sqlite.rs index 218d8c08dfda..fc76166efa03 100644 --- a/diesel/src/query_builder/insert_statement/insert_with_default_for_sqlite.rs +++ b/diesel/src/query_builder/insert_statement/insert_with_default_for_sqlite.rs @@ -5,7 +5,7 @@ use crate::prelude::*; use crate::query_builder::upsert::on_conflict_clause::OnConflictValues; use crate::query_builder::{AstPass, QueryId, ValuesClause}; use crate::query_builder::{DebugQuery, QueryFragment}; -use crate::query_dsl::methods::ExecuteDsl; +use crate::query_dsl::{methods::ExecuteDsl, LoadQuery}; use crate::sqlite::Sqlite; use std::fmt::{self, Debug, Display}; @@ -273,6 +273,206 @@ where } } +impl<'query, V, T, QId, Op, O, U, B, const STATIC_QUERY_ID: bool> + LoadQuery<'query, SqliteConnection, U, B> + for InsertStatement>, T, QId, STATIC_QUERY_ID>, Op> +where + T: QuerySource, + V: ContainsDefaultableValue, + O: Default, + (O, Self): LoadQuery<'query, SqliteConnection, U, B>, +{ + type RowIter<'conn> = <(O, Self) as LoadQuery<'query, SqliteConnection, U, B>>::RowIter<'conn>; + + fn internal_load(self, conn: &mut SqliteConnection) -> QueryResult> { + <(O, Self) as LoadQuery<'query, SqliteConnection, U, B>>::internal_load( + (O::default(), self), + conn, + ) + } +} + +impl<'query, V, T, QId, Op, O, U, B, Target, ConflictOpt, const STATIC_QUERY_ID: bool> + LoadQuery<'query, SqliteConnection, U, B> + for InsertStatement< + T, + OnConflictValues< + BatchInsert>, T, QId, STATIC_QUERY_ID>, + Target, + ConflictOpt, + >, + Op, + > +where + T: QuerySource, + V: ContainsDefaultableValue, + O: Default, + (O, Self): LoadQuery<'query, SqliteConnection, U, B>, +{ + type RowIter<'conn> = <(O, Self) as LoadQuery<'query, SqliteConnection, U, B>>::RowIter<'conn>; + + fn internal_load(self, conn: &mut SqliteConnection) -> QueryResult> { + <(O, Self) as LoadQuery<'query, SqliteConnection, U, B>>::internal_load( + (O::default(), self), + conn, + ) + } +} + +impl RunQueryDsl + for ( + O, + InsertStatement>, T, QId, STATIC_QUERY_ID>, Op>, + ) +where + T: QuerySource, + V: ContainsDefaultableValue, + O: Default, + InsertStatement>, T, QId, STATIC_QUERY_ID>, Op>: + RunQueryDsl, +{ +} + +impl + RunQueryDsl + for ( + O, + InsertStatement< + T, + OnConflictValues< + BatchInsert>, T, QId, STATIC_QUERY_ID>, + Target, + ConflictOpt, + >, + Op, + >, + ) +where + T: QuerySource, + V: ContainsDefaultableValue, + O: Default, + InsertStatement< + T, + OnConflictValues< + BatchInsert>, T, QId, STATIC_QUERY_ID>, + Target, + ConflictOpt, + >, + Op, + >: RunQueryDsl, +{ +} + +impl<'query, V, T, QId, Op, U, B, const STATIC_QUERY_ID: bool> + LoadQuery<'query, SqliteConnection, U, B> + for ( + Yes, + InsertStatement>, T, QId, STATIC_QUERY_ID>, Op>, + ) +where + T: Table + Copy + QueryId + 'static, + Op: Copy + QueryId + QueryFragment, + InsertStatement, Op>: LoadQuery<'query, SqliteConnection, U, B>, + Self: RunQueryDsl, +{ + type RowIter<'conn> = std::vec::IntoIter>; + + fn internal_load(self, conn: &mut SqliteConnection) -> QueryResult> { + let (Yes, query) = self; + + conn.transaction(|conn| { + let mut results = Vec::with_capacity(query.records.values.len()); + + for record in query.records.values { + let stmt = + InsertStatement::new(query.target, record, query.operator, query.returning); + + let result = stmt + .internal_load(conn)? + .next() + .ok_or(crate::result::Error::NotFound)?; + + match &result { + Ok(_) | Err(crate::result::Error::DeserializationError(_)) => { + results.push(result) + } + Err(_) => { + result?; + } + }; + } + + Ok(results.into_iter()) + }) + } +} + +impl<'query, V, T, QId, Op, U, B, Target, ConflictOpt, const STATIC_QUERY_ID: bool> + LoadQuery<'query, SqliteConnection, U, B> + for ( + Yes, + InsertStatement< + T, + OnConflictValues< + BatchInsert>, T, QId, STATIC_QUERY_ID>, + Target, + ConflictOpt, + >, + Op, + >, + ) +where + T: Table + Copy + QueryId + 'static, + T::FromClause: Copy, + Op: Copy, + Target: Copy, + ConflictOpt: Copy, + InsertStatement, Target, ConflictOpt>, Op>: + LoadQuery<'query, SqliteConnection, U, B>, + Self: RunQueryDsl, +{ + type RowIter<'conn> = std::vec::IntoIter>; + + fn internal_load(self, conn: &mut SqliteConnection) -> QueryResult> { + let (Yes, query) = self; + + conn.transaction(|conn| { + let mut results = Vec::with_capacity(query.records.values.values.len()); + + for record in query.records.values.values { + let stmt = InsertStatement { + operator: query.operator, + target: query.target, + records: OnConflictValues { + values: record, + target: query.records.target, + action: query.records.action, + where_clause: query.records.where_clause, + }, + returning: query.returning, + into_clause: query.into_clause, + }; + + let result = stmt + .internal_load(conn)? + .next() + .ok_or(crate::result::Error::NotFound)?; + + match &result { + Ok(_) | Err(crate::result::Error::DeserializationError(_)) => { + results.push(result) + } + Err(_) => { + result?; + } + }; + } + + Ok(results.into_iter()) + }) + } +} + #[allow(missing_debug_implementations, missing_copy_implementations)] #[repr(transparent)] pub struct SqliteBatchInsertWrapper( @@ -400,6 +600,84 @@ where } } +impl<'query, V, T, QId, Op, U, B, const STATIC_QUERY_ID: bool> + LoadQuery<'query, SqliteConnection, U, B> + for ( + No, + InsertStatement, Op>, + ) +where + T: Table + QueryId + 'static, + InsertStatement, Op>: + LoadQuery<'query, SqliteConnection, U, B>, + Self: RunQueryDsl, +{ + type RowIter<'conn> = , + Op, + > as LoadQuery<'query, SqliteConnection, U, B>>::RowIter<'conn>; + + fn internal_load(self, conn: &mut SqliteConnection) -> QueryResult> { + let (No, query) = self; + + let query = InsertStatement { + records: SqliteBatchInsertWrapper(query.records), + operator: query.operator, + target: query.target, + returning: query.returning, + into_clause: query.into_clause, + }; + + query.internal_load(conn) + } +} + +impl<'query, V, T, QId, Op, U, B, Target, ConflictOpt, const STATIC_QUERY_ID: bool> + LoadQuery<'query, SqliteConnection, U, B> + for ( + No, + InsertStatement< + T, + OnConflictValues, Target, ConflictOpt>, + Op, + >, + ) +where + T: Table + QueryId + 'static, + InsertStatement< + T, + OnConflictValues, Target, ConflictOpt>, + Op, + >: LoadQuery<'query, SqliteConnection, U, B>, + Self: RunQueryDsl, +{ + type RowIter<'conn> = , Target, ConflictOpt>, + Op, + > as LoadQuery<'query, SqliteConnection, U, B>>::RowIter<'conn>; + + fn internal_load(self, conn: &mut SqliteConnection) -> QueryResult> { + let (No, query) = self; + + let query = InsertStatement { + operator: query.operator, + target: query.target, + records: OnConflictValues { + values: SqliteBatchInsertWrapper(query.records.values), + target: query.records.target, + action: query.records.action, + where_clause: query.records.where_clause, + }, + returning: query.returning, + into_clause: query.into_clause, + }; + + query.internal_load(conn) + } +} + macro_rules! tuple_impls { ($( $Tuple:tt { diff --git a/diesel_dynamic_schema/tests/dynamic_values.rs b/diesel_dynamic_schema/tests/dynamic_values.rs index 45aa57d23f60..19156a1bd90f 100644 --- a/diesel_dynamic_schema/tests/dynamic_values.rs +++ b/diesel_dynamic_schema/tests/dynamic_values.rs @@ -17,9 +17,9 @@ impl FromSql for MyDynamicValue { use diesel::pg::Pg; use std::num::NonZeroU32; - const VARCHAR_OID: NonZeroU32 = unsafe { NonZeroU32::new_unchecked(1043) }; - const TEXT_OID: NonZeroU32 = unsafe { NonZeroU32::new_unchecked(25) }; - const INTEGER_OID: NonZeroU32 = unsafe { NonZeroU32::new_unchecked(23) }; + const VARCHAR_OID: NonZeroU32 = NonZeroU32::new(1043).unwrap(); + const TEXT_OID: NonZeroU32 = NonZeroU32::new(25).unwrap(); + const INTEGER_OID: NonZeroU32 = NonZeroU32::new(23).unwrap(); match value.get_oid() { VARCHAR_OID | TEXT_OID => { diff --git a/diesel_tests/tests/insert.rs b/diesel_tests/tests/insert.rs index 6abbad4a2d95..4d55caf5d519 100644 --- a/diesel_tests/tests/insert.rs +++ b/diesel_tests/tests/insert.rs @@ -219,7 +219,10 @@ fn insert_record_attached_database_using_returning_clause() { } #[diesel_test_helper::test] -#[cfg(not(any(feature = "sqlite", feature = "mysql")))] +#[cfg(not(any( + not(all(feature = "sqlite", feature = "returning_clauses_for_sqlite_3_35")), + feature = "mysql" +)))] fn insert_records_using_returning_clause() { use crate::schema::users::table as users; let connection = &mut connection(); @@ -829,6 +832,52 @@ fn batch_insert_is_atomic_on_sqlite() { assert_eq!(Ok(0), users.count().get_result(connection)); } +#[diesel_test_helper::test] +#[cfg(all(feature = "sqlite", feature = "returning_clauses_for_sqlite_3_35"))] +fn batch_insert_with_returning_is_atomic_on_sqlite() { + use crate::schema::users::dsl::*; + let connection = &mut connection(); + + let new_users = vec![(id.eq(1), name.eq("Sean")), (id.eq(1), name.eq("Tess"))]; + let result = insert_into(users) + .values(&new_users) + .get_results::(connection); + assert!(result.is_err()); + + assert_eq!(Ok(0), users.count().get_result(connection)); +} + +#[diesel_test_helper::test] +#[cfg(all(feature = "sqlite", feature = "returning_clauses_for_sqlite_3_35"))] +fn batch_insert_with_defaultables_and_returning_is_atomic_on_sqlite() { + use crate::schema::users::dsl::*; + let connection = &mut connection(); + + let new_users = vec![Some(name.eq("Sean")), None]; + let result = insert_into(users) + .values(&new_users) + .get_results::(connection); + assert!(result.is_err()); + + assert_eq!(Ok(0), users.count().get_result(connection)); +} + +#[diesel_test_helper::test] +#[cfg(all(feature = "sqlite", feature = "returning_clauses_for_sqlite_3_35"))] +fn batch_upsert_with_defaultables_and_returning_is_atomic_on_sqlite() { + use crate::schema::users::dsl::*; + let connection = &mut connection(); + + let new_users = vec![Some(name.eq("Sean")), None]; + let result = insert_into(users) + .values(&new_users) + .on_conflict_do_nothing() + .get_results::(connection); + assert!(result.is_err()); + + assert_eq!(Ok(0), users.count().get_result(connection)); +} + // regression test for https://github.com/diesel-rs/diesel/issues/2898 #[diesel_test_helper::test] fn mixed_defaultable_insert() { @@ -969,3 +1018,47 @@ fn batch_upsert_non_default_values() { ]; assert_eq!(users, expected); } + +#[diesel_test_helper::test] +#[cfg(any( + feature = "postgres", + all(feature = "sqlite", feature = "returning_clauses_for_sqlite_3_35") +))] +fn batch_upsert_with_returning() { + use crate::schema::users; + let conn = &mut connection_with_sean_and_tess_in_users_table(); + + let inserted_users = diesel::insert_into(users::table) + .values([ + ( + users::id.eq(1), + users::name.eq("Sean"), + users::hair_color.eq("black"), + ), + ( + users::id.eq(2), + users::name.eq("Tess"), + users::hair_color.eq("blue"), + ), + ]) + .on_conflict(users::id) + .do_update() + .set(users::hair_color.eq(diesel::upsert::excluded(users::hair_color))) + .get_results::(conn) + .unwrap(); + + let expected_users = vec![ + User { + id: 1, + name: "Sean".to_string(), + hair_color: Some("black".to_string()), + }, + User { + id: 2, + name: "Tess".to_string(), + hair_color: Some("blue".to_string()), + }, + ]; + + assert_eq!(inserted_users, expected_users); +} diff --git a/examples/sqlite/all_about_inserts/Cargo.toml b/examples/sqlite/all_about_inserts/Cargo.toml index d3a9bb7ce26b..8a43d17ef344 100644 --- a/examples/sqlite/all_about_inserts/Cargo.toml +++ b/examples/sqlite/all_about_inserts/Cargo.toml @@ -5,7 +5,7 @@ edition.workspace = true publish = false [dependencies] -diesel = { version = "2.2.0", path = "../../../diesel", features = ["sqlite", "chrono"] } +diesel = { version = "2.2.0", path = "../../../diesel", features = ["sqlite", "chrono", "returning_clauses_for_sqlite_3_35"] } serde = { version = "1.0.130", features = ["derive"] } serde_json = "1.0.68" chrono = { version = "0.4.20", default-features = false, features = ["clock", "std"] } diff --git a/examples/sqlite/all_about_inserts/src/lib.rs b/examples/sqlite/all_about_inserts/src/lib.rs index 2e6c9b508d0c..b667a5593081 100644 --- a/examples/sqlite/all_about_inserts/src/lib.rs +++ b/examples/sqlite/all_about_inserts/src/lib.rs @@ -143,16 +143,13 @@ pub fn insert_single_column_batch(conn: &mut SqliteConnection) -> QueryResult(&query).to_string()); + let values = vec![name.eq("Sean"), name.eq("Tess")]; + let query = insert_into(users).values(&values); + let sql = "INSERT INTO `users` (`name`) VALUES (?), (?) \ + -- binds: [\"Sean\", \"Tess\"]"; + assert_eq!(sql, debug_query::(&query).to_string()); } pub fn insert_single_column_batch_with_default(conn: &mut SqliteConnection) -> QueryResult { @@ -190,20 +187,17 @@ pub fn insert_tuple_batch(conn: &mut SqliteConnection) -> QueryResult { #[test] fn examine_sql_from_insert_tuple_batch() { - // Sorry, we can't inspect this as a single query on SQLite. - // You can loop over the values to see each individual insert statement. - // - // use schema::users::dsl::*; + use schema::users::dsl::*; - // let values = vec![ - // (name.eq("Sean"), hair_color.eq("Black")), - // (name.eq("Tess"), hair_color.eq("Brown")), - // ]; - // let query = insert_into(users).values(&values); - // let sql = "INSERT INTO `users` (`name`, `hair_color`) \ - // VALUES (?, ?), (?, ?) \ - // -- binds: [\"Sean\", \"Black\", \"Tess\", \"Brown\"]"; - // assert_eq!(sql, debug_query::(&query).to_string()); + let values = vec![ + (name.eq("Sean"), hair_color.eq("Black")), + (name.eq("Tess"), hair_color.eq("Brown")), + ]; + let query = insert_into(users).values(&values); + let sql = "INSERT INTO `users` (`name`, `hair_color`) \ + VALUES (?, ?), (?, ?) \ + -- binds: [\"Sean\", \"Black\", \"Tess\", \"Brown\"]"; + assert_eq!(sql, debug_query::(&query).to_string()); } pub fn insert_tuple_batch_with_default(conn: &mut SqliteConnection) -> QueryResult { @@ -279,22 +273,12 @@ fn insert_get_results_batch() { let now = select(diesel::dsl::now).get_result::(conn)?; - let inserted_users = conn.transaction::<_, Error, _>(|conn| { - let inserted_count = insert_into(users) - .values(&vec![ - (id.eq(1), name.eq("Sean")), - (id.eq(2), name.eq("Tess")), - ]) - .execute(conn)?; - - Ok(users - .order(id.desc()) - .limit(inserted_count as i64) - .load(conn)? - .into_iter() - .rev() - .collect::>()) - })?; + let inserted_users = insert_into(users) + .values(&vec![ + (id.eq(1), name.eq("Sean")), + (id.eq(2), name.eq("Tess")), + ]) + .get_results(conn)?; let expected_users = vec![ User { @@ -318,18 +302,59 @@ fn insert_get_results_batch() { }); } +#[test] +fn insert_insertable_struct_get_results_batch() { + use diesel::result::Error; + + let conn = &mut establish_connection(); + conn.test_transaction::<_, Error, _>(|conn| { + use diesel::select; + use schema::users::dsl::*; + + let now = select(diesel::dsl::now).get_result::(conn)?; + + let json = r#"[ + { "name": "Sean", "hair_color": "Black" }, + { "name": "Tess", "hair_color": "Brown" } + ]"#; + let user_form = serde_json::from_str::>(json).unwrap(); + + let inserted_users = insert_into(users).values(&user_form).get_results(conn)?; + + let expected_users = vec![ + User { + id: 1, + name: "Sean".into(), + hair_color: Some("Black".into()), + created_at: now, + updated_at: now, + }, + User { + id: 2, + name: "Tess".into(), + hair_color: Some("Brown".into()), + created_at: now, + updated_at: now, + }, + ]; + assert_eq!(expected_users, inserted_users); + + Ok(()) + }); +} + #[test] fn examine_sql_from_insert_get_results_batch() { use schema::users::dsl::*; - // Sorry, we can't inspect this as a single query on SQLite. - // You can loop over the values to see each individual insert statement. - // - // let values = vec![(id.eq(1), name.eq("Sean")), (id.eq(2), name.eq("Tess"))]; - // let insert_query = insert_into(users).values(&values); - // let insert_sql = "INSERT INTO `users` (`id`, `name`) VALUES (?, ?), (?, ?) \ - // -- binds: [1, \"Sean\", 2, \"Tess\"]"; - // assert_eq!(insert_sql, debug_query::(&insert_query).to_string()); + let values = vec![(id.eq(1), name.eq("Sean")), (id.eq(2), name.eq("Tess"))]; + let insert_query = insert_into(users).values(&values); + let insert_sql = "INSERT INTO `users` (`id`, `name`) VALUES (?, ?), (?, ?) \ + -- binds: [1, \"Sean\", 2, \"Tess\"]"; + assert_eq!( + insert_sql, + debug_query::(&insert_query).to_string() + ); let load_query = users.order(id.desc()); let load_sql = "SELECT `users`.`id`, `users`.`name`, \ `users`.`hair_color`, `users`.`created_at`, \ @@ -351,13 +376,9 @@ fn insert_get_result() { let now = select(diesel::dsl::now).get_result::(conn)?; - let inserted_user = conn.transaction::<_, Error, _>(|conn| { - insert_into(users) - .values((id.eq(3), name.eq("Ruby"))) - .execute(conn)?; - - users.order(id.desc()).first(conn) - })?; + let inserted_user = insert_into(users) + .values((id.eq(3), name.eq("Ruby"))) + .get_result(conn)?; let expected_user = User { id: 3,