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 extension support for SQLite #2062

Merged
merged 2 commits into from
Sep 1, 2022

Conversation

bradfier
Copy link
Contributor

Implements #1460

I was trying to find a way to use extensions (specifically vsv and spatialite) with SQLite from Rust, and while Rusqlite supports them, my perferred SQL crate did not, hence this PR to try and rectify that situation.

I found the issue mentioned above and took note of the suggestions made, that we should probably implement this as conservatively as we can, please see the commit message for more specifics.

Testing

I would appreciate some guidance on how to test this from the integration test suite. Something like this will exercise the code in question, the more difficult problem is "How do we get the extension shared object to the test context?".

#[sqlx_macros::test]
async fn it_opens_with_extension() -> anyhow::Result<()> {
    let opts = SqliteConnectOptions::from_str(&dotenvy::var("DATABASE_URL")?)?
        .extension("vsv");

    let conn = SqliteConnection::connect_with(&opts).await?;
    conn.close().await?;

    Ok(())
}

Just sticking a compiled object in isn't appropriate, as it's architecture and OS dependent, and unlike the other databases we don't spin a Docker container up, otherwise we could modify it to add an extension package.

Would it be possible to add a simple extension as a .c file and have a build.rs compile it only in a test context? sqlite3-ipaddr would be a good minimal example as it's only a few hundred lines long and has no external dependencies except for sqlite3ext.h.

@abonander
Copy link
Collaborator

I don't see a problem with just grabbing a precompiled binary: https://github.com/nalgeon/sqlean/releases/tag/0.15.2

This can be done in the CI script itself where the OS is known, mark the test with #[cfg(sqlite_ipaddr)] so it's not run normally, and add RUSTFLAGS: --cfg sqlite_ipaddr to the environment variables.

@bradfier bradfier force-pushed the sqlx-extensions branch 7 times, most recently from 9d1b241 to b83689f Compare August 25, 2022 12:19
@bradfier
Copy link
Contributor Author

I've added that as suggested and enabled it in x.py and in GitHub Actions. Hopefully this should run successfully, it worked in my fork when I tried it on-push.

@bradfier bradfier force-pushed the sqlx-extensions branch 5 times, most recently from 28e82f0 to b32fa82 Compare August 27, 2022 14:24
@bradfier
Copy link
Contributor Author

I've sorted out the CI failures, which were due to GHA's working directory not being the same between steps, see CI on my fork for a successful run.

While SQLite supports loading extensions at run-time via either the C
API or the SQL interface, they strongly recommend [1] only enabling the C
API so that SQL injections don't allow attackers to run arbitrary
extension code.

Here we take the most conservative approach, we enable only the C
function, and then only when the user requests extensions be loaded in
their `SqliteConnectOptions`, and disable it again once we're done
loading those requested modules. We don't add any support for loading
extensions via environment variables or connection strings.

Extensions in the options are stored as an IndexMap as the load order
can have side effects, they will be loaded in the order they are
supplied by the caller.

Extensions with custom entry points are supported, but a default API
is exposed as most users will interact with extensions using the
defaults.

[1]: https://sqlite.org/c3ref/enable_load_extension.html
Extends x.py to download an appropriate shared object file for supported
operating systems, and uses wget to fetch one into the GitHub Actions
context for use by CI.

Overriding LD_LIBRARY_PATH for only this specific DB minimises the
impact on the rest of the suite.
@abonander abonander merged commit 20877d8 into launchbadge:main Sep 1, 2022
@framp
Copy link
Contributor

framp commented Nov 6, 2022

Thanks a lot for this!

I was wondering, how are you dealing with sqlx-cli?
There is no way to specify extensions in the connection string so you can't load extensions when running migrations

A typical usecase would be to create a geometry column with the spatialite extension

CREATE TABLE locations
...
SELECT AddGeometryColumn('locations', 'geometry', 4326, 'POINT', 2);

@yuyoyuppe
Copy link

Same issue here. My migrations must contain BLOB data, so I was going to use readfile.

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