plugins-workspace icon indicating copy to clipboard operation
plugins-workspace copied to clipboard

[sql] Allows one of multiple identical connections to be close

Open wyhaya opened this issue 2 years ago • 5 comments
trafficstars

I'm making a database client that uses data.sqlite itself to store the client's data, and everything is fine, but it can't use data.sqlite again, otherwise when I shut it down it will shut down the client's own database as well, which we can't control because they use the same path.

let db1 = await Database.load('sqlite:/.../db.sqlite')
let db2 = await Database.load('sqlite:/.../db.sqlite')

// Only close db1
db1.close(db1.path)

// Error: attempted to acquire a connection on a closed pool
db2.select(`SELECT name FROM ...`)

Maybe we should use an ID to close the connection instead of path ?

let db = await Database.load('sqlite:/.../db.sqlite')
// {
//     path: '...',
//     id: 0
// }

// Use db.id close
db.close()

wyhaya avatar Feb 21 '23 07:02 wyhaya

I'd be interested in working on this (and on this repo in general), but I'm a little confused, would we want to add an id parameter or generate an id, and if the first, how would you do that in a backwards compatible way (so it doesn't break previous code)

Heidar-An avatar Jul 20 '23 18:07 Heidar-An

any updates on this? cuz i was using the try catch finally and closing, cuz i learned it was a good practice and yet it gave me this annoying error

ReiLoko4 avatar Oct 04 '24 16:10 ReiLoko4

With the current implementation, trying to close a single db instance doesn't really make sense.

Right now, every unique connection string gets a connection pool on the rust side, so in OP's example both db1 and db2 use the same pool. You cannot close individual connections in that pool (at least in the sqlx rust crate), the crate will handle all that internally.

afaik opening multiple pools for the same database is rarely useful so maybe the actual issue here is that we called it Database and not Pool something. -> Please tell me if i'm wrong because i'm not much of an sql dev. If you do, check out the docs i linked in the last paragraph as they explain Pools and why/how you should use them (no rust knowledge required).

I think what we actually should do here is to fix the docs (high prio), add some explicit way to create a new pool for OP's issue (low-ish prio), and then for v3 double check the api and naming conventions (for example if we really want to rename Database to something with Pool in its name).

cc @cijiugechu because i only thought about this because of your PR.

FabianLars avatar Oct 26 '24 18:10 FabianLars

afaik opening multiple pools for the same database is rarely useful so maybe the actual issue here is that we called it Database and not Pool something.

I have found that this is indeed a very rare use case, because in actual projects, the Database object appears as a global state and as a form of side effects. It should be uniformly managed by some mechanism (like redux middlewares), rather than being coupled with business logic.

cijiugechu avatar Oct 27 '24 12:10 cijiugechu

i run on this exact issue like wtf xD how is closing a db .. kills everything xD

TomieAi avatar Jul 22 '25 02:07 TomieAi