Skip to content

Reset a statement after run() the way better-sqlite3 does #238

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.

Sign up for GitHub

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

Open
penberg wants to merge 1 commit into
base: main
Choose a base branch
from
+175 −2
Open

Reset a statement after run() the way better-sqlite3 does #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions integration-tests/tests/async.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,82 @@ test.serial("Database.batch() rejects non-array argument", async (t) => {
await t.throwsAsync(() => db.batch("SELECT 1"), { instanceOf: TypeError });
});

test.serial("A statement that failed with SQLITE_BUSY does not block the next COMMIT", async (t) => {
const path = genDatabaseFilename();
const [holder] = await connect(path);
await holder.exec("PRAGMA journal_mode=WAL");
await holder.exec("CREATE TABLE t(x)");
const [db] = await connect(path, { timeout: 50 });
await holder.exec("BEGIN IMMEDIATE");
// Keep the failed statement referenced so that garbage collection cannot finalize it.
const begin = await db.prepare("BEGIN IMMEDIATE");
await t.throwsAsync(() => begin.run(), { code: "SQLITE_BUSY" });
await holder.exec("ROLLBACK");
await (await db.prepare("BEGIN IMMEDIATE")).run();
await (await db.prepare("INSERT INTO t VALUES (1)")).run();
await (await db.prepare("COMMIT")).run();
const row = await (await db.prepare("SELECT count(*) AS n FROM t")).get();
t.is(row.n, 1);
db.close();
holder.close();
for (const suffix of ["", "-wal", "-shm"]) fs.rmSync(path + suffix, { force: true });
});

test.serial("Statement.run() on INSERT ... RETURNING commits the write", async (t) => {
const path = genDatabaseFilename();
const [db] = await connect(path);
await db.exec("CREATE TABLE t(x)");
// Keep the statement referenced so that garbage collection cannot finalize it.
const insert = await db.prepare("INSERT INTO t VALUES (?) RETURNING x");
const info = await insert.run(42);
t.is(info.changes, 1);
t.is(info.lastInsertRowid, 1);
// The write is visible to, and does not block, another connection.
const [other] = await connect(path, { timeout: 100 });
t.is((await (await other.prepare("SELECT count(*) AS n FROM t")).get()).n, 1);
await (await other.prepare("INSERT INTO t VALUES (2)")).run();
// The same connection can run a transaction afterwards.
await (await db.prepare("BEGIN IMMEDIATE")).run();
await (await db.prepare("INSERT INTO t VALUES (3)")).run();
await (await db.prepare("COMMIT")).run();
t.is((await (await db.prepare("SELECT count(*) AS n FROM t")).get()).n, 3);
other.close();
db.close();
for (const suffix of ["", "-journal", "-wal", "-shm"]) fs.rmSync(path + suffix, { force: true });
});

test.serial("Statement.run() on a SELECT does not keep a read transaction open", async (t) => {
const path = genDatabaseFilename();
const [db] = await connect(path);
await db.exec("CREATE TABLE t(x)");
await db.exec("INSERT INTO t VALUES (1)");
// Keep the statement referenced so that garbage collection cannot finalize it.
const select = await db.prepare("SELECT x FROM t");
t.is((await select.run()).changes, 0);
// In rollback-journal mode an open read transaction would block this write.
const [other] = await connect(path, { timeout: 100 });
await (await other.prepare("INSERT INTO t VALUES (2)")).run();
t.is((await (await db.prepare("SELECT count(*) AS n FROM t")).get()).n, 2);
other.close();
db.close();
for (const suffix of ["", "-journal"]) fs.rmSync(path + suffix, { force: true });
});

test.serial("Statement.run() on a PRAGMA returning a row does not block the next COMMIT", async (t) => {
const path = genDatabaseFilename();
const [db] = await connect(path);
// Keep the statement referenced so that garbage collection cannot finalize it.
const pragma = await db.prepare("PRAGMA journal_mode=WAL");
await pragma.run();
await (await db.prepare("CREATE TABLE t(x)")).run();
await (await db.prepare("BEGIN IMMEDIATE")).run();
await (await db.prepare("INSERT INTO t VALUES (1)")).run();
await (await db.prepare("COMMIT")).run();
t.is((await (await db.prepare("SELECT count(*) AS n FROM t")).get()).n, 1);
db.close();
for (const suffix of ["", "-wal", "-shm"]) fs.rmSync(path + suffix, { force: true });
});

const connect = async (path_opt, options = {}) => {
const path = path_opt ?? "hello.db";
const provider = process.env.PROVIDER;
Expand Down
75 changes: 75 additions & 0 deletions integration-tests/tests/sync.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,81 @@ test.serial("Database.batch() rejects non-array argument", async (t) => {
t.throws(() => db.batch("SELECT 1"), { instanceOf: TypeError });
});

test.serial("A statement that failed with SQLITE_BUSY does not block the next COMMIT", async (t) => {
const path = genDatabaseFilename();
const [holder] = await connect(path);
holder.exec("PRAGMA journal_mode=WAL");
holder.exec("CREATE TABLE t(x)");
const [db, errorType] = await connect(path, { timeout: 50 });
holder.exec("BEGIN IMMEDIATE");
// Keep the failed statement referenced so that garbage collection cannot finalize it.
const begin = db.prepare("BEGIN IMMEDIATE");
t.throws(() => begin.run(), { instanceOf: errorType, code: "SQLITE_BUSY" });
holder.exec("ROLLBACK");
db.prepare("BEGIN IMMEDIATE").run();
db.prepare("INSERT INTO t VALUES (1)").run();
db.prepare("COMMIT").run();
t.is(db.prepare("SELECT count(*) AS n FROM t").get().n, 1);
db.close();
holder.close();
for (const suffix of ["", "-wal", "-shm"]) fs.rmSync(path + suffix, { force: true });
});

test.serial("Statement.run() on INSERT ... RETURNING commits the write", async (t) => {
const path = genDatabaseFilename();
const [db] = await connect(path);
db.exec("CREATE TABLE t(x)");
// Keep the statement referenced so that garbage collection cannot finalize it.
const insert = db.prepare("INSERT INTO t VALUES (?) RETURNING x");
const info = insert.run(42);
t.is(info.changes, 1);
t.is(info.lastInsertRowid, 1);
// The write is visible to, and does not block, another connection.
const [other] = await connect(path, { timeout: 100 });
t.is(other.prepare("SELECT count(*) AS n FROM t").get().n, 1);
other.prepare("INSERT INTO t VALUES (2)").run();
// The same connection can run a transaction afterwards.
db.prepare("BEGIN IMMEDIATE").run();
db.prepare("INSERT INTO t VALUES (3)").run();
db.prepare("COMMIT").run();
t.is(db.prepare("SELECT count(*) AS n FROM t").get().n, 3);
other.close();
db.close();
for (const suffix of ["", "-journal", "-wal", "-shm"]) fs.rmSync(path + suffix, { force: true });
});

test.serial("Statement.run() on a SELECT does not keep a read transaction open", async (t) => {
const path = genDatabaseFilename();
const [db] = await connect(path);
db.exec("CREATE TABLE t(x)");
db.exec("INSERT INTO t VALUES (1)");
// Keep the statement referenced so that garbage collection cannot finalize it.
const select = db.prepare("SELECT x FROM t");
t.is(select.run().changes, 0);
// In rollback-journal mode an open read transaction would block this write.
const [other] = await connect(path, { timeout: 100 });
other.prepare("INSERT INTO t VALUES (2)").run();
t.is(db.prepare("SELECT count(*) AS n FROM t").get().n, 2);
other.close();
db.close();
for (const suffix of ["", "-journal"]) fs.rmSync(path + suffix, { force: true });
});

test.serial("Statement.run() on a PRAGMA returning a row does not block the next COMMIT", async (t) => {
const path = genDatabaseFilename();
const [db] = await connect(path);
// Keep the statement referenced so that garbage collection cannot finalize it.
const pragma = db.prepare("PRAGMA journal_mode=WAL");
pragma.run();
db.prepare("CREATE TABLE t(x)").run();
db.prepare("BEGIN IMMEDIATE").run();
db.prepare("INSERT INTO t VALUES (1)").run();
db.prepare("COMMIT").run();
t.is(db.prepare("SELECT count(*) AS n FROM t").get().n, 1);
db.close();
for (const suffix of ["", "-wal", "-shm"]) fs.rmSync(path + suffix, { force: true });
});

const connect = async (path_opt, options = {}) => {
const path = path_opt ?? "hello.db";
const provider = process.env.PROVIDER;
Expand Down
26 changes: 24 additions & 2 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,18 @@ impl Statement {

let future = async move {
let _timeout_guard = register_timeout(&stmt, query_timeout);
stmt.run(params).await.map_err(Error::from)?;
let result = stmt.run(params).await;
// Reset the statement whether or not the step succeeded, as
// better-sqlite3 does. SQLite leaves a statement in progress after
// a step that returned a row (INSERT ... RETURNING, PRAGMA
// journal_mode=...) or that failed with SQLITE_BUSY. Until it is
// reset, an autocommit write stays uncommitted, its locks stay
// held, and every COMMIT on the connection fails with "SQL
// statements in progress". SQLite only records changes() when the
// statement halts, which the reset forces, so the counters below
// are read after it.
stmt.reset();
result.map_err(Error::from)?;
let changes = if conn.total_changes() == total_changes_before {
0
} else {
Expand Down Expand Up @@ -1386,7 +1397,18 @@ pub fn statement_run_sync(
let total_changes_before = conn.total_changes();
let start = std::time::Instant::now();

inner_stmt.run(params).await.map_err(Error::from)?;
let result = inner_stmt.run(params).await;
// Reset the statement whether or not the step succeeded, as
// better-sqlite3 does. SQLite leaves a statement in progress after
// a step that returned a row (INSERT ... RETURNING, PRAGMA
// journal_mode=...) or that failed with SQLITE_BUSY. Until it is
// reset, an autocommit write stays uncommitted, its locks stay
// held, and every COMMIT on the connection fails with "SQL
// statements in progress". SQLite only records changes() when the
// statement halts, which the reset forces, so the counters below
// are read after it.
inner_stmt.reset();
result.map_err(Error::from)?;
let changes = if conn.total_changes() == total_changes_before {
0
} else {
Expand Down
Loading