From 126fd4e451a61db8a6b79b609ed54235d32aa761 Mon Sep 17 00:00:00 2001 From: Winter Date: Mon, 11 Aug 2025 18:21:43 -0400 Subject: [PATCH] */db: actually enable foreign keys Change-Id: zrzuvtwqqooklrooxvxlsqzrmnkvuytx `sql.DB` pools connections, which means that there's a good chance that our foreign keys aren't actually being enforced, unless a goroutine happens to be using the connection that was made when we set `pragma foreign_keys = 1`. Signed-off-by: Winter --- appview/db/db.go | 3 +-- cmd/punchcardPopulate/main.go | 2 +- eventconsumer/cursor/sqlite.go | 2 +- knotserver/db/init.go | 3 +-- rbac/rbac.go | 2 +- rbac/rbac_test.go | 2 +- spindle/db/db.go | 3 +-- spindle/secrets/sqlite.go | 2 +- 8 files changed, 8 insertions(+), 11 deletions(-) diff --git a/appview/db/db.go b/appview/db/db.go index e3b3d6e..61c7fb0 100644 --- a/appview/db/db.go +++ b/appview/db/db.go @@ -27,14 +27,13 @@ type Execer interface { } func Make(dbPath string) (*DB, error) { - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", dbPath+"?_foreign_keys=1") if err != nil { return nil, err } _, err = db.Exec(` pragma journal_mode = WAL; pragma synchronous = normal; - pragma foreign_keys = on; pragma temp_store = memory; pragma mmap_size = 30000000000; pragma page_size = 32768; diff --git a/cmd/punchcardPopulate/main.go b/cmd/punchcardPopulate/main.go index 244ddef..23ad90a 100644 --- a/cmd/punchcardPopulate/main.go +++ b/cmd/punchcardPopulate/main.go @@ -11,7 +11,7 @@ import ( ) func main() { - db, err := sql.Open("sqlite3", "./appview.db") + db, err := sql.Open("sqlite3", "./appview.db?_foreign_keys=1") if err != nil { log.Fatal("Failed to open database:", err) } diff --git a/eventconsumer/cursor/sqlite.go b/eventconsumer/cursor/sqlite.go index a6b815a..266fb99 100644 --- a/eventconsumer/cursor/sqlite.go +++ b/eventconsumer/cursor/sqlite.go @@ -21,7 +21,7 @@ func WithTableName(name string) SqliteStoreOpt { } func NewSQLiteStore(dbPath string, opts ...SqliteStoreOpt) (*SqliteStore, error) { - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", dbPath+"?_foreign_keys=1") if err != nil { return nil, fmt.Errorf("failed to open sqlite database: %w", err) } diff --git a/knotserver/db/init.go b/knotserver/db/init.go index d114533..b59ee0b 100644 --- a/knotserver/db/init.go +++ b/knotserver/db/init.go @@ -11,7 +11,7 @@ type DB struct { } func Setup(dbPath string) (*DB, error) { - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", dbPath+"?_foreign_keys=1") if err != nil { return nil, err } @@ -19,7 +19,6 @@ func Setup(dbPath string) (*DB, error) { _, err = db.Exec(` pragma journal_mode = WAL; pragma synchronous = normal; - pragma foreign_keys = on; pragma temp_store = memory; pragma mmap_size = 30000000000; pragma page_size = 32768; diff --git a/rbac/rbac.go b/rbac/rbac.go index a7fd5a0..2def55d 100644 --- a/rbac/rbac.go +++ b/rbac/rbac.go @@ -43,7 +43,7 @@ func NewEnforcer(path string) (*Enforcer, error) { return nil, err } - db, err := sql.Open("sqlite3", path) + db, err := sql.Open("sqlite3", path+"?_foreign_keys=1") if err != nil { return nil, err } diff --git a/rbac/rbac_test.go b/rbac/rbac_test.go index d2eba46..4c7252a 100644 --- a/rbac/rbac_test.go +++ b/rbac/rbac_test.go @@ -14,7 +14,7 @@ import ( ) func setup(t *testing.T) *rbac.Enforcer { - db, err := sql.Open("sqlite3", ":memory:") + db, err := sql.Open("sqlite3", ":memory:?_foreign_keys=1") assert.NoError(t, err) a, err := adapter.NewAdapter(db, "sqlite3", "acl") diff --git a/spindle/db/db.go b/spindle/db/db.go index 89be341..faa0b6b 100644 --- a/spindle/db/db.go +++ b/spindle/db/db.go @@ -11,7 +11,7 @@ type DB struct { } func Make(dbPath string) (*DB, error) { - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", dbPath+"?_foreign_keys=1") if err != nil { return nil, err } @@ -19,7 +19,6 @@ func Make(dbPath string) (*DB, error) { _, err = db.Exec(` pragma journal_mode = WAL; pragma synchronous = normal; - pragma foreign_keys = on; pragma temp_store = memory; pragma mmap_size = 30000000000; pragma page_size = 32768; diff --git a/spindle/secrets/sqlite.go b/spindle/secrets/sqlite.go index f514c39..c74fcde 100644 --- a/spindle/secrets/sqlite.go +++ b/spindle/secrets/sqlite.go @@ -24,7 +24,7 @@ func WithTableName(name string) SqliteManagerOpt { } func NewSQLiteManager(dbPath string, opts ...SqliteManagerOpt) (*SqliteManager, error) { - db, err := sql.Open("sqlite3", dbPath) + db, err := sql.Open("sqlite3", dbPath+"?_foreign_keys=1") if err != nil { return nil, fmt.Errorf("failed to open sqlite database: %w", err) } -- 2.43.0 From eac78a617c480b506080ea298991dbb8519b06d0 Mon Sep 17 00:00:00 2001 From: Winter Date: Tue, 12 Aug 2025 12:44:06 -0400 Subject: [PATCH] */db: set sqlite options across all connections in the pool Change-Id: pynywusvrzypyztyotpymyqwtnwommyq This does result in the removal of some options that aren't supported by the SQLite driver [0], but IMO we can stick with the defaults for these. [0]: https://github.com/mattn/go-sqlite3#connection-string Signed-off-by: Winter --- appview/db/db.go | 53 +++++++++++++++++++++++++------------------ knotserver/db/init.go | 23 +++++++++++-------- spindle/db/db.go | 23 +++++++++++-------- 3 files changed, 59 insertions(+), 40 deletions(-) diff --git a/appview/db/db.go b/appview/db/db.go index 61c7fb0..ba5895f 100644 --- a/appview/db/db.go +++ b/appview/db/db.go @@ -27,19 +27,28 @@ type Execer interface { } func Make(dbPath string) (*DB, error) { - db, err := sql.Open("sqlite3", dbPath+"?_foreign_keys=1") + // https://github.com/mattn/go-sqlite3#connection-string + opts := []string{ + "_foreign_keys=1", + "_journal_mode=WAL", + "_synchronous=NORMAL", + "_auto_vacuum=incremental", + } + + db, err := sql.Open("sqlite3", dbPath+"?"+strings.Join(opts, "&")) + if err != nil { + return nil, err + } + + ctx := context.Background() + + conn, err := db.Conn(ctx) if err != nil { return nil, err } - _, err = db.Exec(` - pragma journal_mode = WAL; - pragma synchronous = normal; - pragma temp_store = memory; - pragma mmap_size = 30000000000; - pragma page_size = 32768; - pragma auto_vacuum = incremental; - pragma busy_timeout = 5000; + defer conn.Close() + _, err = conn.ExecContext(ctx, ` create table if not exists registrations ( id integer primary key autoincrement, domain text not null unique, @@ -467,14 +476,14 @@ func Make(dbPath string) (*DB, error) { } // run migrations - runMigration(db, "add-description-to-repos", func(tx *sql.Tx) error { + runMigration(conn, "add-description-to-repos", func(tx *sql.Tx) error { tx.Exec(` alter table repos add column description text check (length(description) <= 200); `) return nil }) - runMigration(db, "add-rkey-to-pubkeys", func(tx *sql.Tx) error { + runMigration(conn, "add-rkey-to-pubkeys", func(tx *sql.Tx) error { // add unconstrained column _, err := tx.Exec(` alter table public_keys @@ -497,7 +506,7 @@ func Make(dbPath string) (*DB, error) { return nil }) - runMigration(db, "add-rkey-to-comments", func(tx *sql.Tx) error { + runMigration(conn, "add-rkey-to-comments", func(tx *sql.Tx) error { _, err := tx.Exec(` alter table comments drop column comment_at; alter table comments add column rkey text; @@ -505,7 +514,7 @@ func Make(dbPath string) (*DB, error) { return err }) - runMigration(db, "add-deleted-and-edited-to-issue-comments", func(tx *sql.Tx) error { + runMigration(conn, "add-deleted-and-edited-to-issue-comments", func(tx *sql.Tx) error { _, err := tx.Exec(` alter table comments add column deleted text; -- timestamp alter table comments add column edited text; -- timestamp @@ -513,7 +522,7 @@ func Make(dbPath string) (*DB, error) { return err }) - runMigration(db, "add-source-info-to-pulls-and-submissions", func(tx *sql.Tx) error { + runMigration(conn, "add-source-info-to-pulls-and-submissions", func(tx *sql.Tx) error { _, err := tx.Exec(` alter table pulls add column source_branch text; alter table pulls add column source_repo_at text; @@ -522,7 +531,7 @@ func Make(dbPath string) (*DB, error) { return err }) - runMigration(db, "add-source-to-repos", func(tx *sql.Tx) error { + runMigration(conn, "add-source-to-repos", func(tx *sql.Tx) error { _, err := tx.Exec(` alter table repos add column source text; `) @@ -533,8 +542,8 @@ func Make(dbPath string) (*DB, error) { // NOTE: this cannot be done in a transaction, so it is run outside [0] // // [0]: https://sqlite.org/pragma.html#pragma_foreign_keys - db.Exec("pragma foreign_keys = off;") - runMigration(db, "recreate-pulls-column-for-stacking-support", func(tx *sql.Tx) error { + conn.ExecContext(ctx, "pragma foreign_keys = off;") + runMigration(conn, "recreate-pulls-column-for-stacking-support", func(tx *sql.Tx) error { _, err := tx.Exec(` create table pulls_new ( -- identifiers @@ -589,10 +598,10 @@ func Make(dbPath string) (*DB, error) { `) return err }) - db.Exec("pragma foreign_keys = on;") + conn.ExecContext(ctx, "pragma foreign_keys = on;") // run migrations - runMigration(db, "add-spindle-to-repos", func(tx *sql.Tx) error { + runMigration(conn, "add-spindle-to-repos", func(tx *sql.Tx) error { tx.Exec(` alter table repos add column spindle text; `) @@ -600,7 +609,7 @@ func Make(dbPath string) (*DB, error) { }) // recreate and add rkey + created columns with default constraint - runMigration(db, "rework-collaborators-table", func(tx *sql.Tx) error { + runMigration(conn, "rework-collaborators-table", func(tx *sql.Tx) error { // create new table // - repo_at instead of repo integer // - rkey field @@ -659,8 +668,8 @@ func Make(dbPath string) (*DB, error) { type migrationFn = func(*sql.Tx) error -func runMigration(d *sql.DB, name string, migrationFn migrationFn) error { - tx, err := d.Begin() +func runMigration(c *sql.Conn, name string, migrationFn migrationFn) error { + tx, err := c.BeginTx(context.Background(), nil) if err != nil { return err } diff --git a/knotserver/db/init.go b/knotserver/db/init.go index b59ee0b..302f6fc 100644 --- a/knotserver/db/init.go +++ b/knotserver/db/init.go @@ -2,6 +2,7 @@ package db import ( "database/sql" + "strings" _ "github.com/mattn/go-sqlite3" ) @@ -11,20 +12,24 @@ type DB struct { } func Setup(dbPath string) (*DB, error) { - db, err := sql.Open("sqlite3", dbPath+"?_foreign_keys=1") + // https://github.com/mattn/go-sqlite3#connection-string + opts := []string{ + "_foreign_keys=1", + "_journal_mode=WAL", + "_synchronous=NORMAL", + "_auto_vacuum=incremental", + } + + db, err := sql.Open("sqlite3", dbPath+"?"+strings.Join(opts, "&")) if err != nil { return nil, err } - _, err = db.Exec(` - pragma journal_mode = WAL; - pragma synchronous = normal; - pragma temp_store = memory; - pragma mmap_size = 30000000000; - pragma page_size = 32768; - pragma auto_vacuum = incremental; - pragma busy_timeout = 5000; + // NOTE: If any other migration is added here, you MUST + // copy the pattern in appview: use a single sql.Conn + // for every migration. + _, err = db.Exec(` create table if not exists known_dids ( did text primary key ); diff --git a/spindle/db/db.go b/spindle/db/db.go index faa0b6b..78121a6 100644 --- a/spindle/db/db.go +++ b/spindle/db/db.go @@ -2,6 +2,7 @@ package db import ( "database/sql" + "strings" _ "github.com/mattn/go-sqlite3" ) @@ -11,20 +12,24 @@ type DB struct { } func Make(dbPath string) (*DB, error) { - db, err := sql.Open("sqlite3", dbPath+"?_foreign_keys=1") + // https://github.com/mattn/go-sqlite3#connection-string + opts := []string{ + "_foreign_keys=1", + "_journal_mode=WAL", + "_synchronous=NORMAL", + "_auto_vacuum=incremental", + } + + db, err := sql.Open("sqlite3", dbPath+"?"+strings.Join(opts, "&")) if err != nil { return nil, err } - _, err = db.Exec(` - pragma journal_mode = WAL; - pragma synchronous = normal; - pragma temp_store = memory; - pragma mmap_size = 30000000000; - pragma page_size = 32768; - pragma auto_vacuum = incremental; - pragma busy_timeout = 5000; + // NOTE: If any other migration is added here, you MUST + // copy the pattern in appview: use a single sql.Conn + // for every migration. + _, err = db.Exec(` create table if not exists _jetstream ( id integer primary key autoincrement, last_time_us integer not null -- 2.43.0