Skip to content

Conversation

@ayuusweetfish
Copy link

@ayuusweetfish ayuusweetfish commented Dec 30, 2025

Resolves #31732, following the proposed fix there.

Note: when the database is explicitly close()d, the associated resources (StatementSync and Session) are already dropped due to Database::close() taking away these references. Hence no additional change is needed to invalidate them; we simply check whether they are None.

stmt.set(None);

let _ = self.conn.borrow_mut().take();

A remaining question is how we should distinguish between the error codes StatementFinalized and AlreadyClosed. Apparently, statements are "finalized" (inner set to None) only when the database is closed, so it seems that these two codes are currently equivalent. The added test case expects two different error messages, but it does not seem very natural.

@CLAassistant
Copy link

CLAassistant commented Dec 30, 2025

CLA assistant check
All committers have signed the CLA.

@ayuusweetfish ayuusweetfish marked this pull request as ready for review December 31, 2025 05:35
…enoland#31722)

SQLite numbered positional parameters (?1, ?2, ?NNN) were incorrectly
treated as named parameters and skipped during binding, causing a
"column index out of range" error. This fix updates the parameter
binding logic to recognize that parameters whose names start with '?'
are positional and should be bound in order, just like anonymous
parameters (?).

Fixes denoland#31719

(cherry picked from commit d08d790)
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.

node:sqlite: Connection lost if DatabaseSync is garbage-collected

3 participants