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

sqlitex.Exec should protect against nesting #50

Open
FiloSottile opened this issue Feb 5, 2019 · 2 comments
Open

sqlitex.Exec should protect against nesting #50

FiloSottile opened this issue Feb 5, 2019 · 2 comments
Labels
enhancement New feature or request

Comments

@FiloSottile
Copy link
Contributor

I was fairly scared to realize that calling Prepare again with the same query resets the statement even if it's still in use, but I can't think of any way of protecting against that.

However, sqlitex.Exec knows until when its Stmt is in use, so it should be possible to protect against a nested Exec with the same query, for example by

I think nesting is the only risk, as concurrent access to the same connection is already not allowed.

@crawshaw
Copy link
Owner

crawshaw commented Feb 7, 2019

When I created the query cache I overloaded some of the existing methods. You can avoid the cache with PrepareTransient, but I was working on the assumption that caching is almost always what people want.

There is however a good argument against what I did: most people using SQLite spend a lot of time looking at the documentation for the C functions. And it's weird that Prepare, which looks so much like sqlite3_prepare, has different semantics.

One possibility is creating an sqlitex.Conn. It would wrap an sqlite.Conn and be the default object managed by sqlitex.Pool. All the extra semantics would then be in the sqlitex, which is clear.

One downside to this is sqlitex has a dependency on reflect, which will make it harder to get compiling on https://github.com/tinygo-org/tinygo. The cache logic is still useful in tinygo environments. That's an argument for introducing a new name on sqlite.Conn.

@AdamSLevy
Copy link
Collaborator

Personally I think that the caching behavior is well documented. But a note could be added that Prep will reset the cached Stmt before returning it, which will cause any existing handle to that statement to also be reset. Stmt's, like Conns are not threadsafe, so this seems like an edge case. but the docs could be improved.

@AdamSLevy AdamSLevy added the enhancement New feature or request label Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants