ericsink / SQLitePCL.raw

A Portable Class Library (PCL) for low-level (raw) access to SQLite
Apache License 2.0
512 stars 106 forks source link

sqlite3_load_extension not working #532

Closed bricelam closed 1 year ago

bricelam commented 1 year ago

We're seeing issues loading mod_spatialite in https://github.com/dotnet/efcore/issues/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 commented 1 year ago

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

https://github.com/ericsink/SQLitePCL.raw/issues/523#issuecomment-1306076412

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 commented 1 year ago

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

julian94 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

2.1.4 is now on nuget.org