From 723023335d457d030465a15ea1f616a855320526 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sun, 1 Dec 2024 20:50:34 +0000 Subject: [PATCH 1/7] Insert many allow active models to have different column set --- Cargo.toml | 2 +- src/entity/base_entity.rs | 40 ++++++++++++ src/query/insert.rs | 99 ++++++++++++++++++++++++----- src/tests_cfg/cake_filling_price.rs | 2 +- 4 files changed, 126 insertions(+), 17 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index eafd5d9c8..692a1d2f2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ tracing = { version = "0.1", default-features = false, features = ["attributes", rust_decimal = { version = "1", default-features = false, optional = true } bigdecimal = { version = "0.4", default-features = false, optional = true } sea-orm-macros = { version = "~1.1.1", path = "sea-orm-macros", default-features = false, features = ["strum"] } -sea-query = { version = "0.32.0", default-features = false, features = ["thread-safe", "hashable-value", "backend-mysql", "backend-postgres", "backend-sqlite"] } +sea-query = { version = "0.32.1", default-features = false, features = ["thread-safe", "hashable-value", "backend-mysql", "backend-postgres", "backend-sqlite"] } sea-query-binder = { version = "0.7.0", default-features = false, optional = true } strum = { version = "0.26", default-features = false } serde = { version = "1.0", default-features = false } diff --git a/src/entity/base_entity.rs b/src/entity/base_entity.rs index 5ed5d7572..dea30d5b2 100644 --- a/src/entity/base_entity.rs +++ b/src/entity/base_entity.rs @@ -467,6 +467,46 @@ pub trait EntityTrait: EntityName { /// # Ok(()) /// # } /// ``` + /// + /// Before 1.1.2, if the active models have different column set, this method would panic. + /// Now, it'd attempt to fill in the missing columns with null + /// (which may or may not be correct, depending on whether the column is nullable): + /// + /// ``` + /// use sea_orm::{entity::*, query::*, tests_cfg::{cake, cake_filling}, DbBackend}; + /// + /// assert_eq!( + /// cake::Entity::insert_many([ + /// cake::ActiveModel { + /// id: NotSet, + /// name: Set("Apple Pie".to_owned()), + /// }, + /// cake::ActiveModel { + /// id: NotSet, + /// name: Set("Orange Scone".to_owned()), + /// } + /// ]) + /// .build(DbBackend::Postgres) + /// .to_string(), + /// r#"INSERT INTO "cake" ("name") VALUES ('Apple Pie'), ('Orange Scone')"#, + /// ); + /// + /// assert_eq!( + /// cake_filling::Entity::insert_many([ + /// cake_filling::ActiveModel { + /// cake_id: ActiveValue::set(2), + /// filling_id: ActiveValue::NotSet, + /// }, + /// cake_filling::ActiveModel { + /// cake_id: ActiveValue::NotSet, + /// filling_id: ActiveValue::set(3), + /// } + /// ]) + /// .build(DbBackend::Postgres) + /// .to_string(), + /// r#"INSERT INTO "cake_filling" ("cake_id", "filling_id") VALUES (2, NULL), (NULL, 3)"#, + /// ); + /// ``` fn insert_many(models: I) -> Insert where A: ActiveModelTrait, diff --git a/src/query/insert.rs b/src/query/insert.rs index b13cd8a43..5ba806374 100644 --- a/src/query/insert.rs +++ b/src/query/insert.rs @@ -3,7 +3,7 @@ use crate::{ PrimaryKeyTrait, QueryTrait, }; use core::marker::PhantomData; -use sea_query::{Expr, InsertStatement, OnConflict, ValueTuple}; +use sea_query::{Expr, InsertStatement, Keyword, OnConflict, SimpleExpr, Value, ValueTuple}; /// Performs INSERT operations on a ActiveModel #[derive(Debug)] @@ -155,9 +155,56 @@ where M: IntoActiveModel, I: IntoIterator, { + let mut columns: Vec<_> = ::Column::iter() + .map(|_| None) + .collect(); + let mut null_value: Vec> = + std::iter::repeat(None).take(columns.len()).collect(); + let mut all_values: Vec> = Vec::new(); + for model in models.into_iter() { - self = self.add(model); + let mut am: A = model.into_active_model(); + self.primary_key = + if !<::PrimaryKey as PrimaryKeyTrait>::auto_increment() { + am.get_primary_key_value() + } else { + None + }; + let mut values = Vec::with_capacity(columns.len()); + for (idx, col) in ::Column::iter().enumerate() { + let av = am.take(col); + match av { + ActiveValue::Set(value) | ActiveValue::Unchanged(value) => { + columns[idx] = Some(col); + null_value[idx] = Some(value.as_null()); + values.push(col.save_as(Expr::val(value))); + } + ActiveValue::NotSet => { + values.push(SimpleExpr::Keyword(Keyword::Null)); + } + } + } + all_values.push(values); } + + self.query + .columns(columns.iter().cloned().filter_map(|c| c)); + + for values in all_values { + self.query + .values_panic(values.into_iter().enumerate().filter_map(|(i, v)| { + if columns[i].is_some() { + if !matches!(v, SimpleExpr::Keyword(Keyword::Null)) { + Some(v) + } else { + null_value[i].clone().map(SimpleExpr::Value) + } + } else { + None + } + })); + } + self } @@ -393,8 +440,11 @@ where mod tests { use sea_query::OnConflict; - use crate::tests_cfg::cake::{self}; - use crate::{ActiveValue, DbBackend, DbErr, EntityTrait, Insert, IntoActiveModel, QueryTrait}; + use crate::tests_cfg::{cake, cake_filling}; + use crate::{ + ActiveValue, DbBackend, DbErr, EntityTrait, Insert, IntoActiveModel, NotSet, QueryTrait, + Set, + }; #[test] fn insert_1() { @@ -439,7 +489,7 @@ mod tests { } #[test] - fn insert_4() { + fn insert_many_1() { assert_eq!( Insert::::new() .add_many([ @@ -459,22 +509,41 @@ mod tests { } #[test] - #[should_panic(expected = "columns mismatch")] - fn insert_5() { - let apple = cake::ActiveModel { - name: ActiveValue::set("Apple".to_owned()), - ..Default::default() + fn insert_many_2() { + assert_eq!( + Insert::::new() + .add_many([ + cake::ActiveModel { + id: NotSet, + name: Set("Apple Pie".to_owned()), + }, + cake::ActiveModel { + id: NotSet, + name: Set("Orange Scone".to_owned()), + } + ]) + .build(DbBackend::Postgres) + .to_string(), + r#"INSERT INTO "cake" ("name") VALUES ('Apple Pie'), ('Orange Scone')"#, + ); + } + + #[test] + fn insert_many_3() { + let apple = cake_filling::ActiveModel { + cake_id: ActiveValue::set(2), + filling_id: ActiveValue::NotSet, }; - let orange = cake::ActiveModel { - id: ActiveValue::set(2), - name: ActiveValue::set("Orange".to_owned()), + let orange = cake_filling::ActiveModel { + cake_id: ActiveValue::NotSet, + filling_id: ActiveValue::set(3), }; assert_eq!( - Insert::::new() + Insert::::new() .add_many([apple, orange]) .build(DbBackend::Postgres) .to_string(), - r#"INSERT INTO "cake" ("id", "name") VALUES (NULL, 'Apple'), (2, 'Orange')"#, + r#"INSERT INTO "cake_filling" ("cake_id", "filling_id") VALUES (2, NULL), (NULL, 3)"#, ); } diff --git a/src/tests_cfg/cake_filling_price.rs b/src/tests_cfg/cake_filling_price.rs index 4d787a4ba..049ee7012 100644 --- a/src/tests_cfg/cake_filling_price.rs +++ b/src/tests_cfg/cake_filling_price.rs @@ -18,7 +18,7 @@ impl EntityName for Entity { pub struct Model { pub cake_id: i32, pub filling_id: i32, - #[cfg(feature = "with-decimal")] + #[cfg(feature = "with-rust_decimal")] pub price: Decimal, #[sea_orm(ignore)] pub ignored_attr: i32, From 79daaa4da4e3ac82c918ea298bfe11f86ab262c0 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sun, 1 Dec 2024 21:17:47 +0000 Subject: [PATCH 2/7] comment and fmt --- src/entity/base_entity.rs | 7 ++++++- src/query/insert.rs | 12 ++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/entity/base_entity.rs b/src/entity/base_entity.rs index dea30d5b2..d543cae92 100644 --- a/src/entity/base_entity.rs +++ b/src/entity/base_entity.rs @@ -473,7 +473,12 @@ pub trait EntityTrait: EntityName { /// (which may or may not be correct, depending on whether the column is nullable): /// /// ``` - /// use sea_orm::{entity::*, query::*, tests_cfg::{cake, cake_filling}, DbBackend}; + /// use sea_orm::{ + /// entity::*, + /// query::*, + /// tests_cfg::{cake, cake_filling}, + /// DbBackend, + /// }; /// /// assert_eq!( /// cake::Entity::insert_many([ diff --git a/src/query/insert.rs b/src/query/insert.rs index 5ba806374..4ba900316 100644 --- a/src/query/insert.rs +++ b/src/query/insert.rs @@ -175,18 +175,19 @@ where let av = am.take(col); match av { ActiveValue::Set(value) | ActiveValue::Unchanged(value) => { - columns[idx] = Some(col); - null_value[idx] = Some(value.as_null()); - values.push(col.save_as(Expr::val(value))); + columns[idx] = Some(col); // mark the column as used + null_value[idx] = Some(value.as_null()); // store the null value with the correct type + values.push(col.save_as(Expr::val(value))); // just like above } ActiveValue::NotSet => { - values.push(SimpleExpr::Keyword(Keyword::Null)); + values.push(SimpleExpr::Keyword(Keyword::Null)); // indicate a missing value } } } all_values.push(values); } + // filter only used column self.query .columns(columns.iter().cloned().filter_map(|c| c)); @@ -194,9 +195,12 @@ where self.query .values_panic(values.into_iter().enumerate().filter_map(|(i, v)| { if columns[i].is_some() { + // only if the column is used if !matches!(v, SimpleExpr::Keyword(Keyword::Null)) { + // use the value expression Some(v) } else { + // use null as standin, which must be Some null_value[i].clone().map(SimpleExpr::Value) } } else { From f7561c7ca7bd3bfa45df9235ecc445e75ad2d590 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sun, 1 Dec 2024 21:24:26 +0000 Subject: [PATCH 3/7] comment --- src/query/insert.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/query/insert.rs b/src/query/insert.rs index 4ba900316..125d1f4d8 100644 --- a/src/query/insert.rs +++ b/src/query/insert.rs @@ -177,7 +177,7 @@ where ActiveValue::Set(value) | ActiveValue::Unchanged(value) => { columns[idx] = Some(col); // mark the column as used null_value[idx] = Some(value.as_null()); // store the null value with the correct type - values.push(col.save_as(Expr::val(value))); // just like above + values.push(col.save_as(Expr::val(value))); // same as add() above } ActiveValue::NotSet => { values.push(SimpleExpr::Keyword(Keyword::Null)); // indicate a missing value @@ -192,6 +192,7 @@ where .columns(columns.iter().cloned().filter_map(|c| c)); for values in all_values { + // since we've aligned the column set, this never panics self.query .values_panic(values.into_iter().enumerate().filter_map(|(i, v)| { if columns[i].is_some() { From 0fa55e8fde3e013dd5498ae4ab4f83ccb4750447 Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Mon, 2 Dec 2024 16:27:03 +0800 Subject: [PATCH 4/7] clippy --- src/query/insert.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/query/insert.rs b/src/query/insert.rs index 125d1f4d8..4741a836a 100644 --- a/src/query/insert.rs +++ b/src/query/insert.rs @@ -188,8 +188,7 @@ where } // filter only used column - self.query - .columns(columns.iter().cloned().filter_map(|c| c)); + self.query.columns(columns.iter().cloned().flatten()); for values in all_values { // since we've aligned the column set, this never panics From 5c8b74d20ca6e363e4a79bcf0ba7e42f94bf2fec Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Mon, 2 Dec 2024 16:55:06 +0800 Subject: [PATCH 5/7] Fixup --- src/query/insert.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/query/insert.rs b/src/query/insert.rs index 4741a836a..8f109ddef 100644 --- a/src/query/insert.rs +++ b/src/query/insert.rs @@ -187,8 +187,15 @@ where all_values.push(values); } - // filter only used column - self.query.columns(columns.iter().cloned().flatten()); + if !all_values.is_empty() { + // filter only used column + self.query.columns(columns.iter().cloned().flatten()); + + // flag used column + for col in columns.iter() { + self.columns.push(col.is_some()); + } + } for values in all_values { // since we've aligned the column set, this never panics From dd07a04df4df4112e61b8b767cacfd2be7dba755 Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Mon, 2 Dec 2024 17:46:39 +0800 Subject: [PATCH 6/7] Refactor --- src/query/insert.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/query/insert.rs b/src/query/insert.rs index 8f109ddef..478fe8d0f 100644 --- a/src/query/insert.rs +++ b/src/query/insert.rs @@ -192,9 +192,7 @@ where self.query.columns(columns.iter().cloned().flatten()); // flag used column - for col in columns.iter() { - self.columns.push(col.is_some()); - } + self.columns = columns.iter().map(Option::is_some).collect(); } for values in all_values { From cd345512031ddf813361a0bd98501553f914b0f8 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Tue, 3 Dec 2024 21:45:02 +0000 Subject: [PATCH 7/7] Docs and restore old implementation --- src/entity/base_entity.rs | 2 +- src/query/insert.rs | 27 +++++++++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/entity/base_entity.rs b/src/entity/base_entity.rs index d543cae92..0f534f531 100644 --- a/src/entity/base_entity.rs +++ b/src/entity/base_entity.rs @@ -468,7 +468,7 @@ pub trait EntityTrait: EntityName { /// # } /// ``` /// - /// Before 1.1.2, if the active models have different column set, this method would panic. + /// Before 1.1.3, if the active models have different column set, this method would panic. /// Now, it'd attempt to fill in the missing columns with null /// (which may or may not be correct, depending on whether the column is nullable): /// diff --git a/src/query/insert.rs b/src/query/insert.rs index 478fe8d0f..502f9a9fd 100644 --- a/src/query/insert.rs +++ b/src/query/insert.rs @@ -112,7 +112,7 @@ where /// /// # Panics /// - /// Panics if the column value has discrepancy across rows + /// Panics if the rows have different column sets from what've previously been cached in the query statement #[allow(clippy::should_implement_trait)] pub fn add(mut self, m: M) -> Self where @@ -149,6 +149,26 @@ where self } + /// Add many Models to Self. This is the legacy implementation priori to `1.1.3`. + /// + /// # Panics + /// + /// Panics if the rows have different column sets + #[deprecated( + since = "1.1.3", + note = "Please use [`Insert::add_many`] which does not panic" + )] + pub fn add_multi(mut self, models: I) -> Self + where + M: IntoActiveModel, + I: IntoIterator, + { + for model in models.into_iter() { + self = self.add(model); + } + self + } + /// Add many Models to Self pub fn add_many(mut self, models: I) -> Self where @@ -265,8 +285,7 @@ where self } - /// Allow insert statement return safely if inserting nothing. - /// The database will not be affected. + /// Allow insert statement to return without error if nothing's been inserted pub fn do_nothing(self) -> TryInsert where A: ActiveModelTrait, @@ -274,7 +293,7 @@ where TryInsert::from_insert(self) } - /// alias to do_nothing + /// Alias to `do_nothing` pub fn on_empty_do_nothing(self) -> TryInsert where A: ActiveModelTrait,