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

add SQLx driver #57

Closed
jxs opened this issue Feb 10, 2020 · 12 comments
Closed

add SQLx driver #57

jxs opened this issue Feb 10, 2020 · 12 comments
Assignees
Labels
enhancement New feature or request

Comments

@jxs
Copy link
Member

jxs commented Feb 10, 2020

No description provided.

@jxs jxs added the enhancement New feature or request label Feb 10, 2020
@jxs jxs changed the title add Sqlx driver add SQLx driver Feb 10, 2020
@belak
Copy link

belak commented May 14, 2020

At least for now, this is harder than you'd expect for multiple reasons:

  • The current API for SQLx does a ping-pong between connection and transaction. When you begin a transaction, it takes ownership of the connection object. This makes writing an impl AsyncTransaction for sqlx::PgConnection very hard... it may only be possible with the current APIs/Traits to implement for a PgPool... then the migration system can grab a connection from the pool, and run through the whole process.
  • There are compiler bugs around <Connection as HasRow>::Row vs PgRow - it has issues inferring that they're the same thing. This can be worked around by using a macro with the concrete types to implement the same functionality everywhere. That may actually be better in the long term. The developer is working on some tweaks which may improve this soon.
  • libsqlite-sys has a version mismatch between rusqlite and sqlx. This can be resolved by migrating back to rusqlite 0.22 until sqlx has upgraded.

I may continue this later with Pool rather than the direct Connections if there's still interest.

@jxs
Copy link
Member Author

jxs commented May 14, 2020

hi, and thanks for looking into this! 👍

The current API for SQLx does a ping-pong between connection and transaction. When you begin a transaction, it takes ownership of the connection object. This makes writing an impl AsyncTransaction for sqlx::PgConnection very hard... it may only be possible with the current APIs/Traits to implement for a PgPool... then the migration system can grab a connection from the pool, and run through the whole process.

that is ok i think, mysql_async support is only implemented for Pool at the moment too.

regarding the other two issues, i'd rather wait for sqlx to update to 0.23, and the compiler bugs to be fixed, what do you think?
Meanwhile i am going to provide migrations from a config file instead of connection, so that we support usecases where the driver is not yet supported like this one

@belak
Copy link

belak commented May 14, 2020

I don't know how soon the compiler bugs will be fixed, but I think waiting for a new SQLx version to work around them would be good enough.

One other question: how would the API for this work? I've got the basic driver done but I'm not as sure about the other necessary changes. Since there would be multiple types which could provide migrations for the different database types it complicates things a bit.

@jxs
Copy link
Member Author

jxs commented May 14, 2020

I don't know how soon the compiler bugs will be fixed, but I think waiting for a new SQLx version to work around them would be good enough.

ok awesome 👍

One other question: how would the API for this work? I've got the basic driver done but I'm not as sure about the other necessary changes. Since there would be multiple types which could provide migrations for the different database types it complicates things a bit.

I am not sure, but what you are saying is that, through SQLx traits we can't have a unified API for all of their supported drivers?

@JamesHinshelwood
Copy link

Has anything changed on the status of this issue since these previous comments?

I'm happy to take a look at implementing something if there aren't any blockers..?

@jxs
Copy link
Member Author

jxs commented Dec 15, 2020

hi @JamesHinshelwood, have the issues mentioned by @belak on #57 (comment) been addressed?

@thomaseizinger
Copy link

libsqlite-sys has a version mismatch between rusqlite and sqlx. This can be resolved by migrating back to rusqlite 0.22 until sqlx has upgraded.

This is already resolved in latest SQLx master, however, there is a big release coming up which seems to change things fundamentally so it might be good to wait for 0.6 to be released.

@jxs
Copy link
Member Author

jxs commented Aug 17, 2021

also, since sqlx now also offers migration features, this might be redundant no?

@jxs jxs closed this as completed Jan 6, 2022
@Andrey36652
Copy link

Hello, I think this is still relevant. If I use sqlx already in a project then it can be considered an overkill to add another driver (tokio-postgres for example) just for migrations support.
Are there any technical reasons why sqlx support coudn't be implemented as per current state of things?

@jxs
Copy link
Member Author

jxs commented Aug 2, 2024

Hi, and thanks for the interest! But doesn't sqlx offer migrations?

@Andrey36652
Copy link

It does, but:

@jxs
Copy link
Member Author

jxs commented Aug 10, 2024

Ok I see, thanks for expanding! As I mentioned on #295 (comment) I don't think it makes sense to add more drivers to this repo, but if you are willing to implement support for sqlx I'd love to include it on the README.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants