adonisjs / lucid

AdonisJS SQL ORM. Supports PostgreSQL, MySQL, MSSQL, Redshift, SQLite and many more
https://lucid.adonisjs.com/
MIT License
1.02k stars 189 forks source link

migration:fresh & db:wipe fail with "ERROR: table sqlite_master may not be modified" #999

Closed Blackwidow-sudo closed 4 months ago

Blackwidow-sudo commented 4 months ago

Package version

20.1.0

Describe the bug

Hi, im currently trying out the new adonis version, but i cant run migration:fresh & db:wipe commands without errors.

I installed the web starter of Adonis v6, which is using the better-sqlite3 client and i always get these errors:

[ error ] delete from `sqlite_master` where `type` in ('table', 'index', 'trigger') and 1 = 1 - table sqlite_master may not be modified
          at Database.prepare (/home/user/ad6-test/node_modules/.pnpm/better-sqlite3@9.4.0/node_modules/better-sqlite3/lib/methods/wrappers.js:5:21)
          at Client_BetterSQLite3._query (/home/user/ad6-test/node_modules/.pnpm/knex@3.1.0_better-sqlite3@9.4.0/node_modules/knex/lib/dialects/better-sqlite3/index.js:35:34)
          at executeQuery (/home/user/ad6-test/node_modules/.pnpm/knex@3.1.0_better-sqlite3@9.4.0/node_modules/knex/lib/execution/internal/query-executioner.js:37:17)
          at Client_BetterSQLite3.query (/home/user/ad6-test/node_modules/.pnpm/knex@3.1.0_better-sqlite3@9.4.0/node_modules/knex/lib/client.js:154:12)
          at Runner.query (/home/user/ad6-test/node_modules/.pnpm/knex@3.1.0_better-sqlite3@9.4.0/node_modules/knex/lib/execution/runner.js:141:36)
          at ensureConnectionCallback (/home/user/ad6-test/node_modules/.pnpm/knex@3.1.0_better-sqlite3@9.4.0/node_modules/knex/lib/execution/internal/ensure-connection-callback.js:13:17)
          at Runner.ensureConnection (/home/user/ad6-test/node_modules/.pnpm/knex@3.1.0_better-sqlite3@9.4.0/node_modules/knex/lib/execution/runner.js:318:20)
          at async Runner.run (/home/user/ad6-test/node_modules/.pnpm/knex@3.1.0_better-sqlite3@9.4.0/node_modules/knex/lib/execution/runner.js:30:19)
          at async BetterSqliteDialect.dropAllTables (file:///home/user/ad6-test/node_modules/.pnpm/@adonisjs+lucid@20.1.0_@adonisjs+assembler@7.1.1_@adonisjs+core@6.2.3_better-sqlite3@9.4.0_luxon@3.4.4/node_modules/@adonisjs/lucid/build/src/dialects/base_sqlite.js:74:9)
          at async DbWipe.performDropTables (file:///home/user/ad6-test/node_modules/.pnpm/@adonisjs+lucid@20.1.0_@adonisjs+assembler@7.1.1_@adonisjs+core@6.2.3_better-sqlite3@9.4.0_luxon@3.4.4/node_modules/@adonisjs/lucid/build/commands/db_wipe.js:57:9)

When i remove the better-sqlite3 pkg and use the sqlite3 pkg as the client, it works fine. But there is not really more context to this issue. Its just installing the web-starter and running the commands. I tried it both on my M1-Macbook and a Windows-PC.

Reproduction repo

Just fresh install of the Adonis v6 web starter (with pnpm)

thetutlage commented 4 months ago

Verified, it does not work with better-sqlite3 package. So we must look into it

Blackwidow-sudo commented 4 months ago

Im not really into this stuff but i found something out that could be interesting:

I played around with better-sqlite3 and found out that PRAGMA writable_schema = 1 has no effect when better-sqlite3 isn't switched to unsafeMode. That would explain why the delete on sqlite_master fails.

But still, i dont understand how the tests didn't catch that (though i did not read through them).

Edit: I did go one layer up to knex for my testing:

import knex from "knex";

const db = knex({
    client: "better-sqlite3",
    connection: {
        filename: "test.sqlite3",
    },
    useNullAsDefault: true,
});

/* This fails, however when you uncomment the unsafeMode calls, it works */
db.client.acquireRawConnection().then((conn) => {
    // conn.unsafeMode(true);
    conn.pragma("writable_schema = 1");

    conn.prepare(
        "DELETE FROM sqlite_master WHERE type IN ('table', 'index', 'trigger')"
    ).run();

    conn.pragma("writable_schema = 0");
    // conn.unsafeMode(false);
});

db.destroy();

I couldn't find any other method to call the better-sqlite3 method from knex. Even acquireRawConnection isn't even documented (thanks copilot 😂)

RomainLanz commented 4 months ago

Closing in favor of the PR.