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 a "sqlite-unbundled" feature that dynamically links to system libsqlite3.so library #3507

Merged
merged 12 commits into from
Oct 2, 2024
Merged
17 changes: 11 additions & 6 deletions .github/workflows/sqlx.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ jobs:
strategy:
matrix:
runtime: [async-std, tokio]
linking: [sqlite, sqlite-unbundled]
needs: check
steps:
- uses: actions/checkout@v4
Expand All @@ -125,7 +126,11 @@ jobs:

- uses: Swatinem/rust-cache@v2
with:
key: "${{ runner.os }}-sqlite-${{ matrix.runtime }}-${{ matrix.tls }}"
key: "${{ runner.os }}-${{ matrix.linking }}-${{ matrix.runtime }}-${{ matrix.tls }}"

- name: Install system sqlite library
if: ${{ matrix.linking == 'sqlite-unbundled' }}
run: sudo apt-get install -y libsqlite3-dev

- run: echo "using ${DATABASE_URL}"

Expand All @@ -135,7 +140,7 @@ jobs:
- run: >
cargo test
--no-default-features
--features any,macros,sqlite,_unstable-all-types,runtime-${{ matrix.runtime }}
--features any,macros,${{ matrix.linking }},_unstable-all-types,runtime-${{ matrix.runtime }}
--
--test-threads=1
env:
Expand All @@ -151,8 +156,8 @@ jobs:
- run: >
cargo build
--no-default-features
--test sqlite-macros
--features any,macros,sqlite,_unstable-all-types,runtime-${{ matrix.runtime }}
--test ${{ matrix.linking }}-macros
--features any,macros,${{ matrix.linking }},_unstable-all-types,runtime-${{ matrix.runtime }}
env:
SQLX_OFFLINE: true
SQLX_OFFLINE_DIR: .sqlx
Expand All @@ -163,8 +168,8 @@ jobs:
- run: >
cargo test
--no-default-features
--test sqlite-macros
--features any,macros,sqlite,_unstable-all-types,runtime-${{ matrix.runtime }}
--test ${{ matrix.linking }}-macros
--features any,macros,${{ matrix.linking }},_unstable-all-types,runtime-${{ matrix.runtime }}
env:
DATABASE_URL: sqlite://tests/sqlite/sqlite.db
SQLX_OFFLINE: true
Expand Down
8 changes: 7 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ _rt-tokio = []
any = ["sqlx-core/any", "sqlx-mysql?/any", "sqlx-postgres?/any", "sqlx-sqlite?/any"]
postgres = ["sqlx-postgres", "sqlx-macros?/postgres"]
mysql = ["sqlx-mysql", "sqlx-macros?/mysql"]
sqlite = ["sqlx-sqlite", "sqlx-macros?/sqlite"]
sqlite = ["sqlx-sqlite/bundled", "sqlx-macros?/sqlite"]
sqlite-unbundled = ["sqlx-sqlite/unbundled", "sqlx-macros?/sqlite-unbundled"]

# types
json = ["sqlx-macros?/json", "sqlx-mysql?/json", "sqlx-postgres?/json", "sqlx-sqlite?/json"]
Expand Down Expand Up @@ -250,6 +251,11 @@ name = "sqlite-macros"
path = "tests/sqlite/macros.rs"
required-features = ["sqlite", "macros"]

[[test]]
name = "sqlite-unbundled-macros"
path = "tests/sqlite/macros.rs"
required-features = ["sqlite-unbundled", "macros"]

[[test]]
name = "sqlite-derives"
path = "tests/sqlite/derives.rs"
Expand Down
9 changes: 8 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,14 @@ be removed in the future.

- `mssql`: Add support for the MSSQL database server.

- `sqlite`: Add support for the self-contained [SQLite](https://sqlite.org/) database engine.
- `sqlite`: Add support for the self-contained [SQLite](https://sqlite.org/) database engine with SQLite bundled and statically-linked.

- `sqlite-unbundled`: The same as above (`sqlite`), but link SQLite from the system instead of the bundled version.
* Allows updating SQLite independently of SQLx or using forked versions.
* You must have SQLite installed on the system or provide a path to the library at build time.
See [the `rusqlite` README](https://github.com/rusqlite/rusqlite?tab=readme-ov-file#notes-on-building-rusqlite-and-libsqlite3-sys) for details.
* May result in link errors if the SQLite version is too old. Version `3.20.0` or newer is recommended.
* Can increase build time due to the use of bindgen.

- `any`: Add support for the `Any` database driver, which can proxy to a database driver at runtime.

Expand Down
1 change: 1 addition & 0 deletions sqlx-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ native-tls = ["sqlx/runtime-tokio-native-tls"]
mysql = ["sqlx/mysql"]
postgres = ["sqlx/postgres"]
sqlite = ["sqlx/sqlite"]
sqlite-unbundled = ["sqlx/sqlite-unbundled"]

# workaround for musl + openssl issues
openssl-vendored = ["openssl/vendored"]
Expand Down
3 changes: 3 additions & 0 deletions sqlx-cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ $ cargo install sqlx-cli --features openssl-vendored

# use Rustls rather than OpenSSL (be sure to add the features for the databases you intend to use!)
$ cargo install sqlx-cli --no-default-features --features rustls

# only for sqlite and use the system sqlite library
$ cargo install sqlx-cli --no-default-features --features sqlite-unbundled
```

## Usage
Expand Down
2 changes: 1 addition & 1 deletion sqlx-cli/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub async fn create(connect_opts: &ConnectOpts) -> anyhow::Result<()> {
let exists = crate::retry_connect_errors(connect_opts, Any::database_exists).await?;

if !exists {
#[cfg(feature = "sqlite")]
#[cfg(any(feature = "sqlite", feature = "sqlite-unbundled"))]
sqlx::sqlite::CREATE_DB_WAL.store(
connect_opts.sqlite_create_db_wal,
std::sync::atomic::Ordering::Release,
Expand Down
2 changes: 1 addition & 1 deletion sqlx-cli/src/opt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ pub struct ConnectOpts {
/// However, if your application sets a `journal_mode` on `SqliteConnectOptions` to something
/// other than `Wal`, then it will have to take the database file out of WAL mode on connecting,
/// which requires an exclusive lock and may return a `database is locked` (`SQLITE_BUSY`) error.
#[cfg(feature = "sqlite")]
#[cfg(any(feature = "sqlite", feature = "sqlite-unbundled"))]
#[clap(long, action = clap::ArgAction::Set, default_value = "true")]
pub sqlite_create_db_wal: bool,
}
Expand Down
6 changes: 3 additions & 3 deletions sqlx-core/src/any/kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub enum AnyKind {
#[cfg(feature = "mysql")]
MySql,

#[cfg(feature = "sqlite")]
#[cfg(any(feature = "sqlite", feature = "sqlite-unbundled"))]
Sqlite,

#[cfg(feature = "mssql")]
Expand Down Expand Up @@ -48,12 +48,12 @@ impl FromStr for AnyKind {
Err(Error::Configuration("database URL has the scheme of a MySQL database but the `mysql` feature is not enabled".into()))
}

#[cfg(feature = "sqlite")]
#[cfg(any(feature = "sqlite", feature = "sqlite-unbundled"))]
_ if url.starts_with("sqlite:") => {
Ok(AnyKind::Sqlite)
}

#[cfg(not(feature = "sqlite"))]
#[cfg(not(any(feature = "sqlite", feature = "sqlite-unbundled")))]
_ if url.starts_with("sqlite:") => {
Err(Error::Configuration("database URL has the scheme of a SQLite database but the `sqlite` feature is not enabled".into()))
}
Expand Down
3 changes: 2 additions & 1 deletion sqlx-macros-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ migrate = ["sqlx-core/migrate"]
# database
mysql = ["sqlx-mysql"]
postgres = ["sqlx-postgres"]
sqlite = ["sqlx-sqlite"]
sqlite = ["sqlx-sqlite/bundled"]
sqlite-unbundled = ["sqlx-sqlite/unbundled"]

# type integrations
json = ["sqlx-core/json", "sqlx-mysql?/json", "sqlx-postgres?/json", "sqlx-sqlite?/json"]
Expand Down
4 changes: 2 additions & 2 deletions sqlx-macros-core/src/database/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ mod sqlx {
#[cfg(feature = "postgres")]
pub use sqlx_postgres as postgres;

#[cfg(feature = "sqlite")]
#[cfg(any(feature = "sqlite", feature = "sqlite-unbundled"))]
abonander marked this conversation as resolved.
Show resolved Hide resolved
pub use sqlx_sqlite as sqlite;
}

Expand All @@ -61,7 +61,7 @@ impl_database_ext! {
row: sqlx::postgres::PgRow,
}

#[cfg(feature = "sqlite")]
#[cfg(any(feature = "sqlite", feature = "sqlite-unbundled"))]
impl_database_ext! {
sqlx::sqlite::Sqlite,
row: sqlx::sqlite::SqliteRow,
Expand Down
7 changes: 6 additions & 1 deletion sqlx-macros-core/src/database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ use sqlx_core::describe::Describe;
use sqlx_core::executor::Executor;
use sqlx_core::type_checking::TypeChecking;

#[cfg(any(feature = "postgres", feature = "mysql", feature = "sqlite"))]
#[cfg(any(
feature = "postgres",
feature = "mysql",
feature = "sqlite",
feature = "sqlite-unbundled"
))]
mod impls;

pub trait DatabaseExt: Database + TypeChecking {
Expand Down
2 changes: 1 addition & 1 deletion sqlx-macros-core/src/derives/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ fn expand_derive_decode_strong_enum(
));
}

if cfg!(feature = "sqlite") {
if cfg!(any(feature = "sqlite", feature = "sqlite-unbundled")) {
tts.extend(quote!(
#[automatically_derived]
impl<'r> ::sqlx::decode::Decode<'r, ::sqlx::sqlite::Sqlite> for #ident {
Expand Down
2 changes: 1 addition & 1 deletion sqlx-macros-core/src/derives/type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ fn expand_derive_has_sql_type_strong_enum(
}
}

if cfg!(feature = "sqlite") {
if cfg!(any(feature = "sqlite", feature = "sqlite-unbundled")) {
tts.extend(quote!(
#[automatically_derived]
impl sqlx::Type<::sqlx::Sqlite> for #ident {
Expand Down
2 changes: 1 addition & 1 deletion sqlx-macros-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub const FOSS_DRIVERS: &[QueryDriver] = &[
QueryDriver::new::<sqlx_mysql::MySql>(),
#[cfg(feature = "postgres")]
QueryDriver::new::<sqlx_postgres::Postgres>(),
#[cfg(feature = "sqlite")]
#[cfg(any(feature = "sqlite", feature = "sqlite-unbundled"))]
QueryDriver::new::<sqlx_sqlite::Sqlite>(),
];

Expand Down
1 change: 1 addition & 0 deletions sqlx-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ migrate = ["sqlx-macros-core/migrate"]
mysql = ["sqlx-macros-core/mysql"]
postgres = ["sqlx-macros-core/postgres"]
sqlite = ["sqlx-macros-core/sqlite"]
sqlite-unbundled = ["sqlx-macros-core/sqlite-unbundled"]

# type
bigdecimal = ["sqlx-macros-core/bigdecimal"]
Expand Down
4 changes: 3 additions & 1 deletion sqlx-sqlite/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ uuid = ["dep:uuid", "sqlx-core/uuid"]

regexp = ["dep:regex"]

bundled = ["libsqlite3-sys/bundled"]
unbundled = ["libsqlite3-sys/buildtime_bindgen"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we specifically didn't want this because of the hit to build times it would cause.

I suppose if people are willing to accept that to link their own SQLite, then it's fine, but then this needs to be clearly documented.

Unfortunately, it looks to be required because the current pre-generated bindings are for version 3.14.0 but we need sqlite3_prepare_v3() which was introduced in 3.20.0, which is 7 years old at this point: https://www.sqlite.org/changes.html#version_3_20_0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(A large part of the reason for going with bundled in the first place was just a general disdain amongst us for how slowly things move with system packages, especially in a lot of Linux distributions.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we really only use sqlite3_prepare_v3 to pass SQLITE_PREPARE_PERSISTENT as an optimization, but it does seem somewhat important: https://www.sqlite.org/c3ref/c_prepare_normalize.html#sqlitepreparepersistent

The SQLITE_PREPARE_PERSISTENT flag is a hint to the query planner that the prepared statement will be retained for a long time and probably reused many times. Without this flag, sqlite3_prepare_v3() and sqlite3_prepare16_v3() assume that the prepared statement will be used just once or at most a few times and then destroyed using sqlite3_finalize() relatively soon. The current implementation acts on this hint by avoiding the use of lookaside memory so as not to deplete the limited store of lookaside memory. Future versions of SQLite may act on this hint differently.

It looks like sqlite3_prepare_v2() behaves the same as _v3() with prepFlags = 0: https://github.com/sqlite/sqlite/blob/a95620c1414b06ac73b656b518ae364ff41f1e81/src/prepare.c#L954

The way the lookaside allocator is described to work, it seems quite detrimental to hold onto prepared statement handles using it for very long, as running out of lookaside memory means spilling over to regular malloc() calls, defeating the whole purpose of the lookaside allocator to begin with: https://www.sqlite.org/malloc.html#lookaside

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked repology and it seems that only ancient distros are shipping a too old sqlite, e.g. CentOS 7. (Some are shipping sqlite 2.x in addition and the naming is quite mixed.)


[dependencies]
futures-core = { version = "0.3.19", default-features = false }
futures-channel = { version = "0.3.19", default-features = false, features = ["sink", "alloc", "std"] }
Expand Down Expand Up @@ -55,7 +58,6 @@ default-features = false
features = [
"pkg-config",
"vcpkg",
"bundled",
"unlock_notify"
]

Expand Down
15 changes: 14 additions & 1 deletion sqlx-sqlite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
//!
//! ### Note: linkage is semver-exempt.
//! This driver uses the `libsqlite3-sys` crate which links the native library for SQLite 3.
//! For portability, we enable the `bundled` feature which builds and links SQLite from source.
//! With the "sqlite" feature, we enable the `bundled` feature which builds and links SQLite from
//! source.
//!
//! We reserve the right to upgrade the version of `libsqlite3-sys` as necessary to pick up new
//! `3.x.y` versions of SQLite.
Expand All @@ -20,6 +21,18 @@
//! ```
//!
//! and then upgrade these crates in lockstep when necessary.
//!
//! ### Dynamic linking
//! To dynamically link to a system SQLite library, the "sqlite-unbundled" feature can be used
//! instead.
//!
//! This allows updating SQLite independently of SQLx or using forked versions, but you must have
//! SQLite installed on the system or provide a path to the library at build time (See
//! [the `rusqlite` README](https://github.com/rusqlite/rusqlite?tab=readme-ov-file#notes-on-building-rusqlite-and-libsqlite3-sys)
//! for details).
//!
//! It may result in link errors if the SQLite version is too old. Version `3.20.0` or newer is
//! recommended. It can increase build time due to the use of bindgen.

// SQLite is a C library. All interactions require FFI which is unsafe.
// All unsafe blocks should have comments pointing to SQLite docs and ensuring that we maintain
Expand Down
2 changes: 1 addition & 1 deletion src/any/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub fn install_default_drivers() {
sqlx_mysql::any::DRIVER,
#[cfg(feature = "postgres")]
sqlx_postgres::any::DRIVER,
#[cfg(feature = "sqlite")]
#[cfg(any(feature = "sqlite", feature = "sqlite-unbundled"))]
sqlx_sqlite::any::DRIVER,
])
.expect("non-default drivers already installed")
Expand Down
7 changes: 5 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ pub use sqlx_mysql::{self as mysql, MySql, MySqlConnection, MySqlExecutor, MySql
#[doc(inline)]
pub use sqlx_postgres::{self as postgres, PgConnection, PgExecutor, PgPool, Postgres};

#[cfg(feature = "sqlite")]
#[cfg_attr(docsrs, doc(cfg(feature = "sqlite")))]
#[cfg(any(feature = "sqlite", feature = "sqlite-unbundled"))]
#[cfg_attr(
docsrs,
doc(cfg(any(feature = "sqlite", feature = "sqlite-unbundled")))
)]
#[doc(inline)]
pub use sqlx_sqlite::{self as sqlite, Sqlite, SqliteConnection, SqliteExecutor, SqlitePool};

Expand Down
2 changes: 1 addition & 1 deletion tests/ui-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ fn ui_tests() {
}
}

if cfg!(feature = "sqlite") {
if cfg!(any(feature = "sqlite", feature = "sqlite-unbundled")) {
if dotenvy::var("DATABASE_URL").map_or(true, |v| {
Path::is_relative(v.trim_start_matches("sqlite://").as_ref())
}) {
Expand Down
Loading