Add a story for upsert

[sgrif]

This is currently a gap in our API. Both SQLite and PG support it, though I'm not sure we can do a backend agnostic abstraction. I'm fine with providing two backend specific abstractions for this case.

[sgrif]

Bumping to the 0.7 milestone. SQLite has extremely simple semantics, and we should at least be able to implement it for at least that backend. PG might get bumped to 0.8.

I don't think we can abstract over this in a database agnostic way. Instead we will provide backend specific APIs. SQLite will probably be insert(&new_record).into(table).on_conflict(replace)

[sgrif]

SQLite provides 5 options for alternate behaviors. REPLACE, ROLLBACK, ABORT, FAIL, and IGNORE. We won't provide support for INSERT OR ROLLBACK, as our handling of transactions is based on lexical scope and we can't properly handle this case.

[Eijebong]

Any ETA for postgres support ? :)

[sgrif]

Is this a blocker for your application? I can definitely get it done for 0.8

[Eijebong]

Yes, it's a blocker.

[sgrif]

Tentative release date for 0.8 is the 28th (if enough little things like this get completed I might release sooner though)

[Eijebong]

Cool, thank you !

[drusellers]

👍

[norcalli]

This doesn't seem to have made it into the 0.8 release for Postgres, is it possible to prioritize a backend specific method for the next release? If I have the time, I may be able to send a PR (over the next few weeks)

[sgrif]

It will be in the next release. 0.8 was featureless as we needed to get macros 1.1 support out.

[sgrif]

I'm going to release 0.9 today. Unfortunately, I wasn't able to complete this in time despite my best efforts. PG's upsert is an extremely complex feature, and it's just going to take some more time for me to implement proper support with an API that is up to our standards. I can't push back the 0.9 release any longer, as I am going on vacation on Tuesday and I want to release with enough time to fix any issues that may occur. So I will move this to the 0.10 release, which is slated for the end of January.

[norcalli]

I understand.

[norcalli]

I didn't mean to submit that comment, dang mobile. I understand, as I've had some dealings with trying to implement it myself in Scala/Slick, and I appreciate waiting to implement it properly.

Have a good vacation!

[seamusabshere]

hey @sgrif do you have a WIP branch for this?

[sgrif]

@seamusabshere No, just a few spikes which I'm going to throw the code away from.

[emk]

I would be interested on working on this (as soon as I finish the uuid upgrade and integration tests). Do you have any ideas about what the API should look like?

[sgrif]

That's the main thing I'm trying to decide on. I need to spike on it a bit more to decide for sure. I think it'll probably be something like the examples below. Feel free to take a swing at it if you'd like, but I suspect that implementing this properly will require a pretty thorough knowledge of our internals. I'm skipping all of the modifiers on the conflict target for now. I'm also unsure if I want to handle multiple on conflict clauses to start, but I've included it below.

// In addition to the examples that I say must fail to compile, all of this must fail to compile if used with a backend other than PG.

// single record ON CONFLICT DO NOTHING
insert(&new_user.on_conflict_do_nothing()).into(users)
// multi-record ON CONFLICT DO NOTHING
insert(&vec![user1, user2].on_conflict_do_nothing()).into(users)
// Need to ensure that this does not compile
insert(&vec![user1.on_conflict_do_nothing()]).into(users)

// everything below should also work with vecs, should also fail to compile if the conflict handler is on a record inside the vec
// single record single handler by constraint name
insert(&new_user.on_conflict(on_constraint("users_pkey"), do_update().set(any_valid_as_changeset)).into(users)
// should fail to compile
insert(&new_user.on_conflict(on_constraint("users_pkey"), do_update().set(posts::title.eq("foo"))).into(users)
// single record single handler with where clause
insert(&new_user.on_conflict(on_constraint("users_pkey"), do_update().set(any_valid_as_changeset).filter(any_valid_predicate)).into(users)
// should fail to compile
insert(&new_user.on_conflict(on_constraint("users_pkey"), do_update().set(any_valid_as_changeset).filter(non_bool_expr)).into(users)
// should fail to compile
insert(&new_user.on_conflict(on_constraint("users_pkey"), do_update().set(any_valid_as_changeset).filter(posts::title.eq("hi"))).into(users)
// single record single handler by single column
insert(&new_user.on_conflict(id, do_update().set(any_valid_as_changeset)).into(users)
// single record single handler by multiple columns
insert(&new_user.on_conflict((project_id, email), do_update().set(any_valid_as_changeset)).into(users)
// single record single handler by arbitrary expr
insert(&new_user.on_conflict(lower(email), do_update().set(any_valid_as_changeset)).into(users)
// should fail to compile
insert(&new_user.on_conflict(posts::id, do_update().set(any_valid_as_changeset)).into(users)
// single record multiple handlers
insert(&new_user.on_conflict(any_valid_args).on_conflict(any_valid_args)).into(users)
// ON CONFLICT DO NOTHING with conflict_target
insert(&new_user.on_conflict(id, do_nothing()))

I think that's everything, though I might have forgotten a few cases.

[emk]

Hmm, yeah, that's complicated enough that it would take me a long time to get up to speed.

Are there any reasonably good ways to work around this missing feature? It turns out that we need it fairly badly in our Rust production app. I can obviously fall back to postgres and send raw SQL if all else fails. The feature we really need is ON CONFLICT DO NOTHING on a vector of records (specifically the second line of your example), which might make this slightly easier to work around.

Any thoughts on the best way to tackle this?

(I'm currently working on integration tests that prove we can mix serde and uuid and diesel without any problem, for the other issue I opened.)

[sgrif]

Depending on how important actual atomicity is, the easiest workaround would just be to do a SELECT query on the fields that could conflict, filter the vector, and then an update. You could also write a stored procedure and use sql_function!. I am going to do on_conflict_do_nothing before the rest though, I might be able to finish just that piece of it soon.

[sgrif]

Yes, I agree. The hard part is mostly just making sure that incorrect uses fail to compile.

[seamusabshere]

hey @sgrif please do push this out to us if you have other priorities

[sgrif]

The plan is to have ON CONFLICT DO NOTHING as part of 0.11.

[sgrif]

Removing from the milestone since ON CONFLICT DO NOTHING is all we are planning on doing for upsert in 0.11. The rest will probably be in 0.12. ON CONFLICT DO NOTHING is implemented by #712

[emk]

It's great to have this!

I accidentally omitted use diesel::pg::upsert::OnConflictExtension; from my file, and Rust printed:

error: no method named `on_conflict_do_nothing` found for type `models::score::Score` in the current scope
   --> src/scoring.rs:303:43
    |
303 |                     diesel::insert(&score.on_conflict_do_nothing())
    |                                           ^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: items from traits can only be used if the trait is in scope; the following trait is implemented but not in scope, perhaps add a `use` for it:
    = help: candidate #1: `use diesel::pg::upsert::OnConflictExtension;`

But this was then followed by dozens of lines of errors like the following:

    = note: required because of the requirements on the impl of `diesel::query_builder::insert_statement::UndecoratedInsertRecord<models::score::scores::table>` for `&std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<std::vec::Vec<_>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`

I'm not sure what's going on with that error message, but in addition to spamming my error output console, it looks a bit suspicious. We do have huge-tables enabled, but I can't figure out why that would result in very deeply nested vectors. So this might really be a separate issue, but I don't understand enough diesel internals to diagnose it.

[sgrif]

It's a Rust bug, not a Diesel one. It sometimes does weird recursive trait lookups when the type at the bottom of it is a type error.

[emk]

Ah, thank you! Good to know. Is there an upstream bug for that which I can watch?

I've fixed that, but now I'm hitting more weird errors. The following code compiles fine:

                for score in new_scores {
                    diesel::insert(&score.on_conflict_do_nothing())
                        .into(scores::table)
                        .execute(&*dconn)?;
                }

But if I change it to:

                for score_chunk in new_scores.chunks(5000) {
                    diesel::insert(&score_chunk.on_conflict_do_nothing())
                        .into(scores::table)
                        .execute(&*dconn)?;
                }

...I get:

error: no method named `execute` found for type `diesel::query_builder::insert_statement::InsertStatement<models::score::scores::table, &diesel::pg::upsert::on_conflict_clause::OnConflictDoNothing<&&[models::score::Score]>>` in the current scope
   --> src/scoring.rs:306:26
    |
306 |                         .execute(&*dconn)?;
    |                          ^^^^^^^
    |
    = note: the method `execute` exists but the following trait bounds were not satisfied: `diesel::query_builder::insert_statement::InsertStatement<models::score::scores::table, &diesel::pg::upsert::on_conflict_clause::OnConflictDoNothing<&&[models::score::Score]>> : diesel::query_builder::QueryFragment<_>`, `&diesel::query_builder::insert_statement::InsertStatement<models::score::scores::table, &diesel::pg::upsert::on_conflict_clause::OnConflictDoNothing<&&[models::score::Score]>> : diesel::query_builder::QueryFragment<_>`, `&mut diesel::query_builder::insert_statement::InsertStatement<models::score::scores::table, &diesel::pg::upsert::on_conflict_clause::OnConflictDoNothing<&&[models::score::Score]>> : diesel::query_builder::QueryFragment<_>`, `&mut diesel::query_builder::insert_statement::InsertStatement<models::score::scores::table, &diesel::pg::upsert::on_conflict_clause::OnConflictDoNothing<&&[models::score::Score]>> : diesel::query_builder::QueryId`

I've tried several variations of the basic syntax, but it generally results in errors before reaching execute.

(I'm inserting rows in batches of 5,000 to avoid generating SQL INSERT INTO statements with more than 65,535 ? placeholders. This was necessary in earlier versions of diesel but I haven't checked the latest.)

[emk]

Here's what happens:

error[E0308]: mismatched types
   --> src/scoring.rs:304:36
    |
304 |                     diesel::insert(score_chunk.on_conflict_do_nothing())
    |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected reference, found struct `diesel::pg::upsert::on_conflict_clause::OnConflictDoNothing`
    |
    = note: expected type `&_`
    = note:    found type `diesel::pg::upsert::on_conflict_clause::OnConflictDoNothing<&&[models::score::Score]>`

The &&[..] is coming from on_conflict_do_nothing, maybe?

[sgrif]

Yeah, maybe give &(*score_chunk).on_conflict_do_nothing() a try or even &OnConflictExtension::on_conflict_do_nothing(score_chunk). I'll add an explicit test for when the type is &[T] and not Vec<T>

[emk]

&(*score_chunk).on_conflict_do_nothing() produces:

error: no method named `on_conflict_do_nothing` found for type `[models::score::Score]` in the current scope
   --> src/scoring.rs:304:52
    |
304 |                     diesel::insert(&(*score_chunk).on_conflict_do_nothing())
    |                                                    ^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: the method `on_conflict_do_nothing` exists but the following trait bounds were not satisfied: `[models::score::Score] : std::marker::Sized`

The second produces:

error[E0277]: the trait bound `[models::score::Score]: std::marker::Sized` is not satisfied
   --> src/scoring.rs:305:37
    |
305 |                     diesel::insert(&OnConflictExtension::on_conflict_do_nothing(score_chunk))
    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Sized` is not implemented for `[models::score::Score]`
    |
    = note: `[models::score::Score]` does not have a constant size known at compile-time
    = note: required because of the requirements on the impl of `diesel::pg::upsert::OnConflictExtension` for `[models::score::Score]`
    = note: required by `diesel::pg::upsert::OnConflictExtension::on_conflict_do_nothing`

There's probably a magic permutation which will work, but I haven't found it yet.

[sgrif]

Nope, I need to add a : ?Sized constraint on the trait I think.

[emk]

OK, cool. :-) Thank you for your help figuring this out!

[sgrif]

I'll release a 0.11.1 with #723 once it's green.

[sgrif]

#793 implements this with two main exceptions:

• ON CONFLICT DO UPDATE with expression indices
• ON CONFLICT DO UPDATE with a where clause on the update

These are both things that we will eventually support, but at the moment they are not prioritized. If you need one of those two features, please let me know.