Skip to content

Commit a49e149

Browse files
committed
Fixed sqlitemigration foreign key toggling
Fixes #54
1 parent 4b4013a commit a49e149

File tree

5 files changed

+160
-51
lines changed

5 files changed

+160
-51
lines changed

CHANGELOG.md

+12-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,18 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8-
[Unreleased]: https://github.com/zombiezen/go-sqlite/compare/v0.13.0...main
8+
[Unreleased]: https://github.com/zombiezen/go-sqlite/compare/v0.13.1...main
9+
10+
## [0.13.1][] - 2023-08-15
11+
12+
Version 0.13.1 fixed a bug with the `sqlitemigration` package.
13+
14+
[0.13.1]: https://github.com/zombiezen/go-sqlite/releases/tag/v0.13.1
15+
16+
### Fixed
17+
18+
- `sqlitemigration` will no longer disable foreign keys during operation
19+
([#54](https://github.com/zombiezen/go-sqlite/issues/54)).
920

1021
## [0.13.0][] - 2023-03-28
1122

go.work.sum

+10-11
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
github.com/google/pprof v0.0.0-20221118152302-e6195bd50e26 h1:Xim43kblpZXfIBQsbuBVKCudVG457BR2GZFIz3uw3hQ=
2+
github.com/google/pprof v0.0.0-20221118152302-e6195bd50e26/go.mod h1:dDKJzRmX4S37WGHujM7tX//fmj1uioxKzKxz3lo4HJo=
23
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
4+
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
35
github.com/mattn/go-sqlite3 v1.14.16 h1:yOQRA0RpS5PFz/oikGwBEqvAWhWg5ufRz4ETLjwpU1Y=
46
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
5-
github.com/google/pprof v0.0.0-20221118152302-e6195bd50e26 h1:Xim43kblpZXfIBQsbuBVKCudVG457BR2GZFIz3uw3hQ=
6-
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
7-
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
87
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
98
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
109
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
@@ -22,19 +21,19 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
2221
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
2322
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
2423
lukechampine.com/uint128 v1.2.0 h1:mBi/5l91vocEN8otkC5bDLhi2KdCticRiwbdB0O+rjI=
24+
lukechampine.com/uint128 v1.2.0/go.mod h1:c4eWIwlEGaxC/+H1VguhU4PHXNWDCDMUlWdIWl2j1gk=
2525
modernc.org/cc/v3 v3.40.0 h1:P3g79IUS/93SYhtoeaHW+kRCIrYaxJ27MFPv+7kaTOw=
26+
modernc.org/cc/v3 v3.40.0/go.mod h1:/bTg4dnWkSXowUO6ssQKnOV0yMVxDYNIsIrzqTFDGH0=
2627
modernc.org/ccgo/v3 v3.16.13 h1:Mkgdzl46i5F/CNR/Kj80Ri59hC8TKAhZrYSaqvkwzUw=
28+
modernc.org/ccgo/v3 v3.16.13/go.mod h1:2Quk+5YgpImhPjv2Qsob1DnZ/4som1lJTodubIcoUkY=
2729
modernc.org/httpfs v1.0.6 h1:AAgIpFZRXuYnkjftxTAZwMIiwEqAfk8aVB2/oA6nAeM=
30+
modernc.org/httpfs v1.0.6/go.mod h1:7dosgurJGp0sPaRanU53W4xZYKh14wfzX420oZADeHM=
2831
modernc.org/opt v0.1.3 h1:3XOZf2yznlhC+ibLltsDGzABUGVx8J6pnFMS3E4dcq4=
32+
modernc.org/opt v0.1.3/go.mod h1:WdSiB5evDcignE70guQKxYUl14mgWtbClRi5wmkkTX0=
2933
modernc.org/strutil v1.1.3 h1:fNMm+oJklMGYfU9Ylcywl0CO5O6nTfaowNsh2wpPjzY=
30-
modernc.org/token v1.0.1 h1:A3qvTqOwexpfZZeyI0FeGPDlSWX5pjZu9hF4lU+EKWg=
31-
modernc.org/z v1.7.0 h1:xkDw/KepgEjeizO2sNco+hqYkU12taxQFqPEmgm1GWE=
32-
lukechampine.com/uint128 v1.2.0 h1:mBi/5l91vocEN8otkC5bDLhi2KdCticRiwbdB0O+rjI=
33-
modernc.org/cc/v3 v3.40.0 h1:P3g79IUS/93SYhtoeaHW+kRCIrYaxJ27MFPv+7kaTOw=
34-
modernc.org/ccgo/v3 v3.16.13 h1:Mkgdzl46i5F/CNR/Kj80Ri59hC8TKAhZrYSaqvkwzUw=
35-
modernc.org/httpfs v1.0.6 h1:AAgIpFZRXuYnkjftxTAZwMIiwEqAfk8aVB2/oA6nAeM=
36-
modernc.org/opt v0.1.3 h1:3XOZf2yznlhC+ibLltsDGzABUGVx8J6pnFMS3E4dcq4=
37-
modernc.org/strutil v1.1.3 h1:fNMm+oJklMGYfU9Ylcywl0CO5O6nTfaowNsh2wpPjzY=
34+
modernc.org/strutil v1.1.3/go.mod h1:MEHNA7PdEnEwLvspRMtWTNnp2nnyvMfkimT1NKNAGbw=
3835
modernc.org/tcl v1.15.1 h1:mOQwiEK4p7HruMZcwKTZPw/aqtGM4aY00uzWhlKKYws=
3936
modernc.org/token v1.0.1 h1:A3qvTqOwexpfZZeyI0FeGPDlSWX5pjZu9hF4lU+EKWg=
37+
modernc.org/token v1.0.1/go.mod h1:UGzOrNV1mAFSEB63lOFHIpNRUVMvYTc6yu1SMY/XTDM=
4038
modernc.org/z v1.7.0 h1:xkDw/KepgEjeizO2sNco+hqYkU12taxQFqPEmgm1GWE=
39+
modernc.org/z v1.7.0/go.mod h1:hVdgNMh8ggTuRG1rGU8x+xGRFfiQUIAw0ZqlPy8+HyQ=

sqlite_test.go

+37
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,43 @@ func TestSerialize(t *testing.T) {
948948
}
949949
}
950950

951+
func TestForeignKey(t *testing.T) {
952+
c, err := sqlite.OpenConn(":memory:")
953+
if err != nil {
954+
t.Fatal(err)
955+
}
956+
defer func() {
957+
if err := c.Close(); err != nil {
958+
t.Error(err)
959+
}
960+
}()
961+
962+
err = sqlitex.ExecuteTransient(c, `PRAGMA foreign_keys = on;`, nil)
963+
if err != nil {
964+
t.Fatal(err)
965+
}
966+
err = sqlitex.ExecuteScript(c, `CREATE TABLE artist(
967+
artistid INTEGER PRIMARY KEY,
968+
artistname TEXT
969+
);
970+
CREATE TABLE track(
971+
trackid INTEGER,
972+
trackname TEXT,
973+
trackartist INTEGER,
974+
FOREIGN KEY(trackartist) REFERENCES artist(artistid)
975+
);`, nil)
976+
if err != nil {
977+
t.Fatal(err)
978+
}
979+
980+
err = sqlitex.ExecuteTransient(c, `INSERT INTO track VALUES(14, 'Mr. Bojangles', 3);`, nil)
981+
if err == nil {
982+
t.Fatal("No error from breaking foreign key")
983+
} else {
984+
t.Log("Got (intentional) error:", err)
985+
}
986+
}
987+
951988
func TestMain(m *testing.M) {
952989
_ = libc.Environ() // Forces libc.SetEnviron; fixes memory accounting balance for environ(7).
953990
libc.MemAuditStart()

sqlitemigration/sqlitemigration.go

+28-39
Original file line numberDiff line numberDiff line change
@@ -287,13 +287,7 @@ func Migrate(ctx context.Context, conn *sqlite.Conn, schema Schema) error {
287287
func migrateDB(ctx context.Context, conn *sqlite.Conn, schema Schema, onStart SignalFunc) error {
288288
defer conn.SetInterrupt(conn.SetInterrupt(ctx.Done()))
289289

290-
userVersionStmt, _, err := conn.PrepareTransient("PRAGMA user_version;")
291-
if err != nil {
292-
return fmt.Errorf("migrate database: %w", err)
293-
}
294-
defer userVersionStmt.Finalize()
295-
296-
schemaVersion, err := ensureAppID(conn, schema.AppID, userVersionStmt)
290+
schemaVersion, err := ensureAppID(conn, schema.AppID)
297291
if err != nil {
298292
return fmt.Errorf("migrate database: %w", err)
299293
}
@@ -303,26 +297,13 @@ func migrateDB(ctx context.Context, conn *sqlite.Conn, schema Schema, onStart Si
303297
var foreignKeysEnabled bool
304298
err = sqlitex.ExecuteTransient(conn, "PRAGMA foreign_keys;", &sqlitex.ExecOptions{
305299
ResultFunc: func(stmt *sqlite.Stmt) error {
306-
foreignKeysEnabled = stmt.ColumnInt(0) != 0
300+
foreignKeysEnabled = stmt.ColumnBool(0)
307301
return nil
308302
},
309303
})
310304
if err != nil {
311305
return fmt.Errorf("migrate database: %w", err)
312306
}
313-
var fkOnStmt, fkOffStmt *sqlite.Stmt
314-
if foreignKeysEnabled {
315-
fkOnStmt, _, err = conn.PrepareTransient("PRAGMA foreign_keys = on;")
316-
if err != nil {
317-
return fmt.Errorf("migrate database: %w", err)
318-
}
319-
defer fkOnStmt.Finalize()
320-
fkOffStmt, _, err = conn.PrepareTransient("PRAGMA foreign_keys = off;")
321-
if err != nil {
322-
return fmt.Errorf("migrate database: %w", err)
323-
}
324-
defer fkOffStmt.Finalize()
325-
}
326307

327308
beginStmt, _, err := conn.PrepareTransient("BEGIN IMMEDIATE;")
328309
if err != nil {
@@ -334,27 +315,24 @@ func migrateDB(ctx context.Context, conn *sqlite.Conn, schema Schema, onStart Si
334315
return fmt.Errorf("migrate database: %w", err)
335316
}
336317
defer commitStmt.Finalize()
337-
for ; schemaVersion < len(schema.Migrations); schemaVersion++ {
318+
for ; int(schemaVersion) < len(schema.Migrations); schemaVersion++ {
338319
migration := schema.Migrations[schemaVersion]
339320
disableFKs := foreignKeysEnabled &&
340-
schemaVersion < len(schema.MigrationOptions) &&
321+
int(schemaVersion) < len(schema.MigrationOptions) &&
341322
schema.MigrationOptions[schemaVersion] != nil &&
342323
schema.MigrationOptions[schemaVersion].DisableForeignKeys
343324
if disableFKs {
344-
if err := stepAndReset(fkOffStmt); err != nil {
325+
// Do not try to optimize by preparing this PRAGMA statement ahead of time.
326+
if err := sqlitex.ExecuteTransient(conn, "PRAGMA foreign_keys = off;", nil); err != nil {
345327
return fmt.Errorf("migrate database: disable foreign keys: %w", err)
346328
}
347329
}
348330

349331
if err := stepAndReset(beginStmt); err != nil {
350332
return fmt.Errorf("migrate database: apply migrations[%d]: %w", schemaVersion, err)
351333
}
352-
if _, err := userVersionStmt.Step(); err != nil {
353-
rollback(conn)
354-
return fmt.Errorf("migrate database: %w", err)
355-
}
356-
actualSchemaVersion := userVersionStmt.ColumnInt(0)
357-
if err := userVersionStmt.Reset(); err != nil {
334+
actualSchemaVersion, err := userVersion(conn)
335+
if err != nil {
358336
rollback(conn)
359337
return fmt.Errorf("migrate database: %w", err)
360338
}
@@ -365,12 +343,12 @@ func migrateDB(ctx context.Context, conn *sqlite.Conn, schema Schema, onStart Si
365343
continue
366344
}
367345

368-
err := sqlitex.ExecScript(conn, fmt.Sprintf("%s;\nPRAGMA user_version = %d;\n", migration, schemaVersion+1))
346+
err = sqlitex.ExecScript(conn, fmt.Sprintf("%s;\nPRAGMA user_version = %d;\n", migration, schemaVersion+1))
369347
if err != nil {
370348
rollback(conn)
371349
return fmt.Errorf("migrate database: apply migrations[%d]: %w", schemaVersion, err)
372350
}
373-
if schemaVersion == len(schema.Migrations)-1 && schema.RepeatableMigration != "" {
351+
if int(schemaVersion) == len(schema.Migrations)-1 && schema.RepeatableMigration != "" {
374352
if err := sqlitex.ExecScript(conn, schema.RepeatableMigration); err != nil {
375353
rollback(conn)
376354
return fmt.Errorf("migrate database: apply repeatable migration: %w", err)
@@ -382,22 +360,36 @@ func migrateDB(ctx context.Context, conn *sqlite.Conn, schema Schema, onStart Si
382360
return fmt.Errorf("migrate database: apply migrations[%d]: %w", schemaVersion, err)
383361
}
384362
if disableFKs {
385-
if err := stepAndReset(fkOnStmt); err != nil {
363+
if err := sqlitex.ExecuteTransient(conn, "PRAGMA foreign_keys = on;", nil); err != nil {
386364
return fmt.Errorf("migrate database: reenable foreign keys: %w", err)
387365
}
388366
}
389367
}
390368
return nil
391369
}
392370

371+
func userVersion(conn *sqlite.Conn) (int32, error) {
372+
var version int32
373+
err := sqlitex.ExecuteTransient(conn, "PRAGMA user_version;", &sqlitex.ExecOptions{
374+
ResultFunc: func(stmt *sqlite.Stmt) error {
375+
version = stmt.ColumnInt32(0)
376+
return nil
377+
},
378+
})
379+
if err != nil {
380+
return 0, fmt.Errorf("get database user_version: %w", err)
381+
}
382+
return version, nil
383+
}
384+
393385
func rollback(conn *sqlite.Conn) {
394386
if conn.AutocommitEnabled() {
395387
return
396388
}
397389
sqlitex.ExecuteTransient(conn, "ROLLBACK;", nil)
398390
}
399391

400-
func ensureAppID(conn *sqlite.Conn, wantAppID int32, userVersionStmt *sqlite.Stmt) (schemaVersion int, err error) {
392+
func ensureAppID(conn *sqlite.Conn, wantAppID int32) (schemaVersion int32, err error) {
401393
defer sqlitex.Save(conn)(&err)
402394

403395
var hasSchema bool
@@ -423,11 +415,8 @@ func ensureAppID(conn *sqlite.Conn, wantAppID int32, userVersionStmt *sqlite.Stm
423415
if dbAppID != wantAppID && !(dbAppID == 0 && !hasSchema) {
424416
return 0, fmt.Errorf("database application_id = %#x (expected %#x)", dbAppID, wantAppID)
425417
}
426-
if _, err := userVersionStmt.Step(); err != nil {
427-
return 0, err
428-
}
429-
schemaVersion = userVersionStmt.ColumnInt(0)
430-
if err := userVersionStmt.Reset(); err != nil {
418+
schemaVersion, err = userVersion(conn)
419+
if err != nil {
431420
return 0, err
432421
}
433422
// Using Sprintf because PRAGMAs don't permit arbitrary expressions, and thus

sqlitemigration/sqlitemigration_test.go

+73
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,79 @@ func TestPool(t *testing.T) {
844844
t.Error("Foreign keys were left disabled after migration")
845845
}
846846
})
847+
848+
t.Run("NoTouchForeignKeys", func(t *testing.T) {
849+
schema := Schema{
850+
AppID: 0xedbeef,
851+
Migrations: []string{
852+
`create table foo ( foreign_keys_enabled bool ); insert into foo values ((select * from pragma_foreign_keys()));`,
853+
},
854+
}
855+
pool := NewPool(filepath.Join(t.TempDir(), "no-touch-foreign-keys.db"), schema, Options{
856+
Flags: sqlite.OpenReadWrite | sqlite.OpenCreate,
857+
PrepareConn: func(conn *sqlite.Conn) error {
858+
return sqlitex.ExecuteTransient(conn, "PRAGMA foreign_keys = on;", nil)
859+
},
860+
})
861+
defer func() {
862+
if err := pool.Close(); err != nil {
863+
t.Error("pool.Close:", err)
864+
}
865+
}()
866+
conn, err := pool.Get(ctx)
867+
if err != nil {
868+
t.Fatal(err)
869+
}
870+
defer pool.Put(conn)
871+
duringMigration, err := sqlitex.ResultBool(conn.Prep("select foreign_keys_enabled from foo;"))
872+
if err != nil {
873+
t.Error(err)
874+
} else if !duringMigration {
875+
t.Error("Foreign keys were disabled during migration")
876+
}
877+
afterMigration, err := sqlitex.ResultBool(conn.Prep("PRAGMA foreign_keys;"))
878+
if err != nil {
879+
t.Error(err)
880+
} else if !afterMigration {
881+
t.Error("Foreign keys were disabled after migration")
882+
}
883+
})
884+
}
885+
886+
func TestMigrate(t *testing.T) {
887+
t.Run("NoTouchForeignKeys", func(t *testing.T) {
888+
conn, err := sqlite.OpenConn(filepath.Join(t.TempDir(), "no-touch-foreign-keys.db"), sqlite.OpenReadWrite, sqlite.OpenCreate)
889+
if err != nil {
890+
t.Fatal(err)
891+
}
892+
defer conn.Close()
893+
err = sqlitex.ExecuteTransient(conn, `PRAGMA foreign_keys = on;`, nil)
894+
if err != nil {
895+
t.Fatal(err)
896+
}
897+
898+
schema := Schema{
899+
AppID: 0xedbeef,
900+
Migrations: []string{
901+
`create table foo ( foreign_keys_enabled bool ); insert into foo values ((select * from pragma_foreign_keys()));`,
902+
},
903+
}
904+
if err := Migrate(context.Background(), conn, schema); err != nil {
905+
t.Error(err)
906+
}
907+
duringMigration, err := sqlitex.ResultBool(conn.Prep("select foreign_keys_enabled from foo;"))
908+
if err != nil {
909+
t.Error(err)
910+
} else if !duringMigration {
911+
t.Error("Foreign keys were disabled during migration")
912+
}
913+
afterMigration, err := sqlitex.ResultBool(conn.Prep("PRAGMA foreign_keys;"))
914+
if err != nil {
915+
t.Error(err)
916+
} else if !afterMigration {
917+
t.Error("Foreign keys were disabled after migration")
918+
}
919+
})
847920
}
848921

849922
// withTestConn makes an independent connection to the given database.

0 commit comments

Comments
 (0)