Skip to content
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

Adds support for PostgreSQL which adds #87 and is mentioned in #246. #621

Merged
merged 1 commit into from
Sep 16, 2019
Merged

Adds support for PostgreSQL which adds #87 and is mentioned in #246. #621

merged 1 commit into from
Sep 16, 2019

Conversation

swedishborgie
Copy link
Contributor

The biggest change to existing code is that replace_into() isn't supported by Diesel for the PostgreSQL back end, instead requiring the use of on_conflict(). This unfortunately requires a branch for save() on most of the models and requires PostgreSQL >= 9.5.

I've included Dockerfile's for amd64 for both the Debian and Alpine variants, it should be straight forward to create new files for the other architectures but I don't currently have access to the other architectures and I didn't want to submit anything untested.

I've only done a couple hours worth of testing but everything seems to work. The migration SQL and model schema are both very similar to the existing two back ends.

Please let me know if you'd like any changes or if I missed anything.

This includes migrations as well as Dockerfile's for amd64.

The biggest change is that replace_into isn't supported by Diesel for the
PostgreSQL backend, instead requiring the use of on_conflict. This
unfortunately requires a branch for save() on all of the models currently
using replace_into.
@dani-garcia
Copy link
Owner

Very nice job @swedishborgie!

It all looks good to me, pity that replace_into isn't supported, but the workaround is simple enough in my view.

One small question I have is the addition of the openssl dependency, as you say for SSL support, but it isn't actually used anywhere, is diesel using it? And in that case shouldn't diesel require it or expose it as a feature flag? (Not that I'd have a problem adding the dependency, it just seems a bit strange)

@swedishborgie
Copy link
Contributor Author

I ran into that issue when trying to build the Alpine image. The Rust PostgreSQL bindings link against libpq, which links against OpenSSL to provide encrypted connection support. The libpq-dev package on Alpine requires that the OPENSSL_config struct be defined while linking and without it the project fails to link. Explicitly including the openssl crate seemed to pull in the right symbols. On Debian everything seemed OK without the import.

Here's the issues I followed to get to the solution I ended up with:

@dani-garcia
Copy link
Owner

Makes sense, thanks for your work!

@dani-garcia dani-garcia merged commit 3a90364 into dani-garcia:master Sep 16, 2019
@ptman
Copy link

ptman commented Oct 29, 2019

Is it possible to migrate from sqlite to postgresql?

@bjo81
Copy link

bjo81 commented Oct 29, 2019

Yes. Let it create the postgresql-database, then use pgloader to migrate with the following config:

load database
     from sqlite:///var/lib/bitwarden_rs/db.sqlite3 
     into postgresql://user:pass@localhost/database
     with data only, include no drop, reset sequences
     excluding table names like '__diesel_schema_migrations'
;

thelittlefireman pushed a commit to thelittlefireman/bitwarden_rs that referenced this pull request Mar 19, 2021
Adds support for PostgreSQL which adds dani-garcia#87 and is mentioned in dani-garcia#246.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants