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

SQLite: on_conflict().do_udpate() support #1854

Open
theduke opened this Issue Sep 18, 2018 · 10 comments

Comments

Projects
None yet
4 participants
@theduke
Contributor

theduke commented Sep 18, 2018

SQLite supports full UPSERT functionality equivalent to Postgres now.

Like: INSERT INTO phonebook(name,phonenumber) VALUES('Alice','704-555-1212') ON CONFLICT(name) DO UPDATE SET phonenumber=excluded.phonenumber;

See: https://www.sqlite.org/lang_UPSERT.html

Would be really great to support this, the same way it works in PG:

value.insert_into(table)
  .on_conflict(id)
  .do_update()
  .set(...)
@sgrif

This comment has been minimized.

Member

sgrif commented Sep 18, 2018

Thank you for the feature request. We're currently in the process of a cleanup of the issue tracker, and aren't currently accepting feature requests (there will be an official policy written soon, but the TL;DR is that open issues should reflect something that is a bug or on our immediate roadmap).

We're happy to discuss feature requests, but the place to do so is discourse.diesel.rs, not the issue tracker.

@sgrif sgrif closed this Sep 18, 2018

@theduke

This comment has been minimized.

Contributor

theduke commented Sep 18, 2018

Understood.
The README should really reflect this new policy, though.

@sgrif

This comment has been minimized.

Member

sgrif commented Sep 18, 2018

there will be an official policy written soon

Thanks for your patience.

@theduke

This comment has been minimized.

Contributor

theduke commented Sep 18, 2018

Guess I missed that. cough

@sgrif

This comment has been minimized.

Member

sgrif commented Sep 18, 2018

I'm actually going to re-open this, since I think implementing it will be fairly easy and would be a nice addition for 1.4

@sgrif sgrif reopened this Sep 18, 2018

@h-michael

This comment has been minimized.

h-michael commented Oct 13, 2018

@sgrif
Have you started implementing it?
If not, may I start implementing ?

@sgrif

This comment has been minimized.

Member

sgrif commented Oct 13, 2018

Go for it

@h-michael

This comment has been minimized.

h-michael commented Oct 13, 2018

@sgrif
Thanks to your reply.
BTW, both of Postgress and Sqlite have same query syntax for upsert.
How do you think how to pg and sqlite share query builder mdule?
If you have any ideas, please tell me your thought.
postgres https://www.postgresql.org/docs/10/static/sql-insert.html
sqlite https://www.sqlite.org/lang_UPSERT.html

@weiznich

This comment has been minimized.

Contributor

weiznich commented Oct 13, 2018

There is already an implementation of on_conflict().do_update() for postgresql here.

What needs to be done:

  • Move that module to diesel::upsert and add reeports at the current location (otherwise this is a breaking change). Also make this module only available if the sqlite or postgres feature is given.
  • See what parts are available on both systems, all other parts should be moved back to the postgres specific module.
  • Add a new trait SupportsOnConflictClause and implement it for Pg and Sqlite. (Similar to SupportsReturningClause here)
  • For all QueryFragment<Pg> impl's left in the new diesel::upsert module, change them to be impl<DB> QueryFragment<DB> for … where DB: SupportsOnConflictClause
  • Look for upsert test that apply for both backends and enable them on sqlite. (For example this one)

Feel free to open a PR early in that progress to get feedback there. Also ask in our gitter channel if you get stuck somewhere.

@h-michael

This comment has been minimized.

h-michael commented Oct 13, 2018

@weiznich
Thanks to your advide.
I'll implement this weekend!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment