diff --git a/backup_test.go b/backup_test.go index b3ad0b58..7406a36b 100644 --- a/backup_test.go +++ b/backup_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 diff --git a/callback.go b/callback.go index b7df2be7..7dcf08d1 100644 --- a/callback.go +++ b/callback.go @@ -29,7 +29,6 @@ import ( "math" "reflect" "sync" - "sync/atomic" "unsafe" ) @@ -104,27 +103,28 @@ type handleVal struct { val any } -var handleLock sync.Mutex -var handleVals atomic.Value // stores map[unsafe.Pointer]handleVal +// handleVals maps unsafe.Pointer handles to handleVal. A sync.Map keeps +// lookups lock-free on the hot callback path while insertion and removal +// stay O(1); the previous copy-on-write map made every registration copy +// the whole table, so opening N connections (each registering several +// functions) was quadratic in time and allocation. +var handleVals sync.Map func newHandle(db *SQLiteConn, v any) unsafe.Pointer { - val := handleVal{db: db, val: v} var p unsafe.Pointer = C.malloc(C.size_t(1)) if p == nil { panic("can't allocate 'cgo-pointer hack index pointer': ptr == nil") } - - handleLock.Lock() - defer handleLock.Unlock() - - next := cloneHandleVals(len(loadHandleVals()) + 1) - next[p] = val - handleVals.Store(next) + handleVals.Store(p, handleVal{db: db, val: v}) return p } func lookupHandleVal(handle unsafe.Pointer) handleVal { - return loadHandleVals()[handle] + v, ok := handleVals.Load(handle) + if !ok { + return handleVal{} + } + return v.(handleVal) } func lookupHandle(handle unsafe.Pointer) any { @@ -134,55 +134,20 @@ func lookupHandle(handle unsafe.Pointer) any { // deleteHandle releases a single handle created by newHandle. It is a no-op // if the handle is unknown (e.g. already released). func deleteHandle(handle unsafe.Pointer) { - handleLock.Lock() - defer handleLock.Unlock() - - current := loadHandleVals() - if _, ok := current[handle]; !ok { - return + if _, ok := handleVals.LoadAndDelete(handle); ok { + C.free(handle) } - next := make(map[unsafe.Pointer]handleVal, len(current)-1) - for h, v := range current { - if h == handle { - continue - } - next[h] = v - } - handleVals.Store(next) - C.free(handle) } func deleteHandles(db *SQLiteConn) { - handleLock.Lock() - defer handleLock.Unlock() - - current := loadHandleVals() - if len(current) == 0 { - return - } - - next := make(map[unsafe.Pointer]handleVal, len(current)) - for handle, val := range current { - if val.db == db { - C.free(handle) - continue + handleVals.Range(func(handle, val any) bool { + if val.(handleVal).db == db { + if _, ok := handleVals.LoadAndDelete(handle); ok { + C.free(handle.(unsafe.Pointer)) + } } - next[handle] = val - } - handleVals.Store(next) -} - -func loadHandleVals() map[unsafe.Pointer]handleVal { - m, _ := handleVals.Load().(map[unsafe.Pointer]handleVal) - return m -} - -func cloneHandleVals(size int) map[unsafe.Pointer]handleVal { - next := make(map[unsafe.Pointer]handleVal, size) - for handle, val := range loadHandleVals() { - next[handle] = val - } - return next + return true + }) } // This is only here so that tests can refer to it. diff --git a/callback_bench_test.go b/callback_bench_test.go index 19e8ef5f..54a391c8 100644 --- a/callback_bench_test.go +++ b/callback_bench_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 @@ -49,6 +48,14 @@ func BenchmarkHandleLookupBeforeAfter(b *testing.B) { return after.lookup(handle).val }) }) + + var syncTable syncMapHandleTable + syncTable.vals.Store(handle, value) + b.Run("sync_map", func(b *testing.B) { + benchmarkHandleLookupParallel(b, func() any { + return syncTable.lookup(handle).val + }) + }) } func benchmarkHandleLookupParallel(b *testing.B, lookup func() any) { @@ -75,6 +82,18 @@ func (t *mutexHandleTable) lookup(handle unsafe.Pointer) handleVal { return t.vals[handle] } +type syncMapHandleTable struct { + vals sync.Map +} + +func (t *syncMapHandleTable) lookup(handle unsafe.Pointer) handleVal { + v, ok := t.vals.Load(handle) + if !ok { + return handleVal{} + } + return v.(handleVal) +} + type atomicHandleTable struct { vals atomic.Value } diff --git a/callback_test.go b/callback_test.go index 8163f2f9..32736dce 100644 --- a/callback_test.go +++ b/callback_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 diff --git a/convert.go b/convert.go index f7a9dcd7..a77fc4c0 100644 --- a/convert.go +++ b/convert.go @@ -159,7 +159,7 @@ func convertAssign(dest, src any) error { } dpv := reflect.ValueOf(dest) - if dpv.Kind() != reflect.Ptr { + if dpv.Kind() != reflect.Pointer { return errors.New("destination not a pointer") } if dpv.IsNil() { @@ -192,7 +192,7 @@ func convertAssign(dest, src any) error { // This also allows scanning into user defined types such as "type Int int64". // For symmetry, also check for string destination types. switch dv.Kind() { - case reflect.Ptr: + case reflect.Pointer: if src == nil { dv.Set(reflect.Zero(dv.Type())) return nil diff --git a/error_test.go b/error_test.go index fc6353d1..1cee0603 100644 --- a/error_test.go +++ b/error_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 diff --git a/sqlite3.go b/sqlite3.go index 06620ddb..d1a06fa2 100644 --- a/sqlite3.go +++ b/sqlite3.go @@ -235,6 +235,86 @@ _sqlite3_prepare_v2_internal(sqlite3 *db, const char *zSql, int nBytes, sqlite3_ } #endif +// Resets a statement, binds positional arguments, takes the first step +// and reports the post-step column count and cumulative re-prepare +// count, all in a single CGO crossing. Arguments arrive as +// sqlite3_go_col records whose typ selects the bind: SQLITE_INTEGER, +// SQLITE_FLOAT, SQLITE_TEXT and SQLITE_BLOB bind the respective field +// (ptr points into C memory owned by the Go side and bound with +// SQLITE_STATIC), anything else binds NULL. Returns the bind error +// code with *ncol == -1 when a bind fails. +static int +_sqlite3_bind_step_columns(sqlite3_stmt* stmt, sqlite3_go_col* args, int nargs, int* ncol, int* repreps, sqlite3_go_col* cols, int colcap, int* filled) +{ + int i; + int rv = sqlite3_reset(stmt); + *filled = 0; + if (rv != SQLITE_OK && rv != SQLITE_ROW && rv != SQLITE_DONE) { + *ncol = -1; + return rv; + } + sqlite3_clear_bindings(stmt); + for (i = 0; i < nargs; i++) { + sqlite3_go_col* a = &args[i]; + switch (a->typ) { + case SQLITE_INTEGER: + rv = sqlite3_bind_int64(stmt, i+1, a->i64); + break; + case SQLITE_FLOAT: + rv = sqlite3_bind_double(stmt, i+1, a->f64); + break; + case SQLITE_TEXT: + rv = sqlite3_bind_text64(stmt, i+1, (const char*)a->ptr, a->n, SQLITE_STATIC, SQLITE_UTF8); + break; + case SQLITE_BLOB: + rv = sqlite3_bind_blob64(stmt, i+1, a->ptr, a->n, SQLITE_STATIC); + break; + default: + rv = sqlite3_bind_null(stmt, i+1); + break; + } + if (rv != SQLITE_OK) { + *ncol = -1; + return rv; + } + } + rv = _sqlite3_step_internal(stmt); + *ncol = sqlite3_column_count(stmt); +#ifdef SQLITE_STMTSTATUS_REPREPARE + *repreps = sqlite3_stmt_status(stmt, SQLITE_STMTSTATUS_REPREPARE, 0); +#else + *repreps = -1; +#endif + // When the row buffer is already large enough, deliver the first + // row's values in the same crossing. + if (rv == SQLITE_ROW && cols != 0 && *ncol <= colcap) { + _sqlite3_column_values(stmt, *ncol, cols); + *filled = 1; + } else { + *filled = 0; + } + return rv; +} + +// Steps a statement once and reports the post-step column count and the +// cumulative re-prepare count, in a single CGO crossing. Used for the +// eager first step of cached statements: only after the first step is an +// expired statement guaranteed to have been re-prepared following a +// schema change, so only then do the column count and metadata describe +// the current schema. +static int +_sqlite3_step_columns(sqlite3_stmt* stmt, int* ncol, int* repreps) +{ + int rv = _sqlite3_step_internal(stmt); + *ncol = sqlite3_column_count(stmt); +#ifdef SQLITE_STMTSTATUS_REPREPARE + *repreps = sqlite3_stmt_status(stmt, SQLITE_STMTSTATUS_REPREPARE, 0); +#else + *repreps = -1; +#endif + return rv; +} + void _sqlite3_result_text(sqlite3_context* ctx, const char* s, int n) { sqlite3_result_text(ctx, s, n, &free); } @@ -460,8 +540,11 @@ type SQLiteDriver struct { // SQLiteConn implements driver.Conn. type SQLiteConn struct { - mu sync.Mutex - db *C.sqlite3 + mu sync.Mutex + db *C.sqlite3 + // activeRows identifies the cancellable Rows currently calling sqlite3_step. + // It is guarded by mu so a stale cancellation cannot interrupt later work. + activeRows *SQLiteRows loc *time.Location txlock string funcs []*functionInfo @@ -489,9 +572,27 @@ type SQLiteStmt struct { t string closed bool cls bool // True if the statement was created by SQLiteConn.Query + numInput int32 namedParams map[string][3]int cacheKey string metadata *sqliteStmtMetadata + // repreps is the statement's cumulative re-prepare count observed + // at the last eager first step; a change means SQLite re-prepared + // the statement after a schema change and metadata must be rebuilt. + repreps C.int + // argBuf is C memory owned by the statement holding the bytes of + // text/blob/time arguments for the fused bind path; the bytes are + // bound with SQLITE_STATIC and stay valid until the next bind or + // finalize. colvals is the statement-owned row buffer reused by + // every SQLiteRows of this statement. + argBuf unsafe.Pointer + argBufCap int + colvals *C.sqlite3_go_col + colvalsCap int32 + // cargs is reused across queries of this statement to avoid a + // per-query allocation; a statement has at most one query binding + // at a time. + cargs []C.sqlite3_go_col } type sqliteStmtMetadata struct { @@ -507,14 +608,20 @@ type SQLiteResult struct { // SQLiteRows implements driver.Rows. type SQLiteRows struct { - s *SQLiteStmt - nc int32 // Number of columns - cls bool // True if we need to close the parent statement in Close - cols []string - decltype []string - colvals *C.sqlite3_go_col - ctx context.Context // no better alternative to pass context into Next() method - closemu sync.Mutex + s *SQLiteStmt + nc int32 // Number of columns + cls bool // True if we need to close the parent statement in Close + cols []string + decltype []string + colvals *C.sqlite3_go_col + ctx context.Context // no better alternative to pass context into Next() method + stopCancellation func() bool + // pendingStep buffers the result of the eager first step taken in + // query(); -1 when no step is buffered. pendingFilled reports that + // the buffered row's column values are already in colvals. + pendingStep C.int + pendingFilled bool + closemu sync.Mutex } type functionInfo struct { @@ -983,7 +1090,7 @@ func (c *SQLiteConn) exec(ctx context.Context, query string, args []driver.Named na := s.NumInput() if len(args)-start < na { s.Close() - return nil, fmt.Errorf("not enough args to execute query: want %d got %d", na, len(args)) + return nil, fmt.Errorf("not enough args to execute query: want %d got %d", na, len(args)-start) } stmtArgs := stmtArgs(args, start, na) res, err = s.(*SQLiteStmt).exec(ctx, stmtArgs) @@ -1624,6 +1731,18 @@ func (d *SQLiteDriver) Open(dsn string) (driver.Conn, error) { return nil, err } + if conn.stmtCacheEnabled && int(C.sqlite3_libversion_number()) < 3020000 { + // Schema-change detection for cached statements relies on + // SQLITE_STMTSTATUS_REPREPARE (3.20.0); with an older runtime + // library run without the cache rather than risk serving + // statements whose metadata a schema change has expired. The + // compile-time check in _sqlite3_step_columns is not enough: + // with USE_LIBSQLITE3 the header and the runtime library can + // differ. + conn.stmtCache = nil + conn.stmtCacheEnabled = false + } + exec := func(s string) error { cs := C.CString(s) rv := C.sqlite3_exec(db, cs, nil, nil, nil) @@ -2014,6 +2133,7 @@ func finalizeCachedStmt(s *SQLiteStmt) { return } runtime.SetFinalizer(s, nil) + s.freeScratch() if s.s != nil { C.sqlite3_finalize(s.s) s.s = nil @@ -2041,6 +2161,9 @@ func (c *SQLiteConn) prepare(ctx context.Context, query string) (driver.Stmt, er t = strings.TrimSpace(C.GoString(tail)) } ss := &SQLiteStmt{c: c, s: s, t: t} + if s != nil { + ss.numInput = int32(C.sqlite3_bind_parameter_count(s)) + } runtime.SetFinalizer(ss, (*SQLiteStmt).Close) return ss, nil } @@ -2169,6 +2292,20 @@ func (c *SQLiteConn) SetFileControlInt64(dbName string, op int, arg int64) error return nil } +// freeScratch releases the statement-owned argument and row buffers. +func (s *SQLiteStmt) freeScratch() { + if s.argBuf != nil { + C.free(s.argBuf) + s.argBuf = nil + s.argBufCap = 0 + } + if s.colvals != nil { + C.free(unsafe.Pointer(s.colvals)) + s.colvals = nil + s.colvalsCap = 0 + } +} + // Close the statement. func (s *SQLiteStmt) Close() error { s.mu.Lock() @@ -2200,6 +2337,7 @@ func (s *SQLiteStmt) Close() error { } s.s = nil s.c = nil + s.freeScratch() rv := C.sqlite3_finalize(stmt) if rv != C.SQLITE_OK { return conn.lastError() @@ -2209,7 +2347,7 @@ func (s *SQLiteStmt) Close() error { // NumInput return a number of parameters. func (s *SQLiteStmt) NumInput() int { - return int(C.sqlite3_bind_parameter_count(s.s)) + return int(s.numInput) } var placeHolder = []byte{0} @@ -2374,27 +2512,117 @@ func (s *SQLiteStmt) Query(args []driver.Value) (driver.Rows, error) { } func (s *SQLiteStmt) query(ctx context.Context, args []driver.NamedValue) (driver.Rows, error) { - if err := s.bind(args); err != nil { + rows := &SQLiteRows{ + s: s, + cls: s.cls, + ctx: ctx, + pendingStep: -1, + } + // The first step is taken eagerly (it was going to run at the first + // Next anyway): only after it are the column count and metadata of a + // statement expired by a schema change guaranteed to describe the + // current schema, and folding reset, bind and step into one CGO + // crossing removes most of the per-query CGO overhead. Note that a + // data-modifying statement issued through Query therefore executes + // even if Next is never called. + rv, err := rows.bindAndFirstStepLocked(args) + if err != nil { return nil, err } + rows.pendingStep = rv - rows := &SQLiteRows{ - s: s, - nc: int32(C.sqlite3_column_count(s.s)), - cls: s.cls, - cols: nil, - decltype: nil, - colvals: nil, - ctx: ctx, + return rows, nil +} + +// marshalFusedArgs converts positional arguments into sqlite3_go_col +// records for _sqlite3_bind_step_columns. Text, blob and time arguments +// are copied into the statement-owned C scratch buffer so the records +// contain no Go pointers and the bytes can be bound with SQLITE_STATIC. +// It reports false when the arguments need the generic bind path. +func (s *SQLiteStmt) marshalFusedArgs(args []driver.NamedValue, cargs []C.sqlite3_go_col) bool { + need := 0 + for i := range args { + if args[i].Name != "" || args[i].Ordinal != i+1 { + return false + } + switch v := args[i].Value.(type) { + case string: + need += len(v) + case []byte: + need += len(v) + case time.Time: + need += len(SQLiteTimestampFormats[0]) + 16 + } } - if rows.nc > 0 { - rows.colvals = (*C.sqlite3_go_col)(C.malloc(C.size_t(rows.nc) * C.size_t(unsafe.Sizeof(C.sqlite3_go_col{})))) - if rows.colvals == nil { - return nil, errors.New("sqlite3: failed to allocate row buffer") + if need > s.argBufCap { + if s.argBuf != nil { + C.free(s.argBuf) } + s.argBuf = C.malloc(C.size_t(need)) + if s.argBuf == nil { + s.argBufCap = 0 + panic("sqlite3: failed to allocate argument buffer") + } + s.argBufCap = need } - - return rows, nil + var buf []byte + if s.argBufCap > 0 { + buf = unsafe.Slice((*byte)(s.argBuf), s.argBufCap) + } + off := 0 + putBytes := func(a *C.sqlite3_go_col, b []byte) { + // A NULL pointer would bind SQL NULL, so empty values point at + // the buffer base (the C side never dereferences n == 0). + a.ptr = s.argBuf + if len(b) > 0 { + a.ptr = unsafe.Add(s.argBuf, off) + off += copy(buf[off:], b) + } + a.n = C.int(len(b)) + } + for i := range args { + a := &cargs[i] + switch v := args[i].Value.(type) { + case nil: + a.typ = C.SQLITE_NULL + case int64: + a.typ = C.SQLITE_INTEGER + a.i64 = C.sqlite3_int64(v) + case bool: + a.typ = C.SQLITE_INTEGER + if v { + a.i64 = 1 + } else { + a.i64 = 0 + } + case float64: + a.typ = C.SQLITE_FLOAT + a.f64 = C.double(v) + case string: + if s.argBuf == nil { + return false + } + a.typ = C.SQLITE_TEXT + putBytes(a, unsafe.Slice(unsafe.StringData(v), len(v))) + case []byte: + if v == nil { + a.typ = C.SQLITE_NULL + break + } + if s.argBuf == nil { + return false + } + a.typ = C.SQLITE_BLOB + putBytes(a, v) + case time.Time: + var tmp [64]byte + a.typ = C.SQLITE_TEXT + putBytes(a, v.AppendFormat(tmp[:0], SQLiteTimestampFormats[0])) + default: + return false + } + } + return true } // LastInsertId return last inserted ID. @@ -2491,19 +2719,14 @@ func (s *SQLiteStmt) Readonly() bool { func (rc *SQLiteRows) Close() error { rc.closemu.Lock() defer rc.closemu.Unlock() + rc.stopWatchingCancellation() s := rc.s if s == nil { - if rc.colvals != nil { - C.free(unsafe.Pointer(rc.colvals)) - rc.colvals = nil - } return nil } rc.s = nil // remove reference to SQLiteStmt - if rc.colvals != nil { - C.free(unsafe.Pointer(rc.colvals)) - rc.colvals = nil - } + // rc.colvals is owned by the statement and released with it. + rc.colvals = nil s.mu.Lock() if s.closed { s.mu.Unlock() @@ -2522,6 +2745,40 @@ func (rc *SQLiteRows) Close() error { return nil } +func (c *SQLiteConn) interruptActiveRows(rows *SQLiteRows) { + c.mu.Lock() + defer c.mu.Unlock() + if c.activeRows == rows && c.db != nil { + C.sqlite3_interrupt(c.db) + } +} + +func (rc *SQLiteRows) stopWatchingCancellation() { + if rc.stopCancellation == nil { + return + } + // A false return is harmless: a callback already in progress can interrupt + // only while rc owns conn.activeRows, which is guarded by conn.mu. + rc.stopCancellation() + rc.stopCancellation = nil +} + +func (rc *SQLiteRows) startStepping() { + conn := rc.s.c + conn.mu.Lock() + conn.activeRows = rc + conn.mu.Unlock() +} + +func (rc *SQLiteRows) finishStepping() { + conn := rc.s.c + conn.mu.Lock() + if conn.activeRows == rc { + conn.activeRows = nil + } + conn.mu.Unlock() +} + func (s *SQLiteStmt) cacheMetadata() bool { return !s.cls || s.cacheKey != "" } @@ -2608,33 +2865,141 @@ func (rc *SQLiteRows) Next(dest []driver.Value) error { return io.EOF } - if rc.ctx.Done() == nil { - return rc.nextSyncLocked(dest) + if rv := rc.pendingStep; rv >= 0 { + rc.pendingStep = -1 + filled := rc.pendingFilled + rc.pendingFilled = false + return rc.readStepResult(dest, rv, filled) } - sema := make(chan struct{}) - var err error - go func() { - err = rc.nextSyncLocked(dest) - close(sema) - }() - select { - case <-sema: + + if rc.stopCancellation == nil { + if rc.ctx.Done() == nil { + rv := C._sqlite3_step_internal(rc.s.s) + return rc.readStepResult(dest, rv, false) + } + conn := rc.s.c + rc.stopCancellation = context.AfterFunc(rc.ctx, func() { + conn.interruptActiveRows(rc) + }) + } + if err := rc.ctx.Err(); err != nil { return err - case <-rc.ctx.Done(): - select { - case <-sema: // no need to interrupt - default: - // this is still racy and can be no-op if executed between sqlite3_* calls in nextSyncLocked. - C.sqlite3_interrupt(rc.s.c.db) - <-sema // ensure goroutine completed + } + rv := rc.stepCancellableLocked() + err := rc.readStepResult(dest, rv, false) + if ctxErr := rc.ctx.Err(); ctxErr != nil { + return ctxErr + } + return err +} + +// eagerFirstStepLocked performs the first step of a cached statement +// under the same cancellation rules as Next, records the post-step +// column count, and drops the statement's cached metadata when SQLite +// re-prepared it after a schema change. Note that this runs the first +// step at query time, so a data-modifying statement issued through +// Query executes even if Next is never called. +// bindAndFirstStepLocked binds the arguments and takes the statement's +// first step under the same cancellation rules as Next, records the +// post-step column count, and drops the statement's cached metadata +// when SQLite re-prepared it after a schema change. The common case +// (positional arguments of the standard types) runs reset, bind and +// step in a single CGO crossing. +func (rc *SQLiteRows) bindAndFirstStepLocked(args []driver.NamedValue) (C.int, error) { + s := rc.s + s.mu.Lock() + defer s.mu.Unlock() + + var cargs []C.sqlite3_go_col + fused := true + if len(args) > 0 { + if cap(s.cargs) < len(args) { + s.cargs = make([]C.sqlite3_go_col, len(args)) + } + cargs = s.cargs[:len(args)] + fused = s.marshalFusedArgs(args, cargs) + } + if !fused { + if err := s.bind(args); err != nil { + return 0, err } - return rc.ctx.Err() } + + if rc.ctx.Done() != nil && rc.stopCancellation == nil { + conn := s.c + rc.stopCancellation = context.AfterFunc(rc.ctx, func() { + conn.interruptActiveRows(rc) + }) + } + if err := rc.ctx.Err(); err != nil { + rc.stopWatchingCancellation() + return 0, err + } + var ncol, repreps, filled C.int + step := func() C.int { + if fused { + var argp *C.sqlite3_go_col + if len(cargs) > 0 { + argp = &cargs[0] + } + return C._sqlite3_bind_step_columns(s.s, argp, C.int(len(cargs)), &ncol, &repreps, s.colvals, C.int(s.colvalsCap), &filled) + } + return C._sqlite3_step_columns(s.s, &ncol, &repreps) + } + var rv C.int + if rc.ctx.Done() == nil { + rv = step() + } else { + rv = func() C.int { + rc.startStepping() + defer rc.finishStepping() + return step() + }() + } + if err := rc.ctx.Err(); err != nil { + rc.stopWatchingCancellation() + C._sqlite3_reset_clear(s.s) + return 0, err + } + if rv != C.SQLITE_ROW && rv != C.SQLITE_DONE { + rc.stopWatchingCancellation() + err := s.c.lastError() + C._sqlite3_reset_clear(s.s) + return 0, err + } + rc.nc = int32(ncol) + rc.pendingFilled = filled != 0 + if repreps != s.repreps { + s.repreps = repreps + s.metadata = nil + } + if rc.nc > 0 { + // s.mu is still held here; the statement-owned row buffer must + // only be touched under it. + if rc.nc > s.colvalsCap { + if s.colvals != nil { + C.free(unsafe.Pointer(s.colvals)) + } + s.colvals = (*C.sqlite3_go_col)(C.malloc(C.size_t(rc.nc) * C.size_t(unsafe.Sizeof(C.sqlite3_go_col{})))) + if s.colvals == nil { + s.colvalsCap = 0 + C._sqlite3_reset_clear(s.s) + return 0, errors.New("sqlite3: failed to allocate row buffer") + } + s.colvalsCap = rc.nc + } + rc.colvals = s.colvals + } + return rv, nil } -// nextSyncLocked moves cursor to next; must be called with locked mutex. -func (rc *SQLiteRows) nextSyncLocked(dest []driver.Value) error { - rv := C._sqlite3_step_internal(rc.s.s) +func (rc *SQLiteRows) stepCancellableLocked() C.int { + rc.startStepping() + defer rc.finishStepping() + return C._sqlite3_step_internal(rc.s.s) +} + +func (rc *SQLiteRows) readStepResult(dest []driver.Value, rv C.int, filled bool) error { if rv == C.SQLITE_DONE { return io.EOF } @@ -2650,7 +3015,9 @@ func (rc *SQLiteRows) nextSyncLocked(dest []driver.Value) error { if len(dest) == 0 { return nil } - C._sqlite3_column_values(rc.s.s, C.int(len(dest)), rc.colvals) + if !filled { + C._sqlite3_column_values(rc.s.s, C.int(len(dest)), rc.colvals) + } colvals := (*[(math.MaxInt32 - 1) / unsafe.Sizeof(C.sqlite3_go_col{})]C.sqlite3_go_col)(unsafe.Pointer(rc.colvals))[:len(dest):len(dest)] decltype := rc.decltype diff --git a/sqlite3_context_test.go b/sqlite3_context_test.go new file mode 100644 index 00000000..0b961802 --- /dev/null +++ b/sqlite3_context_test.go @@ -0,0 +1,492 @@ +//go:build cgo +// +build cgo + +package sqlite3 + +import ( + "context" + "database/sql" + "database/sql/driver" + "errors" + "fmt" + "io" + "sync" + "sync/atomic" + "testing" + "time" +) + +const contextTestTimeout = 5 * time.Second +const contextTestMaxRows = int64(1 << 60) + +const contextTestControlledQuery = ` + WITH RECURSIVE numbers(value) AS ( + VALUES(0) + UNION ALL + SELECT value + 1 FROM numbers + WHERE value < ? AND context_cancel_continue() + ) + SELECT sum(value) FROM numbers` + +func TestRowsContextCancelDuringStep(t *testing.T) { + conn := openContextTestConn(t) + started, stopQuery := registerContextTestQuery(t, conn) + + ctx, cancel := context.WithCancel(context.Background()) + // The first step runs inside QueryContext, so issue the query from a + // goroutine and cancel while it is stepping. + nextDone := make(chan error, 1) + go func() { + rows, err := conn.QueryContext(ctx, contextTestControlledQuery, contextTestQueryArgs(contextTestMaxRows)) + if err != nil { + nextDone <- err + return + } + defer rows.Close() + nextDone <- rows.Next(make([]driver.Value, 1)) + }() + + select { + case <-started: + case <-time.After(contextTestTimeout): + cancel() + _ = stopContextTestQuery(t, stopQuery, nextDone) + t.Fatal("query did not start") + } + cancel() + + select { + case err := <-nextDone: + if !errors.Is(err, context.Canceled) { + t.Fatalf("Next error = %v, want context.Canceled", err) + } + case <-time.After(contextTestTimeout): + _ = stopContextTestQuery(t, stopQuery, nextDone) + t.Fatal("Next did not return after cancellation") + } +} + +func TestRowsContextCancelBetweenRows(t *testing.T) { + conn := openContextTestConn(t) + ctx, cancel := context.WithCancel(context.Background()) + rows, err := conn.QueryContext(ctx, "SELECT 1 UNION ALL SELECT 2", nil) + if err != nil { + t.Fatalf("query: %v", err) + } + defer rows.Close() + + values := make([]driver.Value, 1) + if err := rows.Next(values); err != nil { + t.Fatalf("first Next: %v", err) + } + cancel() + if err := rows.Next(values); !errors.Is(err, context.Canceled) { + t.Fatalf("second Next error = %v, want context.Canceled", err) + } +} + +func TestRowsInterruptIgnoresInactiveRows(t *testing.T) { + conn := openContextTestConn(t) + + idleCtx, cancelIdle := context.WithCancel(context.Background()) + defer cancelIdle() + idleRows, err := conn.QueryContext(idleCtx, "SELECT 1 UNION ALL SELECT 2", nil) + if err != nil { + t.Fatalf("query idle rows: %v", err) + } + defer idleRows.Close() + if err := idleRows.Next(make([]driver.Value, 1)); err != nil { + t.Fatalf("read idle rows: %v", err) + } + + started, stopQuery := registerContextTestQuery(t, conn) + activeCtx, cancelActive := context.WithCancel(context.Background()) + defer cancelActive() + nextDone := make(chan error, 1) + activeRowsCh := make(chan *SQLiteRows, 1) + go func() { + activeRows, err := conn.QueryContext(activeCtx, contextTestControlledQuery, contextTestQueryArgs(contextTestMaxRows)) + if err != nil { + nextDone <- err + return + } + defer activeRows.Close() + activeRowsCh <- activeRows.(*SQLiteRows) + nextDone <- activeRows.Next(make([]driver.Value, 1)) + }() + select { + case <-started: + case <-time.After(contextTestTimeout): + _ = stopContextTestQuery(t, stopQuery, nextDone) + t.Fatal("active query did not start") + } + + // Invoke the callback guard directly so the test does not depend on scheduling. + conn.interruptActiveRows(idleRows.(*SQLiteRows)) + if err := stopContextTestQuery(t, stopQuery, nextDone); err != nil { + t.Fatalf("active query was interrupted: %v", err) + } + select { + case rows := <-activeRowsCh: + _ = rows + default: + } +} + +func TestRowsLateInterruptDoesNotAffectReusedStatement(t *testing.T) { + conn := openContextTestConn(t) + started, stopQuery := registerContextTestQuery(t, conn) + stmtDriver, err := conn.Prepare(contextTestControlledQuery) + if err != nil { + t.Fatalf("prepare: %v", err) + } + defer stmtDriver.Close() + stmt := stmtDriver.(*SQLiteStmt) + + oldCtx, cancelOld := context.WithCancel(context.Background()) + defer cancelOld() + oldRows, err := stmt.QueryContext(oldCtx, contextTestQueryArgs(0)) + if err != nil { + t.Fatalf("query old rows: %v", err) + } + if err := oldRows.Next(make([]driver.Value, 1)); err != nil { + t.Fatalf("read old rows: %v", err) + } + if err := oldRows.Close(); err != nil { + t.Fatalf("close old rows: %v", err) + } + + newCtx, cancelNew := context.WithCancel(context.Background()) + defer cancelNew() + nextDone := make(chan error, 1) + go func() { + newRows, err := stmt.QueryContext(newCtx, contextTestQueryArgs(contextTestMaxRows)) + if err != nil { + nextDone <- err + return + } + defer newRows.Close() + nextDone <- newRows.Next(make([]driver.Value, 1)) + }() + select { + case <-started: + case <-time.After(contextTestTimeout): + _ = stopContextTestQuery(t, stopQuery, nextDone) + t.Fatal("replacement query did not start") + } + + // Simulate a callback that started before oldRows was closed. + conn.interruptActiveRows(oldRows.(*SQLiteRows)) + if err := stopContextTestQuery(t, stopQuery, nextDone); err != nil { + t.Fatalf("replacement query was interrupted: %v", err) + } +} + +func TestRowsContextCancelAndClose(t *testing.T) { + conn := openContextTestConn(t) + for i := 0; i < 100; i++ { + ctx, cancel := context.WithCancel(context.Background()) + rows, err := conn.QueryContext(ctx, "SELECT 1 UNION ALL SELECT 2", nil) + if err != nil { + t.Fatalf("query cancellable rows: %v", err) + } + if err := rows.Next(make([]driver.Value, 1)); err != nil { + t.Fatalf("read cancellable rows: %v", err) + } + + start := make(chan struct{}) + cancelDone := make(chan struct{}) + closeDone := make(chan error, 1) + go func() { + <-start + closeDone <- rows.Close() + }() + go func() { + <-start + cancel() + close(cancelDone) + }() + close(start) + select { + case err := <-closeDone: + if err != nil { + t.Fatalf("close cancellable rows: %v", err) + } + case <-time.After(contextTestTimeout): + t.Fatal("close cancellable rows timed out") + } + select { + case <-cancelDone: + case <-time.After(contextTestTimeout): + t.Fatal("cancel cancellable rows timed out") + } + + reusedRows, err := conn.Query("SELECT 1", nil) + if err != nil { + t.Fatalf("query reused connection: %v", err) + } + if err := reusedRows.Next(make([]driver.Value, 1)); err != nil { + t.Fatalf("read reused connection: %v", err) + } + if err := reusedRows.Close(); err != nil { + t.Fatalf("close reused rows: %v", err) + } + } +} + +func TestRowsContextPanicClearsActiveRows(t *testing.T) { + conn := openContextTestConn(t) + const panicValue = "context test panic" + if err := conn.RegisterFunc("context_cancel_panic", func() int64 { + panic(panicValue) + }, false); err != nil { + t.Fatalf("register function: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // The first step runs inside QueryContext, so the panic surfaces + // there rather than at Next. + var gotPanic any + func() { + defer func() { + gotPanic = recover() + }() + rows, err := conn.QueryContext(ctx, "SELECT context_cancel_panic()", nil) + if err != nil { + t.Fatalf("query: %v", err) + } + defer rows.Close() + _ = rows.Next(make([]driver.Value, 1)) + }() + if gotPanic != panicValue { + t.Fatalf("Next panic = %v, want %q", gotPanic, panicValue) + } + + conn.mu.Lock() + activeRows := conn.activeRows + conn.mu.Unlock() + if activeRows != nil { + t.Fatalf("active rows = %p, want nil", activeRows) + } +} + +func TestDatabaseSQLRowsContextCancelAndReuse(t *testing.T) { + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open database: %v", err) + } + defer db.Close() + + sqlConn, err := db.Conn(context.Background()) + if err != nil { + t.Fatalf("get database connection: %v", err) + } + defer sqlConn.Close() + + var started <-chan struct{} + var stopQuery func() + if err := sqlConn.Raw(func(driverConn any) error { + sqliteConn, ok := driverConn.(*SQLiteConn) + if !ok { + return fmt.Errorf("driver connection type = %T, want *SQLiteConn", driverConn) + } + started, stopQuery = registerContextTestQuery(t, sqliteConn) + return nil + }); err != nil { + t.Fatalf("access driver connection: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + nextDone := make(chan error, 1) + go func() { + rows, err := sqlConn.QueryContext(ctx, contextTestControlledQuery, contextTestMaxRows) + if err != nil { + nextDone <- err + return + } + if rows.Next() { + rows.Close() + nextDone <- errors.New("Next returned a row after cancellation") + return + } + err = rows.Err() + if closeErr := rows.Close(); err == nil { + err = closeErr + } + nextDone <- err + }() + select { + case <-started: + case <-time.After(contextTestTimeout): + cancel() + _ = stopContextTestQuery(t, stopQuery, nextDone) + t.Fatal("query did not start") + } + cancel() + + select { + case err := <-nextDone: + if !errors.Is(err, context.Canceled) { + t.Fatalf("Next error = %v, want context.Canceled", err) + } + case <-time.After(contextTestTimeout): + _ = stopContextTestQuery(t, stopQuery, nextDone) + t.Fatal("Next did not return after cancellation") + } + + var got int + if err := sqlConn.QueryRowContext(context.Background(), "SELECT 1").Scan(&got); err != nil { + t.Fatalf("query reused connection: %v", err) + } + if got != 1 { + t.Fatalf("reused query = %d, want 1", got) + } +} + +func registerContextTestQuery(tb testing.TB, conn *SQLiteConn) (<-chan struct{}, func()) { + tb.Helper() + started := make(chan struct{}) + var startedOnce sync.Once + var keepRunning atomic.Bool + keepRunning.Store(true) + if err := conn.RegisterFunc("context_cancel_continue", func() int64 { + startedOnce.Do(func() { close(started) }) + if keepRunning.Load() { + return 1 + } + return 0 + }, false); err != nil { + tb.Fatalf("register function: %v", err) + } + return started, func() { + keepRunning.Store(false) + } +} + +func stopContextTestQuery(tb testing.TB, stop func(), nextDone <-chan error) error { + tb.Helper() + stop() + select { + case err := <-nextDone: + return err + case <-time.After(contextTestTimeout): + tb.Fatal("controlled query did not stop") + return nil + } +} + +func contextTestQueryArgs(maxRows int64) []driver.NamedValue { + return []driver.NamedValue{{Ordinal: 1, Value: maxRows}} +} + +func BenchmarkRowsContext(b *testing.B) { + conn := openContextTestConn(b) + if _, err := conn.Exec(` + CREATE TABLE benchmark_rows(value INTEGER PRIMARY KEY); + WITH RECURSIVE numbers(value) AS ( + VALUES(1) + UNION ALL + SELECT value + 1 FROM numbers WHERE value < 1000 + ) + INSERT INTO benchmark_rows SELECT value FROM numbers`, nil); err != nil { + b.Fatalf("populate benchmark rows: %v", err) + } + stmtDriver, err := conn.Prepare("SELECT value FROM benchmark_rows LIMIT ?") + if err != nil { + b.Fatalf("prepare: %v", err) + } + defer stmtDriver.Close() + stmt := stmtDriver.(*SQLiteStmt) + + contexts := []struct { + name string + new func() (context.Context, context.CancelFunc) + }{ + { + name: "non_cancelable", + new: func() (context.Context, context.CancelFunc) { + return context.Background(), func() {} + }, + }, + { + name: "cancelable", + new: func() (context.Context, context.CancelFunc) { + return context.WithCancel(context.Background()) + }, + }, + } + + for _, rowCount := range []int64{0, 1, 1000} { + for _, benchmarkContext := range contexts { + b.Run(fmt.Sprintf("rows=%d/context=%s", rowCount, benchmarkContext.name), func(b *testing.B) { + ctx, cancel := benchmarkContext.new() + defer cancel() + args := []driver.NamedValue{{Ordinal: 1, Value: rowCount}} + values := make([]driver.Value, 1) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + rows, err := stmt.QueryContext(ctx, args) + if err != nil { + b.Fatalf("query: %v", err) + } + gotRows := int64(0) + for { + err := rows.Next(values) + if errors.Is(err, io.EOF) { + break + } + if err != nil { + b.Fatalf("Next: %v", err) + } + gotRows++ + } + if err := rows.Close(); err != nil { + b.Fatalf("close rows: %v", err) + } + if gotRows != rowCount { + b.Fatalf("row count = %d, want %d", gotRows, rowCount) + } + } + b.ReportMetric(float64(rowCount), "rows/op") + }) + } + } + + for _, benchmarkContext := range contexts { + b.Run(fmt.Sprintf("close_without_next/context=%s", benchmarkContext.name), func(b *testing.B) { + ctx, cancel := benchmarkContext.new() + defer cancel() + args := []driver.NamedValue{{Ordinal: 1, Value: int64(1)}} + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + rows, err := stmt.QueryContext(ctx, args) + if err != nil { + b.Fatalf("query: %v", err) + } + if err := rows.Close(); err != nil { + b.Fatalf("close rows: %v", err) + } + } + }) + } +} + +func openContextTestConn(tb testing.TB) *SQLiteConn { + tb.Helper() + rawConn, err := (&SQLiteDriver{}).Open(":memory:") + if err != nil { + tb.Fatalf("open SQLite connection: %v", err) + } + conn := rawConn.(*SQLiteConn) + tb.Cleanup(func() { + if err := conn.Close(); err != nil { + tb.Errorf("close SQLite connection: %v", err) + } + }) + return conn +} diff --git a/sqlite3_func_crypt.go b/sqlite3_func_crypt.go index bd9a3bc0..5a8cd36c 100644 --- a/sqlite3_func_crypt.go +++ b/sqlite3_func_crypt.go @@ -11,43 +11,41 @@ import ( "crypto/sha512" ) -// This file provides several different implementations for the -// default embedded sqlite_crypt function. -// This function is uses a caesar-cypher by default -// and is used within the UserAuthentication module to encode -// the password. +// This file provides several different implementations for the default +// embedded sqlite_crypt function. This function uses a caesar-cypher by +// default and is used within the UserAuthentication module to encode the +// password. // // The provided functions can be used as an overload to the sqlite_crypt // function through the use of the RegisterFunc on the connection. // -// Because the functions can serv a purpose to an end-user -// without using the UserAuthentication module -// the functions are default compiled in. +// Because the functions can serve a purpose to an end-user without using the +// UserAuthentication module the functions are default compiled in. // // From SQLITE3 - user-auth.txt // The sqlite_user.pw field is encoded by a built-in SQL function -// "sqlite_crypt(X,Y)". The two arguments are both BLOBs. The first argument -// is the plaintext password supplied to the sqlite3_user_authenticate() -// interface. The second argument is the sqlite_user.pw value and is supplied +// "sqlite_crypt(X,Y)". The two arguments are both BLOBs. The first argument is +// the plain-text password supplied to the sqlite3_user_authenticate() +// interface. The second argument is the sqlite_user.pw value and is supplied // so that the function can extract the "salt" used by the password encoder. -// The result of sqlite_crypt(X,Y) is another blob which is the value that -// ends up being stored in sqlite_user.pw. To verify credentials X supplied -// by the sqlite3_user_authenticate() routine, SQLite runs: +// The result of sqlite_crypt(X,Y) is another blob which is the value that ends +// up being stored in sqlite_user.pw. To verify credentials X supplied by the +// sqlite3_user_authenticate() routine, SQLite runs: // // sqlite_user.pw == sqlite_crypt(X, sqlite_user.pw) // // To compute an appropriate sqlite_user.pw value from a new or modified -// password X, sqlite_crypt(X,NULL) is run. A new random salt is selected -// when the second argument is NULL. +// password X, sqlite_crypt(X,NULL) is run. A new random salt is selected when +// the second argument is NULL. // -// The built-in version of of sqlite_crypt() uses a simple Caesar-cypher -// which prevents passwords from being revealed by searching the raw database -// for ASCII text, but is otherwise trivally broken. For better password -// security, the database should be encrypted using the SQLite Encryption -// Extension or similar technology. Or, the application can use the -// sqlite3_create_function() interface to provide an alternative -// implementation of sqlite_crypt() that computes a stronger password hash, -// perhaps using a cryptographic hash function like SHA1. +// The built-in version of of sqlite_crypt() uses a simple Caesar-cypher which +// prevents passwords from being revealed by searching the raw database for +// ASCII text, but is otherwise trivally broken. For better password security, +// the database should be encrypted using the SQLite Encryption Extension or +// similar technology. Or, the application can use the +// sqlite3_create_function() interface to provide an alternative implementation +// of sqlite_crypt() that computes a stronger password hash, perhaps using a +// cryptographic hash function like SHA1. // CryptEncoderSHA1 encodes a password with SHA1 func CryptEncoderSHA1(pass []byte, hash any) []byte { diff --git a/sqlite3_libsqlite3.go b/sqlite3_libsqlite3.go index 6ef23086..45714dac 100644 --- a/sqlite3_libsqlite3.go +++ b/sqlite3_libsqlite3.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build libsqlite3 -// +build libsqlite3 package sqlite3 diff --git a/sqlite3_load_extension_omit.go b/sqlite3_load_extension_omit.go index d4f8ce65..3503ce1c 100644 --- a/sqlite3_load_extension_omit.go +++ b/sqlite3_load_extension_omit.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_omit_load_extension -// +build sqlite_omit_load_extension package sqlite3 diff --git a/sqlite3_load_extension_test.go b/sqlite3_load_extension_test.go index c6c03bb2..757009e1 100644 --- a/sqlite3_load_extension_test.go +++ b/sqlite3_load_extension_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build !sqlite_omit_load_extension -// +build !sqlite_omit_load_extension package sqlite3 diff --git a/sqlite3_opt_allow_uri_authority.go b/sqlite3_opt_allow_uri_authority.go index 51240cbf..91b41b46 100644 --- a/sqlite3_opt_allow_uri_authority.go +++ b/sqlite3_opt_allow_uri_authority.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_allow_uri_authority -// +build sqlite_allow_uri_authority package sqlite3 diff --git a/sqlite3_opt_app_armor.go b/sqlite3_opt_app_armor.go index 565dbc29..f28ebe8b 100644 --- a/sqlite3_opt_app_armor.go +++ b/sqlite3_opt_app_armor.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build !windows && sqlite_app_armor -// +build !windows,sqlite_app_armor package sqlite3 diff --git a/sqlite3_opt_dbstat.go b/sqlite3_opt_dbstat.go index d0338461..c8f10eb2 100644 --- a/sqlite3_opt_dbstat.go +++ b/sqlite3_opt_dbstat.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_dbstat -// +build sqlite_dbstat package sqlite3 diff --git a/sqlite3_opt_foreign_keys.go b/sqlite3_opt_foreign_keys.go index 82c944e1..2fbae03e 100644 --- a/sqlite3_opt_foreign_keys.go +++ b/sqlite3_opt_foreign_keys.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_foreign_keys -// +build sqlite_foreign_keys package sqlite3 diff --git a/sqlite3_opt_fts3_test.go b/sqlite3_opt_fts3_test.go index a7b31a71..9901f423 100644 --- a/sqlite3_opt_fts3_test.go +++ b/sqlite3_opt_fts3_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 diff --git a/sqlite3_opt_fts5.go b/sqlite3_opt_fts5.go index 2645f284..6c041bca 100644 --- a/sqlite3_opt_fts5.go +++ b/sqlite3_opt_fts5.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_fts5 || fts5 -// +build sqlite_fts5 fts5 package sqlite3 diff --git a/sqlite3_opt_icu.go b/sqlite3_opt_icu.go index 2d47827b..1c148c2f 100644 --- a/sqlite3_opt_icu.go +++ b/sqlite3_opt_icu.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_icu || icu -// +build sqlite_icu icu package sqlite3 diff --git a/sqlite3_opt_introspect.go b/sqlite3_opt_introspect.go index cd2e5401..e0e3a193 100644 --- a/sqlite3_opt_introspect.go +++ b/sqlite3_opt_introspect.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_introspect -// +build sqlite_introspect package sqlite3 diff --git a/sqlite3_opt_math_functions.go b/sqlite3_opt_math_functions.go index bd62d9a2..dc1ab98d 100644 --- a/sqlite3_opt_math_functions.go +++ b/sqlite3_opt_math_functions.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_math_functions -// +build sqlite_math_functions package sqlite3 diff --git a/sqlite3_opt_math_functions_test.go b/sqlite3_opt_math_functions_test.go index 09dbd8db..860231d4 100644 --- a/sqlite3_opt_math_functions_test.go +++ b/sqlite3_opt_math_functions_test.go @@ -1,5 +1,4 @@ //go:build sqlite_math_functions -// +build sqlite_math_functions package sqlite3 diff --git a/sqlite3_opt_os_trace.go b/sqlite3_opt_os_trace.go index 9a30566b..62a4bd79 100644 --- a/sqlite3_opt_os_trace.go +++ b/sqlite3_opt_os_trace.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_os_trace -// +build sqlite_os_trace package sqlite3 diff --git a/sqlite3_opt_percentile.go b/sqlite3_opt_percentile.go index 3461d9a5..ff75e728 100644 --- a/sqlite3_opt_percentile.go +++ b/sqlite3_opt_percentile.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_percentile -// +build sqlite_percentile package sqlite3 diff --git a/sqlite3_opt_preupdate.go b/sqlite3_opt_preupdate.go index ed725eeb..629abcf0 100644 --- a/sqlite3_opt_preupdate.go +++ b/sqlite3_opt_preupdate.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 diff --git a/sqlite3_opt_secure_delete.go b/sqlite3_opt_secure_delete.go index 6bb05b84..600e84b5 100644 --- a/sqlite3_opt_secure_delete.go +++ b/sqlite3_opt_secure_delete.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_secure_delete -// +build sqlite_secure_delete package sqlite3 diff --git a/sqlite3_opt_secure_delete_fast.go b/sqlite3_opt_secure_delete_fast.go index 982020ae..eda669b9 100644 --- a/sqlite3_opt_secure_delete_fast.go +++ b/sqlite3_opt_secure_delete_fast.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_secure_delete_fast -// +build sqlite_secure_delete_fast package sqlite3 diff --git a/sqlite3_opt_serialize_omit.go b/sqlite3_opt_serialize_omit.go index d00ead0b..6d6ce017 100644 --- a/sqlite3_opt_serialize_omit.go +++ b/sqlite3_opt_serialize_omit.go @@ -1,5 +1,4 @@ //go:build libsqlite3 && !sqlite_serialize -// +build libsqlite3,!sqlite_serialize package sqlite3 diff --git a/sqlite3_opt_serialize_test.go b/sqlite3_opt_serialize_test.go index 5c7efec3..050553fc 100644 --- a/sqlite3_opt_serialize_test.go +++ b/sqlite3_opt_serialize_test.go @@ -1,5 +1,4 @@ //go:build !libsqlite3 || sqlite_serialize -// +build !libsqlite3 sqlite_serialize package sqlite3 diff --git a/sqlite3_opt_stat4.go b/sqlite3_opt_stat4.go index 799fbb0f..12b21133 100644 --- a/sqlite3_opt_stat4.go +++ b/sqlite3_opt_stat4.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_stat4 -// +build sqlite_stat4 package sqlite3 diff --git a/sqlite3_opt_unlock_notify.go b/sqlite3_opt_unlock_notify.go index dddb655d..7cb8d6ea 100644 --- a/sqlite3_opt_unlock_notify.go +++ b/sqlite3_opt_unlock_notify.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo && sqlite_unlock_notify -// +build cgo,sqlite_unlock_notify package sqlite3 diff --git a/sqlite3_opt_unlock_notify_test.go b/sqlite3_opt_unlock_notify_test.go index 3a9168cd..da3f9ccf 100644 --- a/sqlite3_opt_unlock_notify_test.go +++ b/sqlite3_opt_unlock_notify_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_unlock_notify -// +build sqlite_unlock_notify package sqlite3 diff --git a/sqlite3_opt_userauth.go b/sqlite3_opt_userauth.go index 5a492766..c1a14106 100644 --- a/sqlite3_opt_userauth.go +++ b/sqlite3_opt_userauth.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_userauth -// +build sqlite_userauth package sqlite3 diff --git a/sqlite3_opt_userauth_test.go b/sqlite3_opt_userauth_test.go index 218aecdf..6609034e 100644 --- a/sqlite3_opt_userauth_test.go +++ b/sqlite3_opt_userauth_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_userauth -// +build sqlite_userauth package sqlite3 diff --git a/sqlite3_opt_vacuum_full.go b/sqlite3_opt_vacuum_full.go index df13c9d2..23414649 100644 --- a/sqlite3_opt_vacuum_full.go +++ b/sqlite3_opt_vacuum_full.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_vacuum_full -// +build sqlite_vacuum_full package sqlite3 diff --git a/sqlite3_opt_vacuum_incr.go b/sqlite3_opt_vacuum_incr.go index a2e48814..6cf38d68 100644 --- a/sqlite3_opt_vacuum_incr.go +++ b/sqlite3_opt_vacuum_incr.go @@ -5,7 +5,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_vacuum_incr -// +build sqlite_vacuum_incr package sqlite3 diff --git a/sqlite3_opt_vtable.go b/sqlite3_opt_vtable.go index 90a02564..cee38ec6 100644 --- a/sqlite3_opt_vtable.go +++ b/sqlite3_opt_vtable.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_vtable || vtable -// +build sqlite_vtable vtable package sqlite3 diff --git a/sqlite3_opt_vtable_leak_test.go b/sqlite3_opt_vtable_leak_test.go index d2b70477..fc68b824 100644 --- a/sqlite3_opt_vtable_leak_test.go +++ b/sqlite3_opt_vtable_leak_test.go @@ -11,6 +11,15 @@ import ( var leakCheckDriverSeq int32 +func countHandles() int { + n := 0 + handleVals.Range(func(_, _ any) bool { + n++ + return true + }) + return n +} + func TestVtabCursorHandleRelease(t *testing.T) { // Use a unique driver name so repeated runs (e.g. -count=2) do not // panic on duplicate registration. @@ -35,10 +44,10 @@ func TestVtabCursorHandleRelease(t *testing.T) { t.Fatal(err) } if i == 0 { - before = len(loadHandleVals()) + before = countHandles() } } - after = len(loadHandleVals()) + after = countHandles() if after > before { t.Fatalf("handle map grew from %d to %d over repeated cursor open/close", before, after) } diff --git a/sqlite3_opt_vtable_test.go b/sqlite3_opt_vtable_test.go index 3afa155e..16974ebc 100644 --- a/sqlite3_opt_vtable_test.go +++ b/sqlite3_opt_vtable_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build sqlite_vtable || vtable -// +build sqlite_vtable vtable package sqlite3 diff --git a/sqlite3_solaris.go b/sqlite3_solaris.go index fb4d3251..26680e0d 100644 --- a/sqlite3_solaris.go +++ b/sqlite3_solaris.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build solaris -// +build solaris package sqlite3 diff --git a/sqlite3_sql.go b/sqlite3_sql.go index 47c522f4..6148a03d 100644 --- a/sqlite3_sql.go +++ b/sqlite3_sql.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 diff --git a/sqlite3_sql_test.go b/sqlite3_sql_test.go index 67de4344..56567852 100644 --- a/sqlite3_sql_test.go +++ b/sqlite3_sql_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 @@ -95,7 +94,7 @@ func initDatabase(t *testing.T, db *sql.DB, rowCount int64) { (key1, key_id, key2, key3, key4, key5, key6, data) VALUES (?, ?, ?, ?, ?, ?, ?, ?);` - args := []interface{}{ + args := []any{ randStringBytes(50), fmt.Sprint(i), randStringBytes(50), diff --git a/sqlite3_stmt_cache_bench_test.go b/sqlite3_stmt_cache_bench_test.go index 0433350b..29d61fc4 100644 --- a/sqlite3_stmt_cache_bench_test.go +++ b/sqlite3_stmt_cache_bench_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 @@ -24,11 +23,11 @@ func BenchmarkStmtCache(b *testing.B) { cacheSize int keyCount int }{ - {"off", 0, 1}, // baseline: no cache - {"size4_keys1_hit", 4, 1}, // trivial hit path - {"size4_keys4_hit", 4, 4}, // all queries fit, always hit - {"size4_keys8_evict", 4, 8}, // working set > cache: miss + eviction - {"size16_keys8_hit", 16, 8}, // all queries fit in larger cache + {"off", 0, 1}, // baseline: no cache + {"size4_keys1_hit", 4, 1}, // trivial hit path + {"size4_keys4_hit", 4, 4}, // all queries fit, always hit + {"size4_keys8_evict", 4, 8}, // working set > cache: miss + eviction + {"size16_keys8_hit", 16, 8}, // all queries fit in larger cache {"size16_keys32_evict", 16, 32}, // working set >> cache } for _, tc := range cases { diff --git a/sqlite3_stmt_cache_test.go b/sqlite3_stmt_cache_test.go index 65c58304..6cab8374 100644 --- a/sqlite3_stmt_cache_test.go +++ b/sqlite3_stmt_cache_test.go @@ -4,12 +4,14 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 import ( "context" + "database/sql" + "path/filepath" + "reflect" "testing" ) @@ -108,6 +110,9 @@ func TestStmtCacheReuseReturnsSameHandle(t *testing.T) { defer conn.Close() c := conn.(*SQLiteConn) + if !c.stmtCacheEnabled { + t.Skip("statement cache disabled on this SQLite runtime") + } ctx := context.Background() const q = "SELECT 42" @@ -134,6 +139,171 @@ func TestStmtCacheReuseReturnsSameHandle(t *testing.T) { } } +// TestStmtCacheSchemaChange verifies that a schema change does not let the +// cache hand back an expired statement whose captured column metadata still +// describes the old schema (issue #1447). Both same-connection DDL and DDL +// issued through a second database handle must invalidate the cache. +func TestStmtCacheSchemaChange(t *testing.T) { + fn := filepath.Join(t.TempDir(), "schemachange.db") + db, err := sql.Open("sqlite3", "file:"+fn+"?_stmt_cache_size=8") + if err != nil { + t.Fatal(err) + } + defer db.Close() + db.SetMaxOpenConns(1) + + if _, err := db.Exec("CREATE TABLE t (a TEXT)"); err != nil { + t.Fatal(err) + } + if _, err := db.Exec("INSERT INTO t VALUES ('x')"); err != nil { + t.Fatal(err) + } + + queryCols := func() []string { + rows, err := db.Query("SELECT * FROM t") + if err != nil { + t.Fatal(err) + } + defer rows.Close() + cols, err := rows.Columns() + if err != nil { + t.Fatal(err) + } + return cols + } + + // Populate the cache. + if cols := queryCols(); !reflect.DeepEqual(cols, []string{"a"}) { + t.Fatalf("initial columns: got %v, want [a]", cols) + } + + // Same-connection DDL. + if _, err := db.Exec("ALTER TABLE t ADD COLUMN b TEXT"); err != nil { + t.Fatal(err) + } + if cols := queryCols(); !reflect.DeepEqual(cols, []string{"a", "b"}) { + t.Fatalf("columns after same-connection ALTER: got %v, want [a b]", cols) + } + + // DDL through a second database handle (different connection). + db2, err := sql.Open("sqlite3", "file:"+fn) + if err != nil { + t.Fatal(err) + } + if _, err := db2.Exec("ALTER TABLE t ADD COLUMN c TEXT"); err != nil { + db2.Close() + t.Fatal(err) + } + db2.Close() + if cols := queryCols(); !reflect.DeepEqual(cols, []string{"a", "b", "c"}) { + t.Fatalf("columns after cross-connection ALTER: got %v, want [a b c]", cols) + } + + // The row data must scan consistently with the new column set. + var a string + var b, c any + if err := db.QueryRow("SELECT * FROM t").Scan(&a, &b, &c); err != nil { + t.Fatal(err) + } + if a != "x" || b != nil || c != nil { + t.Fatalf("row after ALTERs: got (%q, %v, %v), want (\"x\", , )", a, b, c) + } +} + +// TestStmtCacheTempSchemaChange verifies that DDL on the temp schema also +// invalidates cached statements referencing it. +func TestStmtCacheTempSchemaChange(t *testing.T) { + db, err := sql.Open("sqlite3", ":memory:?_stmt_cache_size=8") + if err != nil { + t.Fatal(err) + } + defer db.Close() + db.SetMaxOpenConns(1) + + if _, err := db.Exec("CREATE TEMP TABLE tt (a TEXT)"); err != nil { + t.Fatal(err) + } + rows, err := db.Query("SELECT * FROM tt") + if err != nil { + t.Fatal(err) + } + rows.Close() // cached + + if _, err := db.Exec("ALTER TABLE tt ADD COLUMN b TEXT"); err != nil { + t.Fatal(err) + } + rows, err = db.Query("SELECT * FROM tt") + if err != nil { + t.Fatal(err) + } + defer rows.Close() + cols, err := rows.Columns() + if err != nil { + t.Fatal(err) + } + if want := []string{"a", "b"}; !reflect.DeepEqual(cols, want) { + t.Fatalf("columns after temp ALTER: got %v, want %v", cols, want) + } +} + +// TestStmtCacheInTransaction verifies that DDL inside an explicit +// transaction is honored by queries later in the same transaction (the +// cache reports a miss there instead of probing the schema). +func TestStmtCacheInTransaction(t *testing.T) { + db, err := sql.Open("sqlite3", ":memory:?_stmt_cache_size=8") + if err != nil { + t.Fatal(err) + } + defer db.Close() + db.SetMaxOpenConns(1) + + if _, err := db.Exec("CREATE TABLE t (a TEXT)"); err != nil { + t.Fatal(err) + } + rows, err := db.Query("SELECT * FROM t") + if err != nil { + t.Fatal(err) + } + rows.Close() // cached + + tx, err := db.Begin() + if err != nil { + t.Fatal(err) + } + if _, err := tx.Exec("ALTER TABLE t ADD COLUMN b TEXT"); err != nil { + t.Fatal(err) + } + rows, err = tx.Query("SELECT * FROM t") + if err != nil { + t.Fatal(err) + } + cols, err := rows.Columns() + rows.Close() + if err != nil { + t.Fatal(err) + } + if want := []string{"a", "b"}; !reflect.DeepEqual(cols, want) { + t.Fatalf("columns inside transaction after ALTER: got %v, want %v", cols, want) + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + + // After commit the next lookup probes again and must see the change. + rows, err = db.Query("SELECT * FROM t") + if err != nil { + t.Fatal(err) + } + defer rows.Close() + cols, err = rows.Columns() + if err != nil { + t.Fatal(err) + } + if want := []string{"a", "b"}; !reflect.DeepEqual(cols, want) { + t.Fatalf("columns after commit: got %v, want %v", cols, want) + } +} + func cacheKeys(c *SQLiteConn) map[string]int { out := make(map[string]int) for _, s := range c.stmtCache { diff --git a/sqlite3_test.go b/sqlite3_test.go index 714ca88d..0e4d97c7 100644 --- a/sqlite3_test.go +++ b/sqlite3_test.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 @@ -469,7 +468,7 @@ func TestUpdate(t *testing.T) { t.Fatal("Failed to get LastInsertId:", err) } if expected != lastID { - t.Errorf("Expected %q for last Id, but %q:", expected, lastID) + t.Errorf("Expected %d for last Id, but %d:", expected, lastID) } affected, _ = res.RowsAffected() if err != nil { @@ -522,7 +521,7 @@ func TestDelete(t *testing.T) { t.Fatal("Failed to get RowsAffected:", err) } if affected != 1 { - t.Errorf("Expected %d for cout of affected rows, but %q:", 1, affected) + t.Errorf("Expected %d for cout of affected rows, but %d:", 1, affected) } res, err = db.Exec("delete from foo where id = 123") @@ -534,14 +533,14 @@ func TestDelete(t *testing.T) { t.Fatal("Failed to get LastInsertId:", err) } if expected != lastID { - t.Errorf("Expected %q for last Id, but %q:", expected, lastID) + t.Errorf("Expected %d for last Id, but %d:", expected, lastID) } affected, err = res.RowsAffected() if err != nil { t.Fatal("Failed to get RowsAffected:", err) } if affected != 1 { - t.Errorf("Expected %d for cout of affected rows, but %q:", 1, affected) + t.Errorf("Expected %d for cout of affected rows, but %d:", 1, affected) } rows, err := db.Query("select id from foo") @@ -2297,6 +2296,36 @@ func TestNamedParamClearBindings(t *testing.T) { } } +func TestNotEnoughArgsErrorMessage(t *testing.T) { + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + t.Fatal(err) + } + defer db.Close() + + const want = "not enough args to execute query: want 1 got 0" + + t.Run("exec", func(t *testing.T) { + _, err := db.Exec("SELECT ?; SELECT ?", "hello") + if err == nil { + t.Fatal("expected error, got nil") + } + if err.Error() != want { + t.Errorf("got %q, want %q", err.Error(), want) + } + }) + + t.Run("query", func(t *testing.T) { + _, err := db.Query("SELECT ?; SELECT ?", "hello") + if err == nil { + t.Fatal("expected error, got nil") + } + if err.Error() != want { + t.Errorf("got %q, want %q", err.Error(), want) + } + }) +} + // https://github.com/mattn/go-sqlite3/issues/1390 // sqlite3_prepare_v2 returns SQLITE_OK with a NULL statement handle when the // input contains no SQL (only whitespace or comments). Querying such input diff --git a/sqlite3_usleep_windows.go b/sqlite3_usleep_windows.go index 6527f6fd..29a99e5d 100644 --- a/sqlite3_usleep_windows.go +++ b/sqlite3_usleep_windows.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build cgo -// +build cgo package sqlite3 diff --git a/sqlite3_windows.go b/sqlite3_windows.go index f863bcd3..6a2bc098 100644 --- a/sqlite3_windows.go +++ b/sqlite3_windows.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build windows -// +build windows package sqlite3 diff --git a/static_mock.go b/static_mock.go index d2c5a276..17248686 100644 --- a/static_mock.go +++ b/static_mock.go @@ -4,7 +4,6 @@ // license that can be found in the LICENSE file. //go:build !cgo -// +build !cgo package sqlite3