From 532b20e68b4b6a947b6c981483753db7b33f1499 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Sun, 14 Jan 2024 03:17:00 +0800 Subject: [PATCH 1/2] Improve DataObject.save() generation --- butane/tests/basic.rs | 21 ++++++++++++++++ butane_core/src/codegen/dbobj.rs | 42 ++++++++++++++++++++------------ 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/butane/tests/basic.rs b/butane/tests/basic.rs index 74ae3af9..9c6aa723 100644 --- a/butane/tests/basic.rs +++ b/butane/tests/basic.rs @@ -74,6 +74,12 @@ impl HasOnlyPk { } } +#[model] +#[derive(Default)] +struct HasOnlyAutoPk { + id: AutoPk, +} + #[model] #[derive(Debug, Default, PartialEq, Clone)] pub struct SelfReferential { @@ -235,12 +241,27 @@ testall!(auto_pk); fn only_pk(conn: Connection) { let mut obj = HasOnlyPk::new(1); obj.save(&conn).unwrap(); + assert_eq!(obj.id, 1); // verify we can still save the object even though it has no // fields to modify obj.save(&conn).unwrap(); + // verify it didnt get a new id + assert_eq!(obj.id, 1); } testall!(only_pk); +fn only_auto_pk(conn: Connection) { + let mut obj = HasOnlyAutoPk::default(); + obj.save(&conn).unwrap(); + let pk = obj.id; + // verify we can still save the object even though it has no + // fields to modify + obj.save(&conn).unwrap(); + // verify it didnt get a new id + assert_eq!(obj.id, pk); +} +testall!(only_auto_pk); + fn basic_committed_transaction(mut conn: Connection) { let tr = conn.transaction().unwrap(); diff --git a/butane_core/src/codegen/dbobj.rs b/butane_core/src/codegen/dbobj.rs index 330473b7..e31c2b15 100644 --- a/butane_core/src/codegen/dbobj.rs +++ b/butane_core/src/codegen/dbobj.rs @@ -36,7 +36,14 @@ pub fn impl_dbobject(ast_struct: &ItemStruct, config: &Config) -> TokenStream2 { let values: Vec = push_values(ast_struct, |_| true); let insert_cols = columns(ast_struct, |f| !is_auto(f)); - let save_core = if is_auto(&pk_field) { + let save_core = if auto_pk && values.len() == 1 { + quote!( + if !butane::PrimaryKeyType::is_valid(self.pk()) { + let pk = conn.insert_returning_pk(Self::TABLE, &[], &pkcol, &[])?; + self.#pkident = butane::FromSql::from_sql(pk)?; + } + ) + } else if auto_pk { let pkident = pk_field.ident.clone().unwrap(); let values_no_pk: Vec = push_values(ast_struct, |f: &Field| f != &pk_field); let save_cols = columns(ast_struct, |f| !is_auto(f) && f != &pk_field); @@ -47,17 +54,15 @@ pub fn impl_dbobject(ast_struct: &ItemStruct, config: &Config) -> TokenStream2 { // keys, but butane isn't well set up to take advantage of that, including missing // support for constraints and the `insert_or_update` method not providing a way to // retrieve the pk. - if (butane::PrimaryKeyType::is_valid(&self.#pkident)) { + if butane::PrimaryKeyType::is_valid(self.pk()) { #(#values_no_pk)* - if values.len() > 0 { - conn.update( - Self::TABLE, - pkcol, - butane::ToSql::to_sql_ref(self.pk()), - &[#save_cols], - &values, - )?; - } + conn.update( + Self::TABLE, + pkcol, + butane::ToSql::to_sql_ref(self.pk()), + &[#save_cols], + &values, + )?; } else { #(#values)* let pk = conn.insert_returning_pk(Self::TABLE, &[#insert_cols], &pkcol, &values)?; @@ -72,7 +77,6 @@ pub fn impl_dbobject(ast_struct: &ItemStruct, config: &Config) -> TokenStream2 { ) }; - let numdbfields = fields(ast_struct).filter(|f| is_row_field(f)).count(); let many_save: TokenStream2 = fields(ast_struct) .filter(|f| is_many_to_many(f)) .map(|f| { @@ -93,8 +97,12 @@ pub fn impl_dbobject(ast_struct: &ItemStruct, config: &Config) -> TokenStream2 { .collect(); let dataresult = impl_dataresult(ast_struct, tyname, config); + // Note the many impls following DataObject can not be generic because they implement for T and &T, + // which become conflicting types as &T is included in T. + // https://stackoverflow.com/questions/66241700 quote!( - #dataresult + #dataresult + impl butane::DataObject for #tyname { type PKType = #pktype; type Fields = #fields_type; @@ -106,10 +114,12 @@ pub fn impl_dbobject(ast_struct: &ItemStruct, config: &Config) -> TokenStream2 { } fn save(&mut self, conn: &impl butane::db::ConnectionMethods) -> butane::Result<()> { //future perf improvement use an array on the stack - let mut values: Vec = Vec::with_capacity(#numdbfields); + let mut values: Vec = Vec::with_capacity( + ::COLUMNS.len() + ); let pkcol = butane::db::Column::new( - #pklit, - <#pktype as butane::FieldType>::SQLTYPE); + Self::PKCOL, + ::SQLTYPE); #save_core #many_save Ok(()) From dd74f171801e5dd4dbce80d40e67e5794ad66851 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Sun, 14 Jan 2024 11:27:11 +0800 Subject: [PATCH 2/2] Deduplicate setting autopk field --- butane_core/src/codegen/dbobj.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/butane_core/src/codegen/dbobj.rs b/butane_core/src/codegen/dbobj.rs index e31c2b15..c12326c2 100644 --- a/butane_core/src/codegen/dbobj.rs +++ b/butane_core/src/codegen/dbobj.rs @@ -40,11 +40,12 @@ pub fn impl_dbobject(ast_struct: &ItemStruct, config: &Config) -> TokenStream2 { quote!( if !butane::PrimaryKeyType::is_valid(self.pk()) { let pk = conn.insert_returning_pk(Self::TABLE, &[], &pkcol, &[])?; - self.#pkident = butane::FromSql::from_sql(pk)?; - } + Some(butane::FromSql::from_sql(pk)?) + } else { + None + }; ) } else if auto_pk { - let pkident = pk_field.ident.clone().unwrap(); let values_no_pk: Vec = push_values(ast_struct, |f: &Field| f != &pk_field); let save_cols = columns(ast_struct, |f| !is_auto(f) && f != &pk_field); quote!( @@ -63,17 +64,21 @@ pub fn impl_dbobject(ast_struct: &ItemStruct, config: &Config) -> TokenStream2 { &[#save_cols], &values, )?; + None } else { #(#values)* let pk = conn.insert_returning_pk(Self::TABLE, &[#insert_cols], &pkcol, &values)?; - self.#pkident = butane::FromSql::from_sql(pk)?; - } + Some(butane::FromSql::from_sql(pk)?) + }; ) } else { // do an upsert quote!( - #(#values)* - conn.insert_or_replace(Self::TABLE, &[#insert_cols], &pkcol, &values)?; + { + #(#values)* + conn.insert_or_replace(Self::TABLE, &[#insert_cols], &pkcol, &values)?; + None + }; ) }; @@ -120,7 +125,10 @@ pub fn impl_dbobject(ast_struct: &ItemStruct, config: &Config) -> TokenStream2 { let pkcol = butane::db::Column::new( Self::PKCOL, ::SQLTYPE); - #save_core + let new_pk = #save_core + if let Some(new_pk) = new_pk { + self.#pkident = new_pk; + } #many_save Ok(()) }