search

crawshaw / sqlite

Go SQLite3 driver
ISC License
561 stars 67 forks source link

Per-connection pragmas (synchronous) #101

Closed anacrolix closed 2 years ago

anacrolix commented 3 years ago

Some pragmas, like synchronous are required to be set on each/all connections to ensure that behaviour. With sqlitex.Pool, it's not clear how to ensure a pragma is set for all the connections in the pool. Should I drain the pool of its connections after initialization and execute the pragma on each of them then put them back, or is there a callback or option somewhere I'm missing for this.

AdamSLevy commented 3 years ago

There is no API to run a query on all Conns in a Pool. So yes you need to Drain the pool and execute the pragma on each one and then put them back.

I agree there could be a cleaner API for this use case, as there are many pragmas that need to be set on all connections. I'd consider a PR that added something like that.

An alternative for many pragmas is to specify them as a param in the URI used to open the connection.

anacrolix commented 3 years ago

@AdamSLevy would you be aware of whether synchronous could be set as a URI param? What about mmap_size?

AdamSLevy commented 3 years ago

I don't know off top. The official SQLite docs should cover it. you can also just try it, or just apply it to all conns in the pool.

anacrolix commented 3 years ago

This is how I've solved it in the meanwhile: https://github.com/anacrolix/torrent/blob/78c77c0b455e761350a215a54c2dec8e8c7168fe/storage/sqlite/sqlite-storage.go#L326-L347.

AdamSLevy commented 2 years ago

So I've been giving this some thought because initializing all connections in a pool is a common use-case.

I think that outside of initialization running a query on all Conns in a Pool is an anti-pattern. So I think this should be an optional part of the initialization of a Pool.

I'm a little concerned with the pitfall of a user running non-idempotent queries during initialization. For example if you ran an INSERT during this script it would be run N times. This can be documented but it still feels like a potential pitfall.

I could scan the queries for INSERT, UPDATE or DELETE and block those scripts... but it won't be real query parsing so it would be a little weak, and it starts to feel like overkill.

I considered limiting it to only PRAGMAs, but that precludes using this to set up VIEWS which is necessary to do per connection.

I think I'm just going to document it clearly with a warning and just exec the scripts.

AdamSLevy commented 2 years ago

Another complication I just realized as I was implementing this is that by offering an init script it becomes important to offer a timeout mechanism. The proper way is to expose a context. So perhaps I will actually just make this a new Pool factory function rather than trying to extend Open