sqlite: add StatementSync.prototype.close() and [Symbol.dispose]()#64232
sqlite: add StatementSync.prototype.close() and [Symbol.dispose]()#64232araujogui wants to merge 4 commits into
Conversation
|
Review requested:
|
|
We are currently finalizing statements when the wrapper is deconstructed after GC. This PR only makes sense if we are going to change this to an explicit "user should finalize statements themselves" model, in which case we should also expose a named |
I think we can support both approaches. I don't see any issue with that. We can either let the GC finalize statements as they are today, or let users finalize them explicitly with a |
Honestly, I'm not a fan at all of relying on GC timing. It's too unpredictable and can expose GC timing details (which can be problematic on its own). We originally had |
|
That's the case I had in mind. I don't have a problem with the approach, we just need to be very clear to consumers that the contract is changing. |
cb5696c to
9ed6fae
Compare
This extends explicit resource management support to prepared statements, allowing a StatementSync to be deterministically finalized via a `using` declaration, mirroring the existing DatabaseSync and Session dispose methods. Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
9ed6fae to
7144743
Compare
Renegade334
left a comment
There was a problem hiding this comment.
LGTM. As a follow-up, we should update all of the statement examples in sqlite.md to demonstrate explicit teardown.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #64232 +/- ##
==========================================
- Coverage 90.25% 90.24% -0.02%
==========================================
Files 741 741
Lines 241165 241177 +12
Branches 45428 45426 -2
==========================================
- Hits 217667 217643 -24
- Misses 15069 15114 +45
+ Partials 8429 8420 -9
🚀 New features to boost your workflow:
|
No description provided.