This particular loop occurs a lot.
Give it a name and make it concise.
No functional changes.
The goal is to make upcoming changes a bit clearer.

The multi-statement path was subject to both double-frees and leaks.

Having allocs declared once and re-used across loop iterations
made it hard to manage correctly. Reduce its scope to within the
inner closure and defer freeAllocs there.

Even within a loop, it was easy to mismanage; force callers to deal
with this consistently by passing a pointer to newRows and zeroing there.

Take care to Close rows objects from previous iterations when
a subsequent statement in a multi-statement query fails.

The tests here demonstrate both double-frees and
leaked memory without these changes.

I noticed while in this code that in the optimized
single-statement path, there are some unnecessary calls to
reset/clearBindings after newRows. When newRows fails, its internal
defer calls r.Close, which finalizes the prepared statement.
The subsequent reset and clearBindings calls would then operate
on an already-finalized handle. This would be bad, except that
columnCount/columnName cannot fail on a valid pstmt.
Therefore, the error path is dead code.
I didn't touch it, even though pedantically it looks wrong.
There might also be some dangling SQLITE_STATIC references.
Again, this appears to be safe in practice, so I left it alone.
A follow-up could make it conform to the docs
and ensure extra safety.

I don't plan to do any of this follow-up work;
I am focused for now only on demonstrable, directly testable issues.

improve memory safety of allocs in stmt.query

See merge request !96

The column values and rowid were swapped;
vtabUpdateTrampoline wasn't following SQLite's xUpdate contract.

The existing tests didn't catch this because they used
only a single column. Add more columns, and expand the test.

This is pretty standard Go fare,
but it's worth calling out as a cheap one-liner.

The alternative is to remove this restriction,
but I don't think it's worth the mutex overhead.

fix vtab argv layout

See merge request cznic/sqlite!97

Josh Bleecher Snyder authored Apr 03, 2026 and cznic committed Apr 03, 2026

iKey2 is an int64, not two int32s.

The trampoline's parameter list appears to have been modeled after the
internal SQLite function sqlite3VdbePreUpdateHook, which takes
(i64 iKey1, int iReg, int iBlobWrite).

But that internal function computes iKey2 from iReg before invoking the
public callback, whose actual signature ends with
(sqlite3_int64 iKey1, sqlite3_int64 iKey2).

The trampoline declared the final slot as (iReg int32, iBlobWrite
int32) instead of (iKey2 int64), which works except for large rowids.
Depending on the arch, this likely manifests as truncation.

It's rather unclear how important this is, but on the other hand,
the fix is small and targeted and highly testable,
and for e.g. snowflake-style IDs, it'd show up as silent corruption.

fix preUpdateHookTrampoline signature

See merge request !98

document that registration must precede Open

See merge request !99

tls.Alloc is inappropriate here, because sqlite
owns this pointer and may choose to resize it;
we must use a sqlite allocator.

The test triggers a crash using a close,
but it is also possible (albeit longer and noisier)
to trigger via a re-alloc/re-size.

fix Deserialize allocator

See merge request !100

How to handle Deserialize(nil) is a judgment call.

sqlite3_deserialize(db, schema, NULL, 0, 0, ...) disconnects and starts fresh.
But in database/sql world, connections are pooled, so that seems bad.
But nil as an input is guaranteed to fail, possibly in ugly ways, if not handled explicitly.

Given that we're in Go, and creating a new DB is trivial,
opt for safety and explicitness and early feedback,
and declare Deserialize(nil) to be an error and handle it explicitly.

reject Deserialize(nil) with clear error

See merge request !101

Josh Bleecher Snyder authored Apr 04, 2026 and cznic committed Apr 05, 2026

Similar to 5e394aa2,
except without practical consequences, and therefore untestable.

Low priority, but trivial.

Fixing only to avoid leaving a code rake.

fix commitHookTrampoline and rollbackHookTrampoline signatures

See merge request !102

Josh Bleecher Snyder authored Apr 04, 2026 and cznic committed Apr 06, 2026

`go test -race .` fails for me without this fix.

The sqlite3_module struct was allocated on the Go heap and passed
to sqlite3_create_module_v2 as a uintptr. Later when we access fields
through unsafe.Pointer arithmetic, Go's checkptr instrumentation
rejects the dereference because the pointer was round-tripped through
a uintptr in C-allocated memory.

Use libc.Xcalloc instead, matching the pattern in mutex.go and the VFS code.

The allocation is intentionally never freed; modules are registered once
and must outlive every connection that references them.
There is no UnregisterModule API.

This is a prerequisite change to another fix in the vtab code
which requires `go test -race` to reproduce.

allocate sqlite3_module with C allocator to fix checkptr

See merge request !103

Josh Bleecher Snyder authored Apr 05, 2026 and cznic committed Apr 06, 2026

The non-recursive branch of mutexTry wrote mutex.id
with a plain store while mutexHeld/mutexNotheld
read it with atomic.LoadInt32.

Mixing atomic and non-atomic accesses to the
same word is a data race under Go's memory model.

Use atomic.StoreInt32, consistent with every other
write site (mutexEnter, mutexLeave, and the
recursive branch of mutexTry).

I have a regression test for this, but I'm not
convinced it is worth adding. It's a lot of code
and compute for an obvious fix that is unlikely to regress.

fix data race on mutex.id in mutexTry non-recursive path

See merge request !104

don't leak Backup.Commit dest conn on error

See merge request !105

Josh Bleecher Snyder authored Apr 04, 2026 and cznic committed Apr 06, 2026

The exec path treated SQLITE_ROW identically to SQLITE_DONE,
calling newResult after a single step.

For DML with RETURNING clauses that affect multiple rows,
this meant the statement was never stepped to completion,
causing sqlite3_changes() to return stale counts.

Continue stepping until SQLITE_DONE before reading the result.

This could manifest as a performance regression: we're doing strictly
more work by draining all the rows instead of abandoning them.
But as Russ Cox says, I can make anything fast if it doesn't need
to be correct.

This brings this Exec implementation in line with C sqlite3_exec
in several ways:

- Row draining: C sqlite3_exec loops sqlite3_step until rc != SQLITE_ROW,
  then finalizes.

- Result after completion: C sqlite3_exec  does finalize-after-loop.

- Unconditional drain: C sqlite3_exec with a NULL callback still
  steps through every row.

- Partial commit on interruption: C returns SQLITE_ABORT if the
  callback returns non-zero mid-drain, but already-stepped rows stay
  committed. (Use an explicit transaction for atomicity.)

- Multi-statement iteration: C's outer loop prepares and fully steps
  each sub-statement.

step Exec to completion for SQLITE_ROW

See merge request !106

See [GitLab issue #249](#249), thanks Emrecan BATI!

registerSingleModule set FiVersion = 1, but also populated
FxSavepoint, FxRelease, and FxRollbackTo. SQLite gates dispatch
of those callbacks on iVersion >= 2, so they were silently ignored.
Begin/Sync/Commit/Rollback (version 1 methods) worked fine,
which made the gap easy to miss.

iVersion was 1 from the initial commit.
Version 2 does not change any version-1 callback behavior.
The only behavioral change is that SQLite now dispatches the three
savepoint callbacks, and it still checks each function pointer is
non-zero before calling. The trampolines already handle modules that
don't implement the methods (type-assert, return SQLITE_OK).
So the bump to version 2 is safe.

Version 3 adds xShadowName.
Version 4 adds xIntegrity.
Neither callback is set, so those remain inert;
there's no need to upgrade past 2.

Extend the existing updater vtab test to implement Transactional
and verify that savepoint/release/rollbackto are dispatched.

set sqlite3_module.iVersion to 2 so savepoint callbacks fire

See merge request !107

Josh Bleecher Snyder authored Apr 06, 2026 and cznic committed Apr 07, 2026

The current code assumes a well-behavior io.Reader,
which works until it doesn't.

Add protection: convert f.Read(b) to io.ReadFull(f, b).
While we're here, use the new clear built-in.

Add test coverage, including tests that would have caught
the initial bugs and tests ensuring tail clearing.

fix vfsRead reader handling

See merge request !108

Josh Bleecher Snyder authored Apr 05, 2026 and cznic committed Apr 07, 2026

Two upcoming commits need a bit more control over
exactly which db is used to generate an error.

In anticipation of that, do some light refactoring.

While we're here handle db == 0 gracefully:
skip sqlite3_errmsg, which would misleadingly
return "out of memory" for a NULL handle.

extract errstrForDB from conn.errstr

See merge request !109

Per SQLite docs, sqlite3_backup_init stores errors
on the destination database handle.

In backup mode the destination is remoteConn.db; in
restore mode it is c.db. Previously errcode was always
read from remoteConn.db (wrong for restore) and errmsg
was always read from c.db via c.errstr (wrong for backup).
Read both from the actual destination handle.

read error from correct db handle on backup init failure

See merge request !111

Josh Bleecher Snyder authored Apr 05, 2026 and cznic committed Apr 08, 2026

When sqlite3_open_v2 fails, the SQLite docs require the caller to close
the handle it may have allocated. The previous code did not, leaking the
sqlite3 struct and potentially VFS-level state.

Additionally, newConn did not close libc.TLS on the openV2 failure path,
leaking C-heap memory.

Lastly, the error message was misleading: errstr called sqlite3_errmsg
with c.db == 0 (not yet set), which always returns "out of memory",
producing errors like "unable to open database file: out of memory (14)"
regardless of the actual cause.

Fix all of those.

fix openV2 handle leak, TLS leak, and misleading error on failed open

See merge request !112