From 01caf5224960d91c5bf62a09479d070a0dae4075 Mon Sep 17 00:00:00 2001 From: Charlie Vieth Date: Wed, 5 Feb 2025 22:25:38 -0500 Subject: [PATCH] cache the index of named parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The commit changes bind to cache the position of named parameters, which improves the performance of prepared queries that are re-used. ``` goos: darwin goarch: arm64 pkg: github.com/charlievieth/go-sqlite3 cpu: Apple M4 Pro │ o10.txt │ n10.txt │ │ sec/op │ sec/op vs base │ Scan10Cols-14 4.279µ ± 0% 4.264µ ± 3% ~ (p=0.579 n=10) Suite/BenchmarkExec/Params-14 715.2n ± 1% 716.3n ± 1% ~ (p=0.927 n=10) Suite/BenchmarkExec/NoParams-14 533.1n ± 2% 517.9n ± 3% -2.86% (p=0.001 n=10) Suite/BenchmarkExecContext/Params-14 1.560µ ± 4% 1.504µ ± 0% -3.56% (p=0.000 n=10) Suite/BenchmarkExecContext/NoParams-14 1.523µ ± 2% 1.518µ ± 1% ~ (p=1.000 n=10) Suite/BenchmarkExecStep-14 452.5µ ± 2% 444.6µ ± 1% -1.76% (p=0.015 n=10) Suite/BenchmarkExecContextStep-14 444.1µ ± 2% 451.7µ ± 1% +1.72% (p=0.043 n=10) Suite/BenchmarkExecTx-14 1.607µ ± 1% 1.585µ ± 1% -1.40% (p=0.000 n=10) Suite/BenchmarkQuery-14 1.757µ ± 1% 1.784µ ± 0% +1.51% (p=0.001 n=10) Suite/BenchmarkQuerySimple-14 1.008µ ± 2% 1.015µ ± 1% ~ (p=0.117 n=10) Suite/BenchmarkQueryContext/Background-14 2.380µ ± 1% 2.315µ ± 0% -2.73% (p=0.001 n=10) Suite/BenchmarkQueryContext/WithCancel-14 8.314µ ± 1% 8.236µ ± 4% ~ (p=0.225 n=10) Suite/BenchmarkParams-14 1.952µ ± 1% 2.012µ ± 1% +3.07% (p=0.000 n=10) Suite/BenchmarkStmt-14 1.373µ ± 1% 1.394µ ± 1% +1.57% (p=0.001 n=10) Suite/BenchmarkRows-14 58.61µ ± 3% 59.95µ ± 1% +2.29% (p=0.029 n=10) Suite/BenchmarkStmtRows-14 58.80µ ± 2% 59.82µ ± 2% +1.73% (p=0.050 n=10) Suite/BenchmarkStmt10Cols-14 4.409µ ± 1% 4.466µ ± 0% +1.30% (p=0.006 n=10) Suite/BenchmarkScanRawBytes-14 2.499µ ± 1% 2.470µ ± 1% -1.18% (p=0.001 n=10) Suite/BenchmarkQueryParallel-14 438.3n ± 5% 439.0n ± 4% ~ (p=0.812 n=10) Suite/BenchmarkOpen-14 13.13µ ± 1% 13.05µ ± 2% ~ (p=0.197 n=10) Suite/BenchmarkNamedParams-14 1.675µ ± 1% 1.196µ ± 2% -28.60% (p=0.000 n=10) Suite/BenchmarkParseTime-14 1.040µ ± 2% 1.019µ ± 2% -2.02% (p=0.000 n=10) geomean 4.173µ 4.102µ -1.69% │ o10.txt │ n10.txt │ │ B/op │ B/op vs base │ Scan10Cols-14 712.0 ± 0% 712.0 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExec/Params-14 200.0 ± 0% 216.0 ± 0% +8.00% (p=0.000 n=10) Suite/BenchmarkExec/NoParams-14 64.00 ± 0% 64.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecContext/Params-14 360.0 ± 0% 376.0 ± 0% +4.44% (p=0.000 n=10) Suite/BenchmarkExecContext/NoParams-14 208.0 ± 0% 208.0 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecStep-14 64.00 ± 0% 64.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecContextStep-14 208.0 ± 0% 208.0 ± 0% ~ (p=1.000 n=10) Suite/BenchmarkExecTx-14 520.0 ± 0% 520.0 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkQuery-14 640.0 ± 0% 656.0 ± 0% +2.50% (p=0.000 n=10) Suite/BenchmarkQuerySimple-14 440.0 ± 0% 456.0 ± 0% +3.64% (p=0.000 n=10) Suite/BenchmarkQueryContext/Background-14 396.0 ± 0% 396.0 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkQueryContext/WithCancel-14 1.016Ki ± 0% 1.015Ki ± 0% ~ (p=0.121 n=10) Suite/BenchmarkParams-14 784.0 ± 0% 800.0 ± 0% +2.04% (p=0.000 n=10) Suite/BenchmarkStmt-14 784.0 ± 0% 784.0 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkRows-14 6.047Ki ± 0% 6.062Ki ± 0% +0.26% (p=0.000 n=10) Suite/BenchmarkStmtRows-14 6.055Ki ± 0% 6.055Ki ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkStmt10Cols-14 712.0 ± 0% 712.0 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkScanRawBytes-14 592.0 ± 0% 592.0 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkQueryParallel-14 532.0 ± 0% 548.0 ± 0% +3.01% (p=0.000 n=10) Suite/BenchmarkOpen-14 111.0 ± 1% 111.0 ± 1% ~ (p=1.000 n=10) Suite/BenchmarkNamedParams-14 664.0 ± 0% 600.0 ± 0% -9.64% (p=0.000 n=10) Suite/BenchmarkParseTime-14 444.0 ± 0% 444.0 ± 0% ~ (p=1.000 n=10) ¹ geomean 479.5 482.4 +0.60% ¹ all samples are equal │ o10.txt │ n10.txt │ │ allocs/op │ allocs/op vs base │ Scan10Cols-14 19.00 ± 0% 19.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExec/Params-14 8.000 ± 0% 8.000 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExec/NoParams-14 4.000 ± 0% 4.000 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecContext/Params-14 10.00 ± 0% 10.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecContext/NoParams-14 6.000 ± 0% 6.000 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecStep-14 4.000 ± 0% 4.000 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecContextStep-14 6.000 ± 0% 6.000 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkExecTx-14 18.00 ± 0% 18.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkQuery-14 22.00 ± 0% 22.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkQuerySimple-14 13.00 ± 0% 13.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkQueryContext/Background-14 10.00 ± 0% 10.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkQueryContext/WithCancel-14 26.00 ± 0% 26.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkParams-14 23.00 ± 0% 23.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkStmt-14 23.00 ± 0% 23.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkRows-14 418.0 ± 0% 418.0 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkStmtRows-14 418.0 ± 0% 418.0 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkStmt10Cols-14 19.00 ± 0% 19.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkScanRawBytes-14 18.00 ± 0% 18.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkQueryParallel-14 15.00 ± 0% 15.00 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkOpen-14 3.000 ± 0% 3.000 ± 0% ~ (p=1.000 n=10) ¹ Suite/BenchmarkNamedParams-14 14.00 ± 0% 13.00 ± 0% -7.14% (p=0.000 n=10) Suite/BenchmarkParseTime-14 12.00 ± 0% 12.00 ± 0% ~ (p=1.000 n=10) ¹ geomean 15.93 15.87 -0.34% ¹ all samples are equal ``` --- sqlite3.go | 145 ++++++++++++++++++++++++++++--------------- sqlite3_test.go | 160 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 251 insertions(+), 54 deletions(-) diff --git a/sqlite3.go b/sqlite3.go index b98ccb4b..a7869218 100644 --- a/sqlite3.go +++ b/sqlite3.go @@ -602,12 +602,13 @@ type SQLiteTx struct { // SQLiteStmt implements driver.Stmt. type SQLiteStmt struct { - mu sync.Mutex - c *SQLiteConn - s *C.sqlite3_stmt - closed bool - cls bool // True if the statement was created by SQLiteConn.Query - reset bool // True if the statement needs to reset before re-use + mu sync.Mutex + c *SQLiteConn + s *C.sqlite3_stmt + namedParams map[string]int // Map of named parameter to index + closed bool + cls bool // True if the statement was created by SQLiteConn.Query + reset bool // True if the statement needs to reset before re-use } // SQLiteResult implements sql.Result. @@ -2328,8 +2329,35 @@ func (s *SQLiteStmt) bind(args []driver.NamedValue) error { return nil } -func (s *SQLiteStmt) bindIndices(args []driver.NamedValue) error { - // Find the longest named parameter name. +func (s *SQLiteStmt) missingNamedParams(args []driver.NamedValue) bool { + for _, v := range args { + if v.Name != "" { + if _, ok := s.namedParams[v.Name]; !ok { + return true + } + } + } + return false +} + +func (s *SQLiteStmt) createBindIndices(args []driver.NamedValue) { + if len(args) == 0 { + return + } + s.mu.Lock() + defer s.mu.Unlock() + + // Check if we need to update or create the named params map. + // This check is necessary because a bad set of params could + // be passed to a prepared statement on its first invocation. + if !s.missingNamedParams(args) { + return + } + if s.namedParams == nil { + s.namedParams = make(map[string]int, len(args)) + } + + // Find the longest parameter name n := 0 for _, v := range args { if m := len(v.Name); m > n { @@ -2338,60 +2366,75 @@ func (s *SQLiteStmt) bindIndices(args []driver.NamedValue) error { } buf := make([]byte, 0, n+2) // +2 for placeholder and null terminator - bindIndices := make([][3]int, len(args)) + for _, v := range args { + if v.Name == "" { + continue + } + for _, c := range []byte{':', '@', '$'} { + buf = append(buf[:0], c) + buf = append(buf, v.Name...) + buf = append(buf, 0) + idx := int(C.sqlite3_bind_parameter_index(s.s, (*C.char)(unsafe.Pointer(&buf[0])))) + if idx != 0 { + s.namedParams[v.Name] = idx + break + } + } + } +} + +func (s *SQLiteStmt) bindIndices(args []driver.NamedValue) error { + s.createBindIndices(args) + + bindIndices := make([]int, len(args)) for i, v := range args { - bindIndices[i][0] = args[i].Ordinal + bindIndices[i] = args[i].Ordinal if v.Name != "" { - for j, c := range []byte{':', '@', '$'} { - buf = append(buf[:0], c) - buf = append(buf, v.Name...) - buf = append(buf, 0) - bindIndices[i][j] = int(C.sqlite3_bind_parameter_index(s.s, (*C.char)(unsafe.Pointer(&buf[0])))) - } - args[i].Ordinal = bindIndices[i][0] + // NB: Parameters with unrecognized names should be ignored: + // https://github.com/mattn/go-sqlite3/issues/697 + bindIndices[i] = s.namedParams[v.Name] + args[i].Ordinal = bindIndices[i] } } var rv C.int for i, arg := range args { - for j := range bindIndices[i] { - if bindIndices[i][j] == 0 { - continue + if bindIndices[i] == 0 { + continue + } + n := C.int(bindIndices[i]) + switch v := arg.Value.(type) { + case nil: + rv = C.sqlite3_bind_null(s.s, n) + case string: + p := stringData(v) + rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(p)), C.int(len(v))) + case int64: + rv = C.sqlite3_bind_int64(s.s, n, C.sqlite3_int64(v)) + case bool: + val := 0 + if v { + val = 1 } - n := C.int(bindIndices[i][j]) - switch v := arg.Value.(type) { - case nil: + rv = C.sqlite3_bind_int(s.s, n, C.int(val)) + case float64: + rv = C.sqlite3_bind_double(s.s, n, C.double(v)) + case []byte: + if v == nil { rv = C.sqlite3_bind_null(s.s, n) - case string: - p := stringData(v) - rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(p)), C.int(len(v))) - case int64: - rv = C.sqlite3_bind_int64(s.s, n, C.sqlite3_int64(v)) - case bool: - val := 0 - if v { - val = 1 - } - rv = C.sqlite3_bind_int(s.s, n, C.int(val)) - case float64: - rv = C.sqlite3_bind_double(s.s, n, C.double(v)) - case []byte: - if v == nil { - rv = C.sqlite3_bind_null(s.s, n) - } else { - ln := len(v) - if ln == 0 { - v = placeHolder - } - rv = C._sqlite3_bind_blob(s.s, n, unsafe.Pointer(&v[0]), C.int(ln)) + } else { + ln := len(v) + if ln == 0 { + v = placeHolder } - case time.Time: - b := timefmt.Format(v) - rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(&b[0])), C.int(len(b))) - } - if rv != C.SQLITE_OK { - return s.c.lastError(int(rv)) + rv = C._sqlite3_bind_blob(s.s, n, unsafe.Pointer(&v[0]), C.int(ln)) } + case time.Time: + b := timefmt.Format(v) + rv = C._sqlite3_bind_text(s.s, n, (*C.char)(unsafe.Pointer(&b[0])), C.int(len(b))) + } + if rv != C.SQLITE_OK { + return s.c.lastError(int(rv)) } } return nil diff --git a/sqlite3_test.go b/sqlite3_test.go index ffb896d6..7da76da4 100644 --- a/sqlite3_test.go +++ b/sqlite3_test.go @@ -1093,8 +1093,6 @@ func TestExecer(t *testing.T) { } func newTestDB(t testing.TB) *sql.DB { - // fmt.Sprintf("file:%s?mode=rwc", filename) - // ?mode=rwc db, err := sql.Open("sqlite3", fmt.Sprintf("file:%s?mode=rwc", t.TempDir()+"/test.sqlite3")) if err != nil { t.Fatal("Failed to open database:", err) @@ -2346,7 +2344,9 @@ func TestNamedParam(t *testing.T) { } defer db.Close() - _, err = db.Exec("drop table foo") + if _, err = db.Exec("drop table if exists foo"); err != nil { + t.Fatal(err) + } _, err = db.Exec("create table foo (id integer, name text, amount integer)") if err != nil { t.Fatal("Failed to create table:", err) @@ -2376,6 +2376,67 @@ func TestNamedParam(t *testing.T) { } } +func TestNamedParamReorder(t *testing.T) { + db := newTestDB(t) + const createTableStmt = ` + CREATE TABLE IF NOT EXISTS test_named_params ( + r0 INTEGER NOT NULL, + r1 INTEGER NOT NULL + ); + DELETE FROM test_named_params; + INSERT INTO test_named_params VALUES (10, 11); + INSERT INTO test_named_params VALUES (20, 21);` + + if _, err := db.Exec(createTableStmt); err != nil { + t.Fatal(err) + } + + const query = ` + SELECT + r0, r1 + FROM + test_named_params + WHERE r0 = :v1 AND r1 = :v2; + ` + stmt, err := db.Prepare(query) + if err != nil { + t.Fatal(err) + } + defer stmt.Close() + + test := func(t testing.TB, arg1, arg2 sql.NamedArg, v1, v2 int64) { + t.Helper() + var i1, i2 int64 + err := stmt.QueryRow(arg1, arg2).Scan(&i1, &i2) + if err != nil { + t.Error(err) + return + } + if i1 != v1 && i2 != v2 { + t.Errorf("got: v1=%d v2=%d want: v1=%d v2=%d", i1, i2, v1, v2) + } + } + + // Deliberately add invalid named params to make sure that they + // don't poison the named param cache. + test(ignoreError{t}, sql.Named("v1", 10), sql.Named("foo", 11), 10, 11) + test(ignoreError{t}, sql.Named("bar", 10), sql.Named("foo", 11), 10, 11) + + test(t, sql.Named("v1", 10), sql.Named("v2", 11), 10, 11) + test(t, sql.Named("v2", 11), sql.Named("v1", 10), 10, 11) // Reverse arg order + + // Change argument values + test(t, sql.Named("v1", 20), sql.Named("v2", 21), 20, 21) + test(t, sql.Named("v2", 21), sql.Named("v1", 20), 20, 21) // Reverse arg order + + // Extra argument should error + var v1, v2 int64 + err = stmt.QueryRow(sql.Named("v1", 10), sql.Named("v2", 11), sql.Named("v3", 12)).Scan(&v1, &v2) + if err == nil { + t.Fatal(err) + } +} + func TestRawBytes(t *testing.T) { db, err := sql.Open("sqlite3", ":memory:") if err != nil { @@ -2555,6 +2616,7 @@ var benchmarks = []testing.InternalBenchmark{ {Name: "BenchmarkScanRawBytes", F: benchmarkScanRawBytes}, {Name: "BenchmarkQueryParallel", F: benchmarkQueryParallel}, {Name: "BenchmarkOpen", F: benchmarkOpen}, + {Name: "BenchmarkNamedParams", F: benchmarkNamedParams}, {Name: "BenchmarkParseTime", F: benchmarkParseTime}, } @@ -3337,6 +3399,69 @@ func benchmarkOpen(b *testing.B) { } } +func benchmarkNamedParams(b *testing.B) { + db, err := sql.Open("sqlite3", ":memory:") + if err != nil { + b.Fatal(err) + } + defer db.Close() + + const createTableStmt = ` + DROP TABLE IF EXISTS bench_named_params; + VACUUM; + CREATE TABLE bench_named_params ( + r0 INTEGER NOT NULL, + r1 INTEGER NOT NULL, + r2 INTEGER NOT NULL, + r3 INTEGER NOT NULL + );` + if _, err := db.Exec(createTableStmt); err != nil { + b.Fatal(err) + } + for i := int64(0); i < 1; i++ { + _, err := db.Exec("INSERT INTO bench_named_params VALUES (?, ?, ?, ?);", i, i, i, i) + if err != nil { + b.Fatal(err) + } + } + // _, err = db.Exec("insert into foo(id, name, amount) values(:id, @name, $amount)", + const query = ` + SELECT + r0 + FROM + bench_named_params + WHERE + r0 >= :v0 AND r1 >= :v1 AND r2 >= :v2 AND r3 >= :v3;` + + stmt, err := db.Prepare(query) + if err != nil { + b.Fatal(err) + } + defer stmt.Close() + + args := []any{ + sql.Named("v0", 0), + sql.Named("v1", 0), + sql.Named("v2", 0), + sql.Named("v3", 0), + } + for i := 0; i < b.N; i++ { + rows, err := stmt.Query(args...) + if err != nil { + b.Fatal(err) + } + var v int64 + for rows.Next() { + if err := rows.Scan(&v); err != nil { + b.Fatal(err) + } + } + if err := rows.Err(); err != nil { + b.Fatal(err) + } + } +} + func benchmarkParseTime(b *testing.B) { db, err := sql.Open("sqlite3", ":memory:") if err != nil { @@ -3379,3 +3504,32 @@ func benchmarkParseTime(b *testing.B) { } } } + +var _ testing.TB = ignoreError{} + +// ignoreError prevents a testing.T from error'ing +type ignoreError struct { + *testing.T +} + +func (t ignoreError) FailNow() {} + +func (t ignoreError) Error(args ...any) { + t.Helper() + t.T.Log(args...) +} + +func (t ignoreError) Errorf(format string, args ...any) { + t.Helper() + t.T.Logf(format, args...) +} + +func (t ignoreError) Fatal(args ...any) { + t.Helper() + t.T.Log(args...) +} + +func (t ignoreError) Fatalf(format string, args ...any) { + t.Helper() + t.T.Logf(format, args...) +}