-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add upsert method #137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -255,8 +255,12 @@ let anvil: Product = Product::find(&pool, anvil.id).await?; | |
| // Upsert (insert or update on PK conflict) | ||
| let anvil: Product = anvil.save(&pool).await?; | ||
|
|
||
| // Bulk upsert a collection | ||
| let products = vec![anvil]; | ||
| products.upsert(&pool, Product::ID).await?; | ||
|
|
||
| // Delete by primary key, no instance needed | ||
| Product::destroy(&pool, anvil.id).await?; | ||
| Product::destroy(&pool, Uuid::new_v4()).await?; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check if there are similar destroy patterns in other docs
rg -n -C3 '::destroy\(' --type=md docs/Repository: robinstraub/fabrique Length of output: 2577 🏁 Script executed: # Get the full context around line 263 in models.md
sed -n '240,280p' docs/src/concepts/models.mdRepository: robinstraub/fabrique Length of output: 920 Update destroy example to delete the created record for proper lifecycle demonstration. The example creates an 🤖 Prompt for AI Agents |
||
| # Ok(()) | ||
| # } | ||
| ``` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| use crate::{ | ||
| database::Column, | ||
| dialect::Dialect, | ||
| model::{Model, Persist}, | ||
| }; | ||
|
|
||
| pub trait UniqueBy<M> { | ||
| fn column_names() -> &'static [&'static str]; | ||
| } | ||
|
|
||
| macro_rules! impl_unique_by { | ||
| ($($C:ident),+) => { | ||
| impl<M, $($C),+> UniqueBy<M> for ($($C,)+) | ||
| where | ||
| $($C: Column<M>,)+ | ||
| { | ||
| fn column_names() -> &'static [&'static str] { | ||
| &[$($C::NAME),+] | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| impl_unique_by!(C0); | ||
| impl_unique_by!(C0, C1); | ||
| impl_unique_by!(C0, C1, C2); | ||
| impl_unique_by!(C0, C1, C2, C3); | ||
|
Comment on lines
+11
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial | 💤 Low value Tuple arity capped at 4 — document or extend.
Either extend the macro to a higher arity (e.g., up to 8 or 12, mirroring std's tuple-trait conventions) or note the limit in the trait's rustdoc and in the cookbook section. 🤖 Prompt for AI Agents |
||
|
|
||
| pub trait Upsert<DB: Dialect> { | ||
| type Model: Model; | ||
|
|
||
| fn upsert<'e, A, U>( | ||
| self, | ||
| executor: A, | ||
| unique_by: U, | ||
| ) -> impl Future<Output = Result<(), crate::Error>> | ||
| where | ||
| A: sqlx::Acquire<'e, Database = DB> + Send + 'e, | ||
| U: UniqueBy<Self::Model>; | ||
| } | ||
|
|
||
| impl<M, DB> Upsert<DB> for Vec<M> | ||
| where | ||
| M: Persist<DB> + Send, | ||
| DB: Dialect, | ||
| <DB as sqlx::Database>::Arguments: sqlx::IntoArguments<DB>, | ||
| for<'c> &'c mut <DB as sqlx::Database>::Connection: sqlx::Executor<'c, Database = DB>, | ||
| { | ||
| type Model = M; | ||
|
|
||
| async fn upsert<'e, A, U>(self, executor: A, _unique_by: U) -> Result<(), crate::Error> | ||
| where | ||
| A: sqlx::Acquire<'e, Database = DB> + Send + 'e, | ||
| U: UniqueBy<M>, | ||
| { | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| if self.is_empty() { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let mut conn = executor.acquire().await.map_err(crate::Error::from)?; | ||
|
|
||
| let mut qb = sqlx::QueryBuilder::new("INSERT INTO "); | ||
| qb.push(M::table_name()); | ||
| qb.push(" ("); | ||
| qb.push(M::columns().join(", ")); | ||
| qb.push(") "); | ||
|
|
||
| qb.push_values(self, |separated, model| { | ||
| model.push_bind_values(separated); | ||
| }); | ||
|
|
||
| let unique_cols = U::column_names(); | ||
| qb.push(DB::on_conflict_sql(unique_cols)); | ||
|
|
||
| let update_cols: Vec<&str> = M::columns() | ||
| .iter() | ||
| .copied() | ||
| .filter(|c| !unique_cols.contains(c)) | ||
| .collect(); | ||
| qb.push(DB::do_update_sql(&update_cols)); | ||
|
|
||
| qb.build().execute(&mut *conn).await?; | ||
|
Comment on lines
+71
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find Dialect trait definition and impls for on_conflict_sql / do_update_sql
# to verify behavior when an empty slice is passed.
fd -e rs | xargs rg -nP -C5 '\bdo_update_sql\s*\('
echo "---"
fd -e rs | xargs rg -nP -C5 '\bon_conflict_sql\s*\('
echo "---"
# Look for any explicit empty-slice guard
fd -e rs | xargs rg -nP -C2 'is_empty\(\)|len\(\)\s*==\s*0' | rg -i 'do_update|conflict'Repository: robinstraub/fabrique Length of output: 13107 🏁 Script executed: sed -n '40,50p' fabrique-core/src/dialect.rsRepository: robinstraub/fabrique Length of output: 438 🏁 Script executed: sed -n '68,78p' fabrique-core/src/dialect.rsRepository: robinstraub/fabrique Length of output: 408 🏁 Script executed: sed -n '96,106p' fabrique-core/src/dialect.rsRepository: robinstraub/fabrique Length of output: 408 🏁 Script executed: # Also check the complete upsert function to see context
sed -n '60,85p' fabrique-core/src/upsert.rsRepository: robinstraub/fabrique Length of output: 743 Guard against empty When all columns are part of the conflict target,
All three produce syntax errors. The semantically correct behavior is Add a guard at line 78 to call if update_cols.is_empty() {
qb.push(DB::do_nothing_sql());
} else {
qb.push(DB::do_update_sql(&update_cols));
}🤖 Prompt for AI Agents |
||
| Ok(()) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: robinstraub/fabrique
Length of output: 19004
🏁 Script executed:
sed -n '60,75p' docs/src/concepts/factories.mdRepository: robinstraub/fabrique
Length of output: 439
🏁 Script executed:
rg -n 'factory.*\).*\.make\(\)' --type=md docs/ -B2 -A2Repository: robinstraub/fabrique
Length of output: 629
🏁 Script executed:
Repository: robinstraub/fabrique
Length of output: 156
🏁 Script executed:
Repository: robinstraub/fabrique
Length of output: 1842
🏁 Script executed:
cat -n fabrique/src/factory.rs | head -100Repository: robinstraub/fabrique
Length of output: 3543
🏁 Script executed:
rg -n 'fn make\(' fabrique/src/factory.rs -A 5Repository: robinstraub/fabrique
Length of output: 46
🏁 Script executed:
rg -n 'fn make\(' fabrique-core/src/factory.rs -A 5Repository: robinstraub/fabrique
Length of output: 46
🏁 Script executed:
Repository: robinstraub/fabrique
Length of output: 46
🏁 Script executed:
wc -l fabrique/src/factory.rs && tail -100 fabrique/src/factory.rsRepository: robinstraub/fabrique
Length of output: 2680
🏁 Script executed:
cat -n fabrique-core/src/factory.rs | head -150Repository: robinstraub/fabrique
Length of output: 3086
🏁 Script executed:
cat -n fabrique-derive/src/codegen/factory.rs | head -200Repository: robinstraub/fabrique
Length of output: 8993
🏁 Script executed:
rg -n 'fn generate_factory_method_make' fabrique-derive/src/codegen/factory.rs -A 30Repository: robinstraub/fabrique
Length of output: 1146
🏁 Script executed:
Repository: robinstraub/fabrique
Length of output: 1955
Remove the unnecessary turbofish type parameter.
The example uses
Product::factory::<sqlx::Sqlite>()but this explicit type specification is redundant. Rust can infer the database type from context. Other.make()examples in the docs (e.g.,docs/src/cookbook/bulk-update-and-upsert.mdlines 217–218) omit the turbofish and compile successfully. Update to match the consistent pattern elsewhere:🤖 Prompt for AI Agents