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

sqlite3_load_extension not working #532

Closed
bricelam opened this issue Jan 5, 2023 · 6 comments
Closed

sqlite3_load_extension not working #532

bricelam opened this issue Jan 5, 2023 · 6 comments

Comments

@bricelam
Copy link
Contributor

bricelam commented Jan 5, 2023

We're seeing issues loading mod_spatialite in dotnet/efcore#29584. It looks like a problem with the sqlite3_load_extension implementation.

Here's a repro. ❗ Be sure to install mod_spatialite too.

using SQLitePCL;
using static SQLitePCL.raw;

Batteries_V2.Init();

sqlite3_open(":memory:", out var db);
sqlite3_db_config(db, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1, out _);

// Don't judge :-)
Environment.SetEnvironmentVariable(
    "PATH",
    Environment.ExpandEnvironmentVariables(
        @"%PATH%;%USERPROFILE%\.nuget\packages\mod_spatialite\4.3.0.1\runtimes\win-x64\native"));

// Fails with SQLITE_ERROR and no message
var rc = sqlite3_load_extension(
    db,
    utf8z.FromString("mod_spatialite"),
    utf8z.FromString(null), // Also fails with "sqlite3_modspatialite_init"
    out var error);
if (rc != SQLITE_OK) Console.WriteLine($"error {rc}: {error.utf8_to_string()}");

If I use my own implementation, it works:

// Works!
var rc = MY_sqlite3_load_extension(db, "mod_spatialite\0", null, out _);
Debug.Assert(rc == SQLITE_OK);

// Really.
sqlite3_exec(
    db,
    "SELECT spatialite_version()",
    (_, values, _) =>
    {
        Console.WriteLine(values[0]);

        return 0;
    },
    null,
    out _);

[DllImport("e_sqlite3", CallingConvention = CallingConvention.Cdecl, EntryPoint = "sqlite3_load_extension")]
static extern int MY_sqlite3_load_extension(
    sqlite3 db,
    [MarshalAs(UnmanagedType.LPUTF8Str)] string file,
    [MarshalAs(UnmanagedType.LPUTF8Str)] string proc,
    [MarshalAs(UnmanagedType.LPUTF8Str)] out string error);
@ericsink
Copy link
Owner

ericsink commented Jan 5, 2023

In trying to diagnose #523 I discovered that load_extension is enabled only for the dynamic provider. See this comment:

#523 (comment)

At the time, I didn't understand how you had not run into this problem already, because, IIRC, the main reason I added support for load_extension was for spatialite use cases at your request.

May I assume this would explain the problem you are seeing?

ericsink added a commit that referenced this issue Jan 5, 2023
… for other provider configurations, likely to help with #532
@ericsink
Copy link
Owner

ericsink commented Jan 6, 2023

If my diagnosis is correct, pre-release 2.1.4-pre20230105221937 should contain a fix.

@julian94
Copy link

julian94 commented Jan 6, 2023

I tested this locally with Microsoft.Data.Sqlite 7.0.1.

I first ran it normally with normal Microsoft.Data.Sqlite 7.0.1, and it failed as expected.

Then I replaced SQLitePCLRaw.core.dll and SQLitePCLRaw.provider.e_sqlite3.dll with the new ones from the 2.1.4-pre20230105221937 build and I was able to load mod_spatialite and call "select spatialite_version()".

So it seems like your fix was correct.

@bricelam
Copy link
Contributor Author

bricelam commented Jan 6, 2023

At the time, I didn't understand how you had not run into this problem already, because, IIRC, the main reason I added support for load_extension was for spatialite use cases at your request.

Not catching this is totally on me. I made all the code changes, and our automated tests passed, so I ultimately merged the changes. After customers reported issues, I discovered that because of issues on Linux and macOS, our EF tests just get skipped when SpatiaLite can't be loaded, and I didn't notice they were suddenly being skipped on Windows too. The M.D.SQLite tests also just assert that the exception you get when trying to load an extension is different than when extensions are disabled. So, it just slipped through a big hole in our test coverage and verification.

@ericsink
Copy link
Owner

ericsink commented Jan 6, 2023

Ah. Well, I had opportunities to avoid this problem as well. I'm just glad the mystery is solved.

I'll plan a 2.1.4 probably early next week.

@ericsink
Copy link
Owner

2.1.4 is now on nuget.org

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

No branches or pull requests

3 participants