Commit b0b75f8f4c
b0b75f8f4ca4905b077ee3af67f6da94c64cde43
parent: 02fa58993b
Verified · cmc
cmc <hello@cleberg.net> · 2026-09-11 16:19 UTC
store: a failed fk-off migration step re-enables foreign keys
The pinned connection went back to the pool with foreign keys off
whenever the step failed before the explicit re-enable, and whatever drew
it next ran without them. A defer registered beside the switch-off does
it on every path, reporting its own failure when nothing else did. The
0052 test resolves the issue row id through IssueByNumber.
Ref #203
internal/store/store.go
+13 −2
| @@ -173,7 +173,7 @@ func (s *Store) migrateTo(target int) error { |
| 173 | 173 | if err != nil { |
| 174 | 174 | return err |
| 175 | 175 | } |
| 176 | | step := func(sqlText string, newVersion int, fkOff bool) error { |
| 176 | step := func(sqlText string, newVersion int, fkOff bool) (retErr error) { |
| 177 | 177 | if !fkOff { |
| 178 | 178 | tx, err := s.DB.Begin() |
| 179 | 179 | if err != nil { |
| @@ -206,6 +206,17 @@ func (s *Store) migrateTo(target int) error { |
| 206 | 206 | if _, err := conn.ExecContext(ctx, "PRAGMA foreign_keys = OFF"); err != nil { |
| 207 | 207 | return err |
| 208 | 208 | } |
| 209 | // The connection goes back to the pool when this returns, so every |
| 210 | // path out of here has to put foreign keys back on first. |
| 211 | restoreFK := func() error { |
| 212 | _, err := conn.ExecContext(ctx, "PRAGMA foreign_keys = ON") |
| 213 | return err |
| 214 | } |
| 215 | defer func() { |
| 216 | if err := restoreFK(); err != nil && retErr == nil { |
| 217 | retErr = err |
| 218 | } |
| 219 | }() |
| 209 | 220 | tx, err := conn.BeginTx(ctx, nil) |
| 210 | 221 | if err != nil { |
| 211 | 222 | return err |
| @@ -220,7 +231,7 @@ func (s *Store) migrateTo(target int) error { |
| 220 | 231 | if err := tx.Commit(); err != nil { |
| 221 | 232 | return err |
| 222 | 233 | } |
| 223 | | if _, err := conn.ExecContext(ctx, "PRAGMA foreign_keys = ON"); err != nil { |
| 234 | if err := restoreFK(); err != nil { |
| 224 | 235 | return err |
| 225 | 236 | } |
| 226 | 237 | rows, err := conn.QueryContext(ctx, "PRAGMA foreign_key_check") |
internal/store/store_test.go
+8 −1
| @@ -166,10 +166,17 @@ func TestMigration0052KeepsMembershipsAndForeignKeys(t *testing.T) { |
| 166 | 166 | if err != nil { |
| 167 | 167 | t.Fatal(err) |
| 168 | 168 | } |
| 169 | | iid, err := s.CreateIssue(rid, uid, "one", "", "md") |
| 169 | number, err := s.CreateIssue(rid, uid, "one", "", "md") |
| 170 | 170 | if err != nil { |
| 171 | 171 | t.Fatal(err) |
| 172 | 172 | } |
| 173 | // CreateIssue returns the per-repo number; the rows below reference |
| 174 | // the issues.id row. |
| 175 | issue, err := s.IssueByNumber(rid, number) |
| 176 | if err != nil { |
| 177 | t.Fatal(err) |
| 178 | } |
| 179 | iid := issue.ID |
| 173 | 180 | if _, err := s.DB.Exec("INSERT INTO labels (repo_id, name, color) VALUES (?, 'bug', '#ff0000')", rid); err != nil { |
| 174 | 181 | t.Fatal(err) |
| 175 | 182 | } |