Skip to content

Switch to sqlx for compile-time checked queries #874

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

Closed
jyn514 opened this issue Jul 2, 2020 · 9 comments · Fixed by #2618
Closed

Switch to sqlx for compile-time checked queries #874

jyn514 opened this issue Jul 2, 2020 · 9 comments · Fixed by #2618
Labels
C-technical-debt Category: This makes the code harder to read and modify, but has no impact on end users E-hard Effort: This will require a lot of work P-medium Medium priority S-waiting-on-async Status: waiting on async migration

Comments

@jyn514
Copy link
Member

jyn514 commented Jul 2, 2020

A common source of outages is an incorrect query with no tests (#862, #829, #648, etc). While it's good to have tests for all queries, it's not always feasible (especially until #822 is fixed). SQLx offers that advantage that it can check all queries at build time without having to write a test for that specific query. It is also fully async (cc @Kixiron), although that might mean we have to wait until we switch to tokio 0.2.

To avoid needless database connections, this should only connect to the database when a query is changed or added, or a schema change is made: launchbadge/sqlx#249. During CI it should always connect to the database.

@jyn514 jyn514 added E-medium Effort: This requires a fair amount of work P-medium Medium priority labels Jul 2, 2020
@pietroalbini
Copy link
Member

During CI it should not connect to the database, or do a build with and without the database: we want to make sure the cached data is correct and a build can work without a connection.

@Nemo157
Copy link
Member

Nemo157 commented Jul 2, 2020

By my understanding there will be a separate CLI utility that prepares the queries. So we can run a build job that calls sqlx-cli prepare or whatever the command is, then git diff --color --exit-code to show and fail if any file changed. Then the normal build can run without connecting to the database.

@Kixiron
Copy link
Member

Kixiron commented Jul 2, 2020

I think this’d be a great change, I’ll take a look at it once we’re async-capable

@jyn514 jyn514 added the C-technical-debt Category: This makes the code harder to read and modify, but has no impact on end users label Jul 14, 2020
@jyn514 jyn514 assigned jyn514 and unassigned jyn514 Jul 15, 2020
@Kixiron Kixiron added the S-waiting-on-async Status: waiting on async migration label Jul 18, 2020
@jyn514 jyn514 self-assigned this Sep 24, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 24, 2020

I have a WIP for this in https://github.com/jyn514/docs.rs/tree/sqlx, using block_on since we're still stuck on iron.

@Nemo157
Copy link
Member

Nemo157 commented Sep 24, 2020

(compare link: master...jyn514:sqlx)

@jyn514
Copy link
Member Author

jyn514 commented Sep 26, 2020

Currently blocked on launchbadge/sqlx#700.

@jyn514
Copy link
Member Author

jyn514 commented Sep 26, 2020

One footgun I found with this: If a migration fails and leaves the database in an inconsistent state, you can no longer compile the code to fix it, because query! gives an error. I think sqlx prepare should fix that though.

@Kixiron
Copy link
Member

Kixiron commented Sep 26, 2020

The sqlx cli tool can also run migrations, so that can be used to fix anything broken

@jyn514 jyn514 added E-hard Effort: This will require a lot of work and removed E-medium Effort: This requires a fair amount of work labels Oct 22, 2020
@jyn514 jyn514 removed their assignment Feb 15, 2022
@syphar
Copy link
Member

syphar commented Oct 12, 2022

short update here:
in my axum-experimental branch I came onto two issues trying sqlx:

  • composite types in arrays are not supported in parameters & queries ( I believe fixed in 0.7, but i didn't dig deeper on which exact issues / PRs fixed it). We use this for the releases.features field. If this is the only remaining issue we might think about migrating the whole field to JSONB, or wait for 0.7. solved too, I had to manually implement PgHasArrayType here.
  • binding arrays to parameters in queries also raised errors, short digging didn't give me a solution, long digging needed. (this is solved)

apart from that the user experience especially with the query! macro is really awesome. Sometimes an nullable field was returned as non-option and this needed tweaking the query to help the sqlx parser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-technical-debt Category: This makes the code harder to read and modify, but has no impact on end users E-hard Effort: This will require a lot of work P-medium Medium priority S-waiting-on-async Status: waiting on async migration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants