From 43f32984f76a27603db88f5e8544c27b801709f6 Mon Sep 17 00:00:00 2001 From: JeyongOh Date: Wed, 29 Apr 2020 20:35:42 +0900 Subject: [PATCH 1/4] [Add] FOR NO KEY UPDATE lock mode for postgresql --- src/query-builder/QueryExpressionMap.ts | 2 +- src/query-builder/SelectQueryBuilder.ts | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/query-builder/QueryExpressionMap.ts b/src/query-builder/QueryExpressionMap.ts index b88310dcaa..30f19c5d18 100644 --- a/src/query-builder/QueryExpressionMap.ts +++ b/src/query-builder/QueryExpressionMap.ts @@ -150,7 +150,7 @@ export class QueryExpressionMap { /** * Locking mode. */ - lockMode?: "optimistic"|"pessimistic_read"|"pessimistic_write"|"dirty_read"; + lockMode?: "optimistic"|"pessimistic_read"|"pessimistic_write"|"dirty_read"|"for_no_key_update"; /** * Current version of the entity, used for locking. diff --git a/src/query-builder/SelectQueryBuilder.ts b/src/query-builder/SelectQueryBuilder.ts index 483b2dced8..93aa8c5f4b 100644 --- a/src/query-builder/SelectQueryBuilder.ts +++ b/src/query-builder/SelectQueryBuilder.ts @@ -967,12 +967,12 @@ export class SelectQueryBuilder extends QueryBuilder implements /** * Sets locking mode. */ - setLock(lockMode: "pessimistic_read"|"pessimistic_write"|"dirty_read"): this; + setLock(lockMode: "pessimistic_read"|"pessimistic_write"|"dirty_read"|"for_no_key_update"): this; /** * Sets locking mode. */ - setLock(lockMode: "optimistic"|"pessimistic_read"|"pessimistic_write"|"dirty_read", lockVersion?: number|Date): this { + setLock(lockMode: "optimistic"|"pessimistic_read"|"pessimistic_write"|"dirty_read"|"for_no_key_update", lockVersion?: number|Date): this { this.expressionMap.lockMode = lockMode; this.expressionMap.lockVersion = lockVersion; return this; @@ -1672,6 +1672,12 @@ export class SelectQueryBuilder extends QueryBuilder implements } else { throw new LockNotSupportedOnGivenDriverError(); } + case "for_no_key_update": + if (driver instanceof PostgresDriver) { + return " FOR NO KEY UPDATE" + } else { + throw new LockNotSupportedOnGivenDriverError(); + } default: return ""; } @@ -1806,7 +1812,7 @@ export class SelectQueryBuilder extends QueryBuilder implements if (!this.expressionMap.mainAlias) throw new Error(`Alias is not set. Use "from" method to set an alias.`); - if ((this.expressionMap.lockMode === "pessimistic_read" || this.expressionMap.lockMode === "pessimistic_write") && !queryRunner.isTransactionActive) + if ((this.expressionMap.lockMode === "pessimistic_read" || this.expressionMap.lockMode === "pessimistic_write" || this.expressionMap.lockMode === "for_no_key_update") && !queryRunner.isTransactionActive) throw new PessimisticLockTransactionRequiredError(); if (this.expressionMap.lockMode === "optimistic") { From 143299b49b008e213b686f04bd2a2eed227c0551 Mon Sep 17 00:00:00 2001 From: JeyongOh Date: Wed, 29 Apr 2020 21:14:36 +0900 Subject: [PATCH 2/4] [Add] for no key update lock test --- .../locking/query-builder-locking.ts | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test/functional/query-builder/locking/query-builder-locking.ts b/test/functional/query-builder/locking/query-builder-locking.ts index eefdddfd05..a5279c4805 100644 --- a/test/functional/query-builder/locking/query-builder-locking.ts +++ b/test/functional/query-builder/locking/query-builder-locking.ts @@ -77,6 +77,28 @@ 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) { + return connection.createQueryBuilder(PostWithVersion, "post") + .setLock("for_no_key_update") + .where("post.id = :id", { id: 1 }) + .getOne().should.be.rejectedWith(PessimisticLockTransactionRequiredError) + } + return; + }))); + + 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) { + return connection.manager.transaction(entityManager => { + return Promise.all([entityManager.createQueryBuilder(PostWithVersion, "post") + .setLock("for_no_key_update") + .where("post.id = :id", { id: 1}) + .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) return; @@ -141,6 +163,30 @@ 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) { + const sql = connection.createQueryBuilder(PostWithVersion, "post") + .where("post.id = :id", { id: 1 }) + .getSql(); + + expect(sql.indexOf("FOR NO KEY UPDATE") === -1).to.be.true; + } + return; + }))); + + 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) { + const sql = connection.createQueryBuilder(PostWithVersion, "post") + .setLock("pessimistic_write") + .where("post.id = :id", { id: 1 }) + .getSql(); + + expect(sql.indexOf("FOR NO KEY UPDATE") !== -1).to.be.true; + } + return; + + }))); + it("should throw error if optimistic lock used with getMany method", () => Promise.all(connections.map(async connection => { return connection.createQueryBuilder(PostWithVersion, "post") @@ -291,4 +337,19 @@ describe("query builder > locking", () => { 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)) { + return connection.manager.transaction(entityManager => { + return Promise.all([ + entityManager.createQueryBuilder(PostWithVersion, "post") + .setLock("for_no_key_update") + .where("post.id = :id", { id: 1 }) + .getOne().should.be.rejectedWith(LockNotSupportedOnGivenDriverError), + ]); + }); + } + + return; + }))); + }); From 0861fc23db3e8610e1b92127b8d75fd93c53f1c8 Mon Sep 17 00:00:00 2001 From: JeyongOh Date: Wed, 29 Apr 2020 22:27:06 +0900 Subject: [PATCH 3/4] [Fix] lint --- src/query-builder/SelectQueryBuilder.ts | 2 +- .../query-builder/locking/query-builder-locking.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/query-builder/SelectQueryBuilder.ts b/src/query-builder/SelectQueryBuilder.ts index 93aa8c5f4b..572e6b30cc 100644 --- a/src/query-builder/SelectQueryBuilder.ts +++ b/src/query-builder/SelectQueryBuilder.ts @@ -1674,7 +1674,7 @@ export class SelectQueryBuilder extends QueryBuilder implements } case "for_no_key_update": if (driver instanceof PostgresDriver) { - return " FOR NO KEY UPDATE" + return " FOR NO KEY UPDATE"; } 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 a5279c4805..e8e7f98e88 100644 --- a/test/functional/query-builder/locking/query-builder-locking.ts +++ b/test/functional/query-builder/locking/query-builder-locking.ts @@ -82,19 +82,19 @@ describe("query builder > locking", () => { return connection.createQueryBuilder(PostWithVersion, "post") .setLock("for_no_key_update") .where("post.id = :id", { id: 1 }) - .getOne().should.be.rejectedWith(PessimisticLockTransactionRequiredError) + .getOne().should.be.rejectedWith(PessimisticLockTransactionRequiredError); } return; }))); 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) { return connection.manager.transaction(entityManager => { return Promise.all([entityManager.createQueryBuilder(PostWithVersion, "post") .setLock("for_no_key_update") .where("post.id = :id", { id: 1}) .getOne().should.not.be.rejected]); - }) + }); } return; }))); From f304144f129ddb799125f33d44a2d366e980797e Mon Sep 17 00:00:00 2001 From: JeyongOh Date: Thu, 30 Apr 2020 04:13:19 +0900 Subject: [PATCH 4/4] [Fix] test --- test/functional/query-builder/locking/query-builder-locking.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/query-builder/locking/query-builder-locking.ts b/test/functional/query-builder/locking/query-builder-locking.ts index e8e7f98e88..adcf742087 100644 --- a/test/functional/query-builder/locking/query-builder-locking.ts +++ b/test/functional/query-builder/locking/query-builder-locking.ts @@ -177,7 +177,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) { const sql = connection.createQueryBuilder(PostWithVersion, "post") - .setLock("pessimistic_write") + .setLock("for_no_key_update") .where("post.id = :id", { id: 1 }) .getSql();