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

[bun:sqlite] Support sqlite3_file_control, better closing behavior, implement Disposable #10262

Merged
merged 3 commits into from
Apr 15, 2024

Conversation

Jarred-Sumner
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner commented Apr 14, 2024

What does this PR do?

You can now manage the lifetime of a Database instance in bun:sqlite via the using keyword:

import { Database } from "bun:sqlite";

{
  using db = new Database("mydb.sqlite");
  using query = db.query("select 'Hello world' as message;");
  console.log(query.get()); // => { message: "Hello world" }
}

Database#fileControl lets you use the sqlite3_file_control API

For example:

import { Database } from "bun:sqlite";

const db = new Database();
// Ensure WAL mode is NOT persistent
// this prevents wal files from lingering after the database is closed
db.fileControl(SQL.constants.SQLITE_FCNTL_PERSIST_WAL, 0);

Automatically closing databases when the Database object GC'd was not implemented correctly before (bun would only automatically close the database before process exit). This has now been fixed

database.close(true) will now throw an error if the database has any pending resources associated with it.

Fixes #2537

How did you verify your code works?

Tests

Copy link
Contributor

github-actions bot commented Apr 14, 2024

Copy link
Contributor

github-actions bot commented Apr 14, 2024

@Jarred-Sumner 2 files with test failures on bun-darwin-aarch64:

View test output

#d71158f4b3921a5900fbfe9cecc5d92152768a7b

Copy link
Contributor

github-actions bot commented Apr 14, 2024

@gvilums gvilums merged commit 3f10d52 into main Apr 15, 2024
27 of 33 checks passed
@gvilums gvilums deleted the jarred/sqlite-close branch April 15, 2024 20:06
@pekeler
Copy link

pekeler commented Apr 17, 2024

This doesn't work well for me in 1.1.4. Typescript doesn't know about the SQLITE_FCNTL_* constants, the fileControl() function, the close() parameter, and is complaining about the db not having a dispose() method when using 'using'.
image

@gvilums
Copy link
Contributor

gvilums commented Apr 17, 2024

@pekeler this might be an issue with our type declarations - have you tried running the code and seeing if it works?

@pekeler
Copy link

pekeler commented Apr 26, 2024

@gvilums the code works for simple use cases (ignoring the type errors). In scenarios where the DB is used for a bit more than one simple query, db.close(true); produces a database is locked error. I do keep track of all my prepared statements and make sure they're finalized before closing the DB, so something doesn't feel right.

@gvilums
Copy link
Contributor

gvilums commented Apr 26, 2024

Hm, that does indeed sound like something is going wrong in bun. Could you open an issue for this?

@pekeler
Copy link

pekeler commented May 28, 2024

@gvilums I have so far failed to create a simple project to reproduce this issue. However, I also noticed that db.close(true) doesn't work after running migrations and created an issue with a simple project that reproduces the issue here #11418. Hopefully the underlying cause is the same.

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.

Database.prepare without .finalize results in left-over wal file
3 participants