Make insert statement not panic when inserting nothing#1708
Make insert statement not panic when inserting nothing#1708tyt2y3 merged 7 commits intoSeaQL:masterfrom
Conversation
src/query/insert.rs
Outdated
| /// }; | ||
| /// assert_eq!( | ||
| /// cake::Entity::insert(orange) | ||
| /// .on_empty_do_nothing() |
There was a problem hiding this comment.
umm, why do we need to change the examples here? I think this PR should have no changes to exsiting features
src/query/traits.rs
Outdated
| } | ||
|
|
||
| /// Insert query Trait | ||
| pub trait InsertTrait<A>: Sized |
There was a problem hiding this comment.
why do we need a new Trait here? it is not strictly necessary? I am cautious about new traits, mostly because of the potential impact to compile time. And introducing a new type to the public API require a lot of thought.
There was a problem hiding this comment.
Not a must, we just want to share the common methods between Insert and TryInsert. Methods such as one, many, add...etc.
There was a problem hiding this comment.
They can all implement the same methods. I don't see InsertTrait being used in a type signature anywhere so I think it is not needed. Unless someone is accepting a generic I: InsertTrait.
There was a problem hiding this comment.
My concern is code duplication. Implement the same method twice, for two struct, Insert and TryInsert.
We can use macro, but I think InsertTrait to define shared methods might be better. Both are okay for me.
There was a problem hiding this comment.
So the idea is to avoid unused trait?
There was a problem hiding this comment.
Yes. technically TryInsert contains an Insert so any implementation can simply be delegated. I am not seeing where's the potential saving in code duplication.
|
Looks great so far! Please finish up this PR, and then make a follow up PR: So, in https://github.com/SeaQL/sea-orm/blob/master/tests/upsert_tests.rs we have: let res = Entity::insert_many([..])
.on_conflict(on_conflict)
.exec(db)
.await;
assert_eq!(res.err(), Some(DbErr::RecordNotInserted));Can we add a Then, we'd have (in addition to the above): let res = Entity::insert_many([..])
.on_conflict(on_conflict)
.do_nothing()
.exec(db)
.await;
assert_eq!(res, Ok(TryInsertResult::Conflicted));I am aware that we'd have to write |
| .into_iter() | ||
| .filter(|_| false); | ||
|
|
||
| let empty_insert = Bakery::insert_many(empty_iterator) |
There was a problem hiding this comment.
* Optional Field SeaQL/sea-orm#1513 * .gitignore SeaQL/sea-orm#1334 * migration table custom name SeaQL/sea-orm#1511 * OR condition relation SeaQL/sea-orm#1433 * DerivePartialModel SeaQL/sea-orm#1597 * space for migration file naming SeaQL/sea-orm#1570 * composite key up to 12 SeaQL/sea-orm#1508 * seaography integration SeaQL/sea-orm#1599 * QuerySelect SimpleExpr SeaQL/sea-orm#1702 * sqlErr SeaQL/sea-orm#1707 * migration check SeaQL/sea-orm#1519 * postgres array SeaQL/sea-orm#1565 * param intoString SeaQL/sea-orm#1439 * **skipped** re-export SeaQL/sea-orm#1661 * ping SeaQL/sea-orm#1627 * on empty do nothing SeaQL/sea-orm#1708 * on conflict do nothing SeaQL/sea-orm#1712 * **skipped** upgrade versions * active enum fail safe SeaQL/sea-orm#1374 * relation generation check SeaQL/sea-orm#1435 * entity generation bug SeaQL/sea-schema#105 * **skipped** bug fix that does not require edits * EnumIter change SeaQL/sea-orm#1535 * completed and fixed a previous todo SeaQL/sea-orm#1570 * amended wordings and structures * Edit * Remove temp file --------- Co-authored-by: Billy Chan <[email protected]>
🎉 Released In 0.12.1 🎉Thank you everyone for the contribution! |
3 participants
PR Info
New Features
Bug Fixes
Changes