sea-orm
sea-orm copied to clipboard
Support on conflict
PR Info
This PR exposes the ON CONFLICT support that was added in sea-query
Adds
Option for a new insert_or that allows you to specify an on-conflict behavior
Public API Rationale
I think there were 3 ways this could be exposed from a public API point of view:
- Modify the existing
insertfunction to add a new parameter - Keep the
insertfunction as-is, but introduce some state in the ActiveModel to handle conflicts - Create a new function called
insert_or
- I didn't pick (1) because this feels like too big of a breaking change.
- I didn't pick (2) because to be, handling conflict is something you do in a per-query basis and not in the model. For example, in some cases you may want to do nothing on a conflict, some cases you may want to error, some cases you may want to override.
- (3) seemed like the best solution
Handling of return value
One of the conflict options is ON CONFLICT DO NOTHING. This doesn't work with the current API where the insert function is assumed to always have a return value (do nothing returns an empty SQL result if a conflict is found)
There are two ways this can be solved:
- When using
ON CONFLICT DO NOTHING, check if nothing was returned and then do afindto find the entry that caused the conflict and return that as the result of the insert - Make that when using
insert_or, we have an optional return instead of a mandatory return
I picked (2) seems (1) probably has performance implications
TODO
- [ ] When upgrading to
sea-query0.24.0 (which introduces the on-conflict support), I had to addwindow: Noneto a few functions. Is this correct? (see https://github.com/SeaQL/sea-query/pull/271) - [ ] When using
do nothing, we can't callafter_savesince nothing was saved. Is this okay? - [ ] I'm not sure what to do about testing since it seems the insert functions are not directly tested either. Should it be done as an integration test in
tests/relational_tests? - [ ] Add documentation for
insert_or
I rebased on top of #673 to fix the bug with the 0.24.0 upgrade
@billy1624 I'm concerned that the performance implications of doing this probably aren't desired for cases where ON CONFLICT DO NOTHING is used (the only case where nothing is returned)
Rationale
ON CONFLICT can only be used on unique constraints. Although there are some tables with multiple unique constraints, most of the time your unique constraint is the primary key (possibly a composite key)
Example: A database with 3 tables:
- Persons table
- Jobs table
- Some table that maps the many-many relation of <PersonId, JobId>
If you want to insert a new <PersonId, JobId> pair, you can just insert with an ON CONFLICT DO NOTHING because if row already exists, you already know all the data in the row so you don't care about the return value. Having sea-orm do a 2nd query to get the row provides no additional information and double the number of database roundtrips required
Potential compromise solution
Instead of implementing insert_or as a member function of ActiveModel, we could decide to force users to use the Entity::insert() function instead that way the user has to explicitly pick exec or exec_with_returning so they can pick which they want.
If we really want to provide insert_or as a member function, we could add in the documentation to encourage users to use the Entity function if they explicitly want to ignore the return value
If you want to insert a new
<PersonId, JobId>pair, you can just insert with anON CONFLICT DO NOTHINGbecause if row already exists, you already know all the data in the row so you don't care about the return value. Having sea-orm do a 2nd query to get the row provides no additional information and double the number of database roundtrips required
Make sense. I think the return type of insert_or is confusing, Result<Option<Model>, ...>, as you have said the return values doesn't matter, we could just return Result<(), ...>. However, one can't access the inserted ActiveModel anymore as it has been moved because the signature of insert_or<'a, C>(self, ...).
async fn insert_or<'a, C>(self, db: &'a C, on_conflict: &OnConflict) -> Result<Option<<Self::Entity as EntityTrait>::Model>, DbErr>
Sorry for the delay but this seems like a complicated topic. Hopefully I will be able to provide some meaningful input soon
Hey @SebastienGllmt, sorry for the delay.
The ON CONFLICT features has been added to SeaORM on another PR:
- https://github.com/SeaQL/sea-orm/pull/791
Thank you so much for the contributions. And do let us know if you have idea on further improving it :)