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

Break drivers out into separate crates, clean up some technical debt #2039

Merged
merged 2 commits into from
Feb 2, 2023

Conversation

abonander
Copy link
Collaborator

@abonander abonander commented Aug 11, 2022

Original Text

The goal of this PR is to make it possible to build SQLx without any runtime features enabled, or with just runtime-async-std or runtime-tokio enabled (without a TLS provider). As a side-effect, multiple runtime and TLS features can be enabled simultaneously, in which case Tokio (if a Tokio context is available) and native-tls take precedent over async-std and rustls, respectively.

Along the way, I'm fixing some perf issues re. buffering that's causing us to score very poorly on Diesel's metrics suite.

Preliminary benchmarks show a ~50% improvement on "Trivial query (postgres)" for 10,000 rows, bringing SQLx's time back to the same order of magnitude as Diesel's (7ms vs 3ms).

Amended Text

To support developing our planned SQLx Pro offering (#1616) while being able to reuse as much code as possible, we decided to separate out the database drivers into individual crates and do some other significant refactors that have been waiting in the wings. Since I already had a significant refactor pending in this PR, it made sense to roll these changes into it.

Breaking Change Summary

This is not an exhaustive list of breaking changes for 0.7.0, only those that are part of this PR. The full list will be in the CHANGELOG.md entry as usual.

  • All deprecated items have been removed.
  • The mssql feature and associated database driver has been deleted from the source tree. It will return as part of our planned SQLx Pro offering as a from-scratch rewrite with extra features (such as TLS) and type integrations that were previously missing.
  • The runtime-actix-* features have been deleted. They were previously changed to be aliases of their runtime-tokio-* counterparts for backwards compatibility reasons, but their continued existence is misleading as SQLx has no special knowledge of Actix anymore.
    • To fix, simply replace the runtime-actix-* feature with its runtime-tokio-* equivalent.
  • The git2 feature has been removed. This was a requested integration from a while ago that over time made less and less sense to be part of SQLx itself. We have to be careful with the crates we add to our public API as each one introduces yet another semver hazard. The expected replacement is to make #[derive(sqlx::Type)] useful enough that users can write wrapper types for whatever they want to use without SQLx needing to be specifically aware of it.
  • The Executor impls for Transaction and PoolConnection have been deleted because they cannot exist in the new crate architecture without rewriting the Executor trait entirely.
    • To fix this breakage, simply add a dereference where an impl Executor is expected, as they both dereference to the inner connection type which will still implement it:
      • &mut transaction -> &mut *transaction
      • &mut connection -> &mut *connection
    • These cannot be blanket impls as it triggers an overflow in the compiler due to the lack of lazy normalization, and
      the driver crates cannot provide their own impls due to the orphan rule.
    • We're expecting to do another major refactor of traits when Rust 1.65 releases with Generic Associated Types (GATs).
      This will mean another major release of SQLx but ideally most API usage will not need to change significantly, if at all.
  • The fields of Migrator are now #[doc(hidden)] and semver-exempt; they weren't meant to be public.
  • The offline feature has been removed from the sqlx facade crate and is enabled unconditionally as most users are expected to have enabled it anyway and disabling it doesn't seem to appreciably affect compile times.
  • The decimal feature has been renamed to rust_decimal to match the crate it actually provides integrations for.
  • AnyDriver and AnyConnection now require either sqlx::any::install_drivers() or sqlx::any::install_default_drivers() to be called at some point during the process' lifetime before the first connection is made, as the set of possible drivers is now determined at runtime. This was determined to be the least painful way to provide knowledge of database drivers to Any without them being hardcoded.

@abonander abonander force-pushed the ab/rt-refactor branch 7 times, most recently from 5756499 to 3e93acd Compare August 13, 2022 00:35
@abonander abonander force-pushed the ab/rt-refactor branch 13 times, most recently from 9ae5cc1 to 88e5885 Compare August 24, 2022 02:18
@abonander abonander changed the base branch from main to 0.7-dev September 15, 2022 00:35
@abonander abonander force-pushed the ab/rt-refactor branch 2 times, most recently from 1204b85 to 0d27f44 Compare September 15, 2022 04:12
@abonander abonander changed the title Refactor runtime features to be optional, TLS features to be orthogonal, and improve performance WIP huge refactors Sep 15, 2022
@abonander abonander force-pushed the ab/rt-refactor branch 5 times, most recently from e02b9cb to ae2b293 Compare September 21, 2022 03:33
@abonander abonander force-pushed the ab/rt-refactor branch 2 times, most recently from 6ae0a86 to 79d6441 Compare February 1, 2023 20:49
@abonander abonander marked this pull request as ready for review February 1, 2023 20:49
@abonander abonander changed the title WIP huge refactors Break drivers out into separate crates, clean up some technical debt Feb 1, 2023
@abonander abonander force-pushed the ab/rt-refactor branch 5 times, most recently from 28df9fb to ebba100 Compare February 1, 2023 23:13
@abonander
Copy link
Collaborator Author

@tyrelr I'm getting a weird failure in the SQLite describe tests on this branch: https://github.com/launchbadge/sqlx/actions/runs/4069586336/jobs/7009510856#step:7:1062

also cleans up significant technical debt
@abonander
Copy link
Collaborator Author

Once I get the other checks passing I'll merge this and then we can figure out what's going on.

@abonander abonander merged commit 7c05ea6 into 0.7-dev Feb 2, 2023
@abonander abonander deleted the ab/rt-refactor branch February 2, 2023 00:47
@tyrelr
Copy link
Contributor

tyrelr commented Feb 3, 2023

@tyrelr I'm getting a weird failure in the SQLite describe tests on this branch: https://github.com/launchbadge/sqlx/actions/runs/4069586336/jobs/7009510856#step:7:1062

I'm assuming a new cargo.lock lead to a new sqlite binary, which happened to optimize that query in a way that broke the logic? But not a huge deal since (at least on my machine) the rebased PR #2253 fixes it.

abonander added a commit that referenced this pull request Feb 18, 2023
…2039)

* WIP rt refactors

* refactor: break drivers out into separate crates

also cleans up significant technical debt
abonander added a commit that referenced this pull request Feb 21, 2023
…2039)

* WIP rt refactors

* refactor: break drivers out into separate crates

also cleans up significant technical debt
fversaci added a commit to fversaci/reddit_fetcher that referenced this pull request Mar 7, 2023
Aandreba pushed a commit to Aandreba/sqlx that referenced this pull request Mar 31, 2023
…aunchbadge#2039)

* WIP rt refactors

* refactor: break drivers out into separate crates

also cleans up significant technical debt
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.

2 participants