Skip to content

Commit

Permalink
migrate: Refactor _lockMigrations to avoid forUpdate
Browse files Browse the repository at this point in the history
Since _isLocked is only called just before _lockMigrations, it is redundant
and we can accomplish the same  thing by checking the rowCount returned
by the update (verified by testing with postgres, mysql, sqlite, and
mssql).

The benefit of this change is improving compatbility with CockroachDB,
which does not support FOR UPDATE.

Updates knex#2002
  • Loading branch information
bdarnell committed Aug 13, 2019
1 parent f264b1a commit cc87d59
Showing 1 changed file with 8 additions and 17 deletions.
25 changes: 8 additions & 17 deletions lib/migrate/Migrator.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,32 +240,23 @@ class Migrator {
}
}

_isLocked(trx) {
const tableName = getLockTableName(this.config.tableName);
return getTable(this.knex, tableName, this.config.schemaName)
.transacting(trx)
.forUpdate()
.select('*')
.then((data) => data[0].is_locked);
}

_lockMigrations(trx) {
const tableName = getLockTableName(this.config.tableName);
return getTable(this.knex, tableName, this.config.schemaName)
.transacting(trx)
.update({ is_locked: 1 });
.where('is_locked', '=', 0)
.update({ is_locked: 1 })
.then((rowCount) => {
if (rowCount != 1) {
throw new Error('Migration table is already locked');
}
});
}

_getLock(trx) {
const transact = trx ? (fn) => fn(trx) : (fn) => this.knex.transaction(fn);
return transact((trx) => {
return this._isLocked(trx)
.then((isLocked) => {
if (isLocked) {
throw new Error('Migration table is already locked');
}
})
.then(() => this._lockMigrations(trx));
return this._lockMigrations(trx);
}).catch((err) => {
throw new LockError(err.message);
});
Expand Down

0 comments on commit cc87d59

Please sign in to comment.