v2 #2
v2 #2
Conversation
- Implemented FileTemplate method - Locking during migrations - Custom transactions / disable transactions
josephbuchma
force-pushed
the
v2
branch
2 times, most recently
from
9e1a0be to
8738ef5
Compare
mysql.go
Outdated
| func (drv *Driver) Lock() error { | ||
| // NOTE: versionTx will be implicitly committed by LOCK TABLES | ||
| // the only reason we need it is to get exclusive db connection. | ||
| // TODO: go1.9 has DB.Conn() which returns exclusive connection, use it when time comes. |
There was a problem hiding this comment.
We don't use 1.9 in production yet, waiting some time before upgrading minor version of Go is quite popular practice. I built custom migrations appliance on top of migrate package. Moving it to DB.Conn() is not a big deal, let's wait a while.
mysql.go
Outdated
| versionUpdSQL = "DELETE FROM " + versionsTableName + " WHERE version = ?" | ||
| } | ||
| if _, err = drv.versionTx.Exec(versionUpdSQL, f.Version); err != nil { | ||
| drv.rollbackVersionTx() |
There was a problem hiding this comment.
for the record, if the rollback fails, the error is lost here.
mysql.go
Outdated
| type migration struct { | ||
| // notx determines if default transaction | ||
| // should be disabled | ||
| notx bool |
mysql.go
Outdated
| return fmt.Errorf("Failed to commit lines %d-%d: %s", s.txbegin, s.txend, err) | ||
| } | ||
|
|
||
| // newStmt is a DRYer for migration.parse |
mysql.go
Outdated
| // It locks schema_migrations table, so concurrent execution is safe. | ||
| func (drv *Driver) Migrate(f file.File) error { | ||
| if drv.versionTx == nil { | ||
| return errors.New("migrate must call Lock before Migrate") |
There was a problem hiding this comment.
Why don't you lock here instead of returning an error?
There could be an unlock added as a defer too.
There was a problem hiding this comment.
If we could lock/unlock here, there would be no reason for exposing Lock/Unlock methods in driver interface. migrate package is responsible for locking
| Place SQL between "-- TXBEGIN" and "-- TXEND" comments for custom transaction: | ||
| - you CAN have multiple separate transactions in single migration | ||
| - any SQL not wrapped into TXBEGIN - TXEND will be executed without transaction. | ||
| Add "-- NOTX" comment above all SQL to disable default transaction. NOTE: |
There was a problem hiding this comment.
We should probably unify this with the existing -- disable_ddl_transaction in pg driver
There was a problem hiding this comment.
I think it's better to keep this "directive" comments short to avoid potential misspelling, or we should add some prefix (like ++ in goose) to apply validations and throw error if user misspelled something
Upd for db-journey/migrate#3 and some extra features: