diff --git a/docs/find-options.md b/docs/find-options.md index a6741538455..79ff1e7411c 100644 --- a/docs/find-options.md +++ b/docs/find-options.md @@ -239,6 +239,7 @@ Support of lock modes, and SQL statements they translate to, are listed in the t | Oracle | FOR UPDATE | FOR UPDATE | (nothing) | | | | | SQL Server | WITH (HOLDLOCK, ROWLOCK) | WITH (UPDLOCK, ROWLOCK) | WITH (NOLOCK) | | | | | AuroraDataApi | LOCK IN SHARE MODE | FOR UPDATE | (nothing) | | | | +| CockroachDB | | FOR UPDATE | (nothing) | | FOR UPDATE NOWAIT | FOR NO KEY UPDATE | ``` diff --git a/src/query-builder/SelectQueryBuilder.ts b/src/query-builder/SelectQueryBuilder.ts index f26dfe86bd2..e4b37a8c8c4 100644 --- a/src/query-builder/SelectQueryBuilder.ts +++ b/src/query-builder/SelectQueryBuilder.ts @@ -1679,7 +1679,7 @@ export class SelectQueryBuilder extends QueryBuilder implements let lockTablesClause = ""; if (this.expressionMap.lockTables) { - if (!(driver instanceof PostgresDriver)) { + if (!(driver instanceof PostgresDriver || driver instanceof CockroachDriver)) { throw new TypeORMError("Lock tables not supported in selected driver"); } if (this.expressionMap.lockTables.length < 1) { @@ -1710,7 +1710,7 @@ export class SelectQueryBuilder extends QueryBuilder implements return " FOR UPDATE"; } - else if (driver instanceof PostgresDriver ) { + else if (driver instanceof PostgresDriver || driver instanceof CockroachDriver) { return " FOR UPDATE" + lockTablesClause; } else if (driver instanceof SqlServerDriver) { @@ -1730,7 +1730,7 @@ export class SelectQueryBuilder extends QueryBuilder implements throw new LockNotSupportedOnGivenDriverError(); } case "pessimistic_write_or_fail": - if (driver instanceof PostgresDriver) { + if (driver instanceof PostgresDriver || driver instanceof CockroachDriver) { return " FOR UPDATE" + lockTablesClause + " NOWAIT"; } else if (driver instanceof MysqlDriver) { @@ -1741,7 +1741,7 @@ export class SelectQueryBuilder extends QueryBuilder implements } case "for_no_key_update": - if (driver instanceof PostgresDriver) { + if (driver instanceof PostgresDriver || driver instanceof CockroachDriver) { return " FOR NO KEY UPDATE" + lockTablesClause; } else { throw new LockNotSupportedOnGivenDriverError(); diff --git a/test/functional/query-builder/locking/query-builder-locking.ts b/test/functional/query-builder/locking/query-builder-locking.ts index 18e49ea4c98..2a299e051a5 100644 --- a/test/functional/query-builder/locking/query-builder-locking.ts +++ b/test/functional/query-builder/locking/query-builder-locking.ts @@ -61,9 +61,20 @@ describe("query builder > locking", () => { }))); it("should not throw error if pessimistic lock used with transaction", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof CockroachDriver || connection.driver instanceof SapDriver) + if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof SapDriver) return; + if (connection.driver instanceof CockroachDriver) { + return connection.manager.transaction(entityManager => { + return Promise.all([ + entityManager.createQueryBuilder(PostWithVersion, "post") + .setLock("pessimistic_write") + .where("post.id = :id", { id: 1 }) + .getOne().should.not.be.rejected + ]); + }); + } + return connection.manager.transaction(entityManager => { return Promise.all([ entityManager.createQueryBuilder(PostWithVersion, "post") @@ -80,7 +91,7 @@ describe("query builder > locking", () => { }))); it("should throw error if for no key update lock used without transaction", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof PostgresDriver) { + if (connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver) { return connection.createQueryBuilder(PostWithVersion, "post") .setLock("for_no_key_update") .where("post.id = :id", { id: 1 }) @@ -90,7 +101,7 @@ describe("query builder > locking", () => { }))); it("should not throw error if for no key update lock used with transaction", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof PostgresDriver) { + if (connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver) { return connection.manager.transaction(entityManager => { return Promise.all([entityManager.createQueryBuilder(PostWithVersion, "post") .setLock("for_no_key_update") @@ -119,14 +130,14 @@ describe("query builder > locking", () => { return connection.createQueryBuilder(PostWithVersion, "post") .setLock("pessimistic_partial_write") .where("post.id = :id", { id: 1 }) - .getOne().should.be.rejectedWith(PessimisticLockTransactionRequiredError); + .getOne().should.be.rejectedWith(PessimisticLockTransactionRequiredError); } } return; - }))); + }))); it("should not throw error if pessimistic_partial_write lock used with transaction", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof PostgresDriver) { + if (connection.driver instanceof PostgresDriver) { return connection.manager.transaction(entityManager => { return Promise.all([entityManager.createQueryBuilder(PostWithVersion, "post") .setLock("pessimistic_partial_write") @@ -149,12 +160,12 @@ describe("query builder > locking", () => { .getOne().should.not.be.rejected]); }); } - } + } return; }))); - + it("should throw error if pessimistic_write_or_fail lock used without transaction", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof PostgresDriver) { + if (connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver) { return connection.createQueryBuilder(PostWithVersion, "post") .setLock("pessimistic_write_or_fail") .where("post.id = :id", { id: 1 }) @@ -173,11 +184,11 @@ describe("query builder > locking", () => { .getOne().should.be.rejectedWith(PessimisticLockTransactionRequiredError); } } - return; - }))); + return; + }))); it("should not throw error if pessimistic_write_or_fail lock used with transaction", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof PostgresDriver) { + if (connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver) { return connection.manager.transaction(entityManager => { return Promise.all([entityManager.createQueryBuilder(PostWithVersion, "post") .setLock("pessimistic_write_or_fail") @@ -199,9 +210,9 @@ describe("query builder > locking", () => { .getOne().should.not.be.rejected]); }); } - } + } return; - }))); + }))); it("should attach pessimistic read lock statement on query if locking enabled", () => Promise.all(connections.map(async connection => { if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof CockroachDriver || connection.driver instanceof SapDriver) @@ -250,7 +261,7 @@ describe("query builder > locking", () => { }))); it("should attach pessimistic write lock statement on query if locking enabled", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof CockroachDriver || connection.driver instanceof SapDriver) + if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof SapDriver) return; const sql = connection.createQueryBuilder(PostWithVersion, "post") @@ -258,7 +269,7 @@ describe("query builder > locking", () => { .where("post.id = :id", { id: 1 }) .getSql(); - if (connection.driver instanceof MysqlDriver || connection.driver instanceof PostgresDriver || connection.driver instanceof OracleDriver) { + if (connection.driver instanceof MysqlDriver || connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver || connection.driver instanceof OracleDriver) { expect(sql.indexOf("FOR UPDATE") !== -1).to.be.true; } else if (connection.driver instanceof SqlServerDriver) { @@ -268,7 +279,7 @@ describe("query builder > locking", () => { }))); it("should not attach for no key update lock statement on query if locking is not used", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof PostgresDriver) { + if (connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver) { const sql = connection.createQueryBuilder(PostWithVersion, "post") .where("post.id = :id", { id: 1 }) .getSql(); @@ -279,7 +290,7 @@ describe("query builder > locking", () => { }))); it("should attach for no key update lock statement on query if locking enabled", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof PostgresDriver) { + if (connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver) { const sql = connection.createQueryBuilder(PostWithVersion, "post") .setLock("for_no_key_update") .where("post.id = :id", { id: 1 }) @@ -300,7 +311,7 @@ describe("query builder > locking", () => { expect(sql.indexOf("FOR UPDATE SKIP LOCKED") === -1).to.be.true; } return; - }))); + }))); it("should attach pessimistic_partial_write lock statement on query if locking enabled", () => Promise.all(connections.map(async connection => { if (connection.driver instanceof PostgresDriver || connection.driver instanceof MysqlDriver) { @@ -313,10 +324,10 @@ describe("query builder > locking", () => { } return; - }))); + }))); it("should not attach pessimistic_write_or_fail lock statement on query if locking is not used", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof PostgresDriver || connection.driver instanceof MysqlDriver) { + if (connection.driver instanceof PostgresDriver || connection.driver instanceof MysqlDriver || connection.driver instanceof CockroachDriver) { const sql = connection.createQueryBuilder(PostWithVersion, "post") .where("post.id = :id", { id: 1 }) .getSql(); @@ -324,10 +335,10 @@ describe("query builder > locking", () => { expect(sql.indexOf("FOR UPDATE NOWAIT") === -1).to.be.true; } return; - }))); + }))); it("should attach pessimistic_write_or_fail lock statement on query if locking enabled", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof PostgresDriver || connection.driver instanceof MysqlDriver) { + if (connection.driver instanceof PostgresDriver || connection.driver instanceof MysqlDriver || connection.driver instanceof CockroachDriver) { const sql = connection.createQueryBuilder(PostWithVersion, "post") .setLock("pessimistic_write_or_fail") .where("post.id = :id", { id: 1 }) @@ -337,7 +348,7 @@ describe("query builder > locking", () => { } return; - }))); + }))); it("should throw error if optimistic lock used with getMany method", () => Promise.all(connections.map(async connection => { @@ -471,7 +482,7 @@ describe("query builder > locking", () => { }))); it("should throw error if pessimistic locking not supported by given driver", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof CockroachDriver || connection.driver instanceof SapDriver) + if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof SapDriver) return connection.manager.transaction(entityManager => { return Promise.all([ entityManager.createQueryBuilder(PostWithVersion, "post") @@ -486,11 +497,21 @@ describe("query builder > locking", () => { ]); }); + if (connection.driver instanceof CockroachDriver) + return connection.manager.transaction(entityManager => { + return Promise.all([ + entityManager.createQueryBuilder(PostWithVersion, "post") + .setLock("pessimistic_read") + .where("post.id = :id", { id: 1 }) + .getOne().should.be.rejectedWith(LockNotSupportedOnGivenDriverError), + ]); + }); + return; }))); it("should throw error if for no key update locking not supported by given driver", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) { + if (!(connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver)) { return connection.manager.transaction(entityManager => { return Promise.all([ entityManager.createQueryBuilder(PostWithVersion, "post") @@ -505,7 +526,7 @@ describe("query builder > locking", () => { }))); it("should only specify locked tables in FOR UPDATE OF clause if argument is given", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) + if (!(connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver)) return; const sql = connection.createQueryBuilder(Post, "post") @@ -524,7 +545,7 @@ describe("query builder > locking", () => { }))); it("should not allow empty array for lockTables", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) + if (!(connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver)) return; return connection.manager.transaction(entityManager => { @@ -538,7 +559,7 @@ describe("query builder > locking", () => { }))); it("should throw error when specifying a table that is not part of the query", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) + if (!(connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver)) return; return connection.manager.transaction(entityManager => { @@ -552,22 +573,38 @@ describe("query builder > locking", () => { }))); it("should allow on a left join", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) - return; + if (connection.driver instanceof CockroachDriver) { + return connection.manager.transaction(entityManager => { + return Promise.all([ + entityManager.createQueryBuilder(Post, "post") + .leftJoin("post.author", "user") + .setLock('pessimistic_write', undefined, ["post"]) + .getOne(), + entityManager.createQueryBuilder(Post, "post") + .leftJoin("post.author", "user") + .setLock('pessimistic_write') + .getOne(), + ]); + }); + } - return connection.manager.transaction(entityManager => { + if (connection.driver instanceof PostgresDriver) { + return connection.manager.transaction(entityManager => { - return Promise.all([ - entityManager.createQueryBuilder(Post, "post") - .leftJoin("post.author", "user") - .setLock('pessimistic_write', undefined, ["post"]) - .getOne(), - entityManager.createQueryBuilder(Post, "post") - .leftJoin("post.author", "user") - .setLock('pessimistic_write') - .getOne().should.be.rejectedWith('FOR UPDATE cannot be applied to the nullable side of an outer join'), - ]); - }); + return Promise.all([ + entityManager.createQueryBuilder(Post, "post") + .leftJoin("post.author", "user") + .setLock('pessimistic_write', undefined, ["post"]) + .getOne(), + entityManager.createQueryBuilder(Post, "post") + .leftJoin("post.author", "user") + .setLock('pessimistic_write') + .getOne().should.be.rejectedWith('FOR UPDATE cannot be applied to the nullable side of an outer join'), + ]); + }); + } + + return; }))); it("should allow using lockTables on all types of locking", () => Promise.all(connections.map(async connection => { @@ -602,7 +639,7 @@ describe("query builder > locking", () => { }))); it("should allow locking a relation of a relation", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) + if (!(connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver)) return; return connection.manager.transaction(entityManager => { diff --git a/test/functional/repository/find-options-locking/find-options-locking.ts b/test/functional/repository/find-options-locking/find-options-locking.ts index 4a2d2a36a19..c1ecb1888b3 100644 --- a/test/functional/repository/find-options-locking/find-options-locking.ts +++ b/test/functional/repository/find-options-locking/find-options-locking.ts @@ -30,9 +30,18 @@ describe("repository > find options > locking", () => { after(() => closeTestingConnections(connections)); it("should throw error if pessimistic lock used without transaction", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof CockroachDriver || connection.driver instanceof SapDriver) + if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof SapDriver) return; + if (connection.driver instanceof CockroachDriver) { + return Promise.all([ + connection + .getRepository(PostWithVersion) + .findOne(1, { lock: { mode: "pessimistic_write" } }) + .should.be.rejectedWith(PessimisticLockTransactionRequiredError), + ]); + } + return Promise.all([ connection .getRepository(PostWithVersion) @@ -47,9 +56,20 @@ describe("repository > find options > locking", () => { }))); it("should not throw error if pessimistic lock used with transaction", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof CockroachDriver || connection.driver instanceof SapDriver) + if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof SapDriver) return; + if (connection.driver instanceof CockroachDriver) { + return connection.manager.transaction(entityManager => { + return Promise.all([ + entityManager + .getRepository(PostWithVersion) + .findOne(1, { lock: { mode: "pessimistic_write" } }) + .should.not.be.rejected + ]); + }); + } + return connection.manager.transaction(entityManager => { return Promise.all([ entityManager @@ -99,7 +119,7 @@ describe("repository > find options > locking", () => { }))); it("should attach pessimistic write lock statement on query if locking enabled", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof CockroachDriver || connection.driver instanceof SapDriver) + if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof SapDriver) return; const executedSql: string[] = []; @@ -251,7 +271,7 @@ describe("repository > find options > locking", () => { }))); it("should throw error if pessimistic locking not supported by given driver", () => Promise.all(connections.map(async connection => { - if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof CockroachDriver || connection.driver instanceof SapDriver) + if (connection.driver instanceof AbstractSqliteDriver || connection.driver instanceof SapDriver) return connection.manager.transaction(entityManager => { return Promise.all([ entityManager @@ -266,11 +286,21 @@ describe("repository > find options > locking", () => { ]); }); + if (connection.driver instanceof CockroachDriver) + return connection.manager.transaction(entityManager => { + return Promise.all([ + entityManager + .getRepository(PostWithVersion) + .findOne(1, { lock: { mode: "pessimistic_read" } }) + .should.be.rejectedWith(LockNotSupportedOnGivenDriverError), + ]); + }); + return; }))); it("should not allow empty array for lockTables", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) + if (!(connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver)) return; return connection.manager.transaction(entityManager => { @@ -284,7 +314,7 @@ describe("repository > find options > locking", () => { }))); it("should throw error when specifying a table that is not part of the query", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) + if (!(connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver)) return; return connection.manager.transaction(entityManager => { @@ -299,21 +329,35 @@ describe("repository > find options > locking", () => { }))); it("should allow on a left join", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) - return; + if (connection.driver instanceof CockroachDriver) { + return connection.manager.transaction(entityManager => { + return Promise.all([ + entityManager.getRepository(Post).findOne({ + relations: ['author'], + lock: {mode: 'pessimistic_write', tables: ['post']} + }), + entityManager.getRepository(Post).findOne({ + relations: ['author'], + lock: {mode: 'pessimistic_write'} + }) + ]); + }); + } - return connection.manager.transaction(entityManager => { - return Promise.all([ - entityManager.getRepository(Post).findOne({ - relations: ['author'], - lock: {mode: 'pessimistic_write', tables: ['post']} - }), - entityManager.getRepository(Post).findOne({ - relations: ['author'], - lock: {mode: 'pessimistic_write'} - }).should.be.rejectedWith('FOR UPDATE cannot be applied to the nullable side of an outer join') - ]); - }); + if (connection.driver instanceof PostgresDriver) { + return connection.manager.transaction(entityManager => { + return Promise.all([ + entityManager.getRepository(Post).findOne({ + relations: ['author'], + lock: {mode: 'pessimistic_write', tables: ['post']} + }), + entityManager.getRepository(Post).findOne({ + relations: ['author'], + lock: {mode: 'pessimistic_write'} + }).should.be.rejectedWith('FOR UPDATE cannot be applied to the nullable side of an outer join') + ]); + }); + } }))); it("should allow using lockTables on all types of locking", () => Promise.all(connections.map(async connection => { @@ -348,7 +392,7 @@ describe("repository > find options > locking", () => { }))); it("should allow locking a relation of a relation", () => Promise.all(connections.map(async connection => { - if (!(connection.driver instanceof PostgresDriver)) + if (!(connection.driver instanceof PostgresDriver || connection.driver instanceof CockroachDriver)) return; return connection.manager.transaction(entityManager => {