From 3284808b63552d81456752187c0d130db76007ed Mon Sep 17 00:00:00 2001 From: Heejae-Kim Date: Sat, 12 Feb 2022 20:07:59 +0900 Subject: [PATCH] feat: add WITH (lock) clause for MSSQL select with join queries (#8507) * fix: add lock clause for MSSQL select with join clause typeorm didn't supported LOCK clause in SELECT + JOIN query. For example, we cannot buld SQL such as "SELECT * FROM USER U WITH(NOLOCK) INNER JOIN ORDER WITH(NOLOCK) O ON U.ID=O.UserID". This pull request enables LOCK with SELECT + JOIN sql query. Closes: #4764 * chore: add test cases * chore: refactor method name, miscellaneous changes on createTableLockExpression --- src/query-builder/SelectQueryBuilder.ts | 50 +++-- test/github-issues/4764/entity/Cart.ts | 41 ++++ test/github-issues/4764/entity/CartItems.ts | 30 +++ test/github-issues/4764/entity/User.ts | 25 +++ test/github-issues/4764/issue-4764.ts | 208 ++++++++++++++++++++ 5 files changed, 333 insertions(+), 21 deletions(-) create mode 100644 test/github-issues/4764/entity/Cart.ts create mode 100644 test/github-issues/4764/entity/CartItems.ts create mode 100644 test/github-issues/4764/entity/User.ts create mode 100644 test/github-issues/4764/issue-4764.ts diff --git a/src/query-builder/SelectQueryBuilder.ts b/src/query-builder/SelectQueryBuilder.ts index f163733125..30d56a3284 100644 --- a/src/query-builder/SelectQueryBuilder.ts +++ b/src/query-builder/SelectQueryBuilder.ts @@ -1437,21 +1437,6 @@ export class SelectQueryBuilder extends QueryBuilder implements if (allSelects.length === 0) allSelects.push({ selection: "*" }); - let lock: string = ""; - if (this.connection.driver instanceof SqlServerDriver) { - switch (this.expressionMap.lockMode) { - case "pessimistic_read": - lock = " WITH (HOLDLOCK, ROWLOCK)"; - break; - case "pessimistic_write": - lock = " WITH (UPDLOCK, ROWLOCK)"; - break; - case "dirty_read": - lock = " WITH (NOLOCK)"; - break; - } - } - // Use certain index let useIndex: string = ""; if (this.expressionMap.useIndex) { @@ -1473,7 +1458,7 @@ export class SelectQueryBuilder extends QueryBuilder implements const select = this.createSelectDistinctExpression(); const selection = allSelects.map(select => select.selection + (select.aliasName ? " AS " + this.escape(select.aliasName) : "")).join(", "); - return select + selection + " FROM " + froms.join(", ") + lock + useIndex; + return select + selection + " FROM " + froms.join(", ") + this.createTableLockExpression() + useIndex; } /** @@ -1528,7 +1513,7 @@ export class SelectQueryBuilder extends QueryBuilder implements // table to join, without junction table involved. This means we simply join direct table. if (!parentAlias || !relation) { const destinationJoin = joinAttr.alias.subQuery ? joinAttr.alias.subQuery : this.getTableName(destinationTableName); - return " " + joinAttr.direction + " JOIN " + destinationJoin + " " + this.escape(destinationTableAlias) + + return " " + joinAttr.direction + " JOIN " + destinationJoin + " " + this.escape(destinationTableAlias) + this.createTableLockExpression() + (joinAttr.condition ? " ON " + this.replacePropertyNames(joinAttr.condition) : ""); } @@ -1541,7 +1526,7 @@ export class SelectQueryBuilder extends QueryBuilder implements parentAlias + "." + relation.propertyPath + "." + joinColumn.referencedColumn!.propertyPath; }).join(" AND "); - return " " + joinAttr.direction + " JOIN " + this.getTableName(destinationTableName) + " " + this.escape(destinationTableAlias) + " ON " + this.replacePropertyNames(condition + appendedCondition); + return " " + joinAttr.direction + " JOIN " + this.getTableName(destinationTableName) + " " + this.escape(destinationTableAlias) + this.createTableLockExpression() + " ON " + this.replacePropertyNames(condition + appendedCondition); } else if (relation.isOneToMany || relation.isOneToOneNotOwner) { @@ -1555,7 +1540,7 @@ export class SelectQueryBuilder extends QueryBuilder implements parentAlias + "." + joinColumn.referencedColumn!.propertyPath; }).join(" AND "); - return " " + joinAttr.direction + " JOIN " + this.getTableName(destinationTableName) + " " + this.escape(destinationTableAlias) + " ON " + this.replacePropertyNames(condition + appendedCondition); + return " " + joinAttr.direction + " JOIN " + this.getTableName(destinationTableName) + " " + this.escape(destinationTableAlias) + this.createTableLockExpression() + " ON " + this.replacePropertyNames(condition + appendedCondition); } else { // means many-to-many const junctionTableName = relation.junctionEntityMetadata!.tablePath; @@ -1587,8 +1572,8 @@ export class SelectQueryBuilder extends QueryBuilder implements }).join(" AND "); } - return " " + joinAttr.direction + " JOIN " + this.getTableName(junctionTableName) + " " + this.escape(junctionAlias) + " ON " + this.replacePropertyNames(junctionCondition) + - " " + joinAttr.direction + " JOIN " + this.getTableName(destinationTableName) + " " + this.escape(destinationTableAlias) + " ON " + this.replacePropertyNames(destinationCondition + appendedCondition); + return " " + joinAttr.direction + " JOIN " + this.getTableName(junctionTableName) + " " + this.escape(junctionAlias) + this.createTableLockExpression() + " ON " + this.replacePropertyNames(junctionCondition) + + " " + joinAttr.direction + " JOIN " + this.getTableName(destinationTableName) + " " + this.escape(destinationTableAlias) + this.createTableLockExpression() + " ON " + this.replacePropertyNames(destinationCondition + appendedCondition); } }); @@ -1693,6 +1678,29 @@ export class SelectQueryBuilder extends QueryBuilder implements return ""; } + /** + * Creates "LOCK" part of SELECT Query after table Clause + * ex. + * SELECT 1 + * FROM USER U WITH (NOLOCK) + * JOIN ORDER O WITH (NOLOCK) + * ON U.ID=O.OrderID + */ + private createTableLockExpression(): string { + if(this.connection.driver instanceof SqlServerDriver) { + switch (this.expressionMap.lockMode) { + case "pessimistic_read": + return " WITH (HOLDLOCK, ROWLOCK)"; + case "pessimistic_write": + return " WITH (UPDLOCK, ROWLOCK)"; + case "dirty_read": + return " WITH (NOLOCK)"; + } + } + + return ""; + } + /** * Creates "LOCK" part of SQL query. */ diff --git a/test/github-issues/4764/entity/Cart.ts b/test/github-issues/4764/entity/Cart.ts new file mode 100644 index 0000000000..a24709e42a --- /dev/null +++ b/test/github-issues/4764/entity/Cart.ts @@ -0,0 +1,41 @@ +import { + Column, + Entity, + JoinColumn, + OneToMany, + OneToOne, + PrimaryGeneratedColumn, +} from "../../../../src"; +import { CartItems } from "./CartItems"; +import { User } from "./User"; + +@Entity() +export class Cart { + @PrimaryGeneratedColumn() + ID!: number; + + @Column() + UNID!: number; + + @Column() + Type!: string; + + @Column() + Cycle?: number; + + @Column() + Term?: string; + + @Column() + RegDate!: Date; + + @Column() + ModifiedDate!: Date; + + @OneToMany((type) => CartItems, (t) => t.Cart) + CartItems?: CartItems[]; + + @OneToOne((type) => User, (t) => t.Cart) + @JoinColumn({ name: "UNID" }) + User?: User; +} diff --git a/test/github-issues/4764/entity/CartItems.ts b/test/github-issues/4764/entity/CartItems.ts new file mode 100644 index 0000000000..f7466fce44 --- /dev/null +++ b/test/github-issues/4764/entity/CartItems.ts @@ -0,0 +1,30 @@ +import { Column, Entity, JoinColumn, ManyToOne, PrimaryGeneratedColumn } from "../../../../src"; +import { Cart } from "./Cart"; + +@Entity() +export class CartItems { + @PrimaryGeneratedColumn() + ID!: number; + + @Column() + CartID!: number; + + @Column() + ItemID!: number; + + @Column() + OptionID!: number; + + @Column() + Quantity!: number; + + @Column() + RegDate!: Date; + + @Column() + ModifiedDate!: Date; + + @ManyToOne((type) => Cart, (t) => t.CartItems) + @JoinColumn({ name: "CartID" }) + Cart?: Cart; +} diff --git a/test/github-issues/4764/entity/User.ts b/test/github-issues/4764/entity/User.ts new file mode 100644 index 0000000000..4f55387648 --- /dev/null +++ b/test/github-issues/4764/entity/User.ts @@ -0,0 +1,25 @@ +import { + Column, + Entity, + OneToOne, + PrimaryGeneratedColumn, +} from "../../../../src"; +import { Cart } from "./Cart"; + +@Entity() +export class User { + @PrimaryGeneratedColumn() + ID!: number; + + @Column() + name!: string; + + @Column() + RegDate!: Date; + + @Column() + ModifiedDate!: Date; + + @OneToOne((type) => Cart, (t) => t.User) + Cart?: Cart; +} diff --git a/test/github-issues/4764/issue-4764.ts b/test/github-issues/4764/issue-4764.ts new file mode 100644 index 0000000000..030eae064f --- /dev/null +++ b/test/github-issues/4764/issue-4764.ts @@ -0,0 +1,208 @@ +import { expect } from "chai"; +import "reflect-metadata"; +import { Connection } from "../../../src/index"; +import { + closeTestingConnections, + createTestingConnections, + reloadTestingDatabases, +} from "../../utils/test-utils"; +import { User } from "./entity/User"; +import { Cart } from "./entity/Cart"; +import { CartItems } from "./entity/CartItems"; + +describe("mssql > add lock clause for MSSQL select with join clause", () => { + // ------------------------------------------------------------------------- + // Configuration + // ------------------------------------------------------------------------- + + // connect to db + let connections: Connection[]; + + before( + async () => + (connections = await createTestingConnections({ + enabledDrivers: ["mssql"], + entities: [__dirname + "/entity/*{.js,.ts}"], + schemaCreate: true, + dropSchema: true, + })) + ); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + // ------------------------------------------------------------------------- + // Specifications + // ------------------------------------------------------------------------- + it("should not have Lock clause", async () => { + Promise.all( + connections.map(async (connection) => { + const lock = " WITH (NOLOCK)"; + const selectQuery = connection + .createQueryBuilder() + .select("cart") + .from(Cart, "cart") + .where("1=1") + .getQuery(); + + console.log(selectQuery); + expect(selectQuery.includes(lock)).not.to.equal(true); + + await connection.query(selectQuery); + }) + ); + }); + + it("should have WITH (NOLOCK) clause", async () => { + Promise.all( + connections.map(async (connection) => { + const lock = " WITH (NOLOCK)"; + const selectQuery = connection + .createQueryBuilder() + .select("cart") + .from(Cart, "cart") + .setLock("dirty_read") + .where("1=1") + .getQuery(); + + console.log(selectQuery); + expect(selectQuery.includes(lock)).to.equal(true); + + await connection.query(selectQuery); + }) + ); + }); + + it("should have two WITH (NOLOCK) clause", async () => { + Promise.all( + connections.map(async (connection) => { + const lock = " WITH (NOLOCK)"; + const selectQuery = connection + .createQueryBuilder() + .select("cart") + .from(Cart, "cart") + .innerJoinAndSelect("cart.CartItems", "cartItems") + .setLock("dirty_read") + .where("1=1") + .getQuery(); + + console.log(selectQuery); + expect(countInstances(selectQuery, lock)).to.equal(2); + + await connection.query(selectQuery); + }) + ); + }); + + it("should have three WITH (NOLOCK) clause", async () => { + Promise.all( + connections.map(async (connection) => { + const lock = " WITH (NOLOCK)"; + const selectQuery = connection + .createQueryBuilder() + .select("cart") + .from(Cart, "cart") + .innerJoinAndSelect("cart.User", "user") + .innerJoinAndSelect("cart.CartItems", "cartItems") + .setLock("dirty_read") + .where("1=1") + .getQuery(); + + console.log(selectQuery); + expect(countInstances(selectQuery, lock)).to.equal(3); + + await connection.query(selectQuery); + }) + ); + }); + + it("should have three WITH (NOLOCK) clause (without relation)", async () => { + Promise.all( + connections.map(async (connection) => { + const lock = " WITH (NOLOCK)"; + const selectQuery = connection + .createQueryBuilder() + .select("cart") + .from(Cart, "cart") + .innerJoin(User, "user", "user.ID=cart.UNID") + .innerJoin( + CartItems, + "cartItems", + "cart.ID=cartItems.CartID" + ) + .setLock("dirty_read") + .where("cart.ID=1") + .getQuery(); + + console.log(selectQuery); + expect(countInstances(selectQuery, lock)).to.equal(3); + + await connection.query(selectQuery); + }) + ); + }); + + it("should have WITH (HOLDLOCK, ROWLOCK) clause", async () => { + Promise.all( + connections.map(async (connection) => { + const lock = " WITH (HOLDLOCK, ROWLOCK)"; + const selectQuery = connection + .createQueryBuilder() + .select("cart") + .from(Cart, "cart") + .setLock("pessimistic_read") + .where("1=1") + .getQuery(); + + console.log(selectQuery); + expect(selectQuery.includes(lock)).to.equal(true); + + await connection.query(selectQuery); + }) + ); + }); + + it("should have WITH (UPLOCK, ROWLOCK) clause", async () => { + Promise.all( + connections.map(async (connection) => { + const lock = " WITH (UPDLOCK, ROWLOCK)"; + const selectQuery = connection + .createQueryBuilder() + .select("cart") + .from(Cart, "cart") + .setLock("pessimistic_write") + .where("1=1") + .getQuery(); + + console.log(selectQuery); + expect(selectQuery.includes(lock)).to.equal(true); + + await connection.query(selectQuery); + }) + ); + }); + + it("should have two WITH (UPDLOCK, ROWLOCK) clause", async () => { + Promise.all( + connections.map(async (connection) => { + const lock = " WITH (UPDLOCK, ROWLOCK)"; + const selectQuery = connection + .createQueryBuilder() + .select("cart") + .from(Cart, "cart") + .innerJoinAndSelect("cart.CartItems", "cartItems") + .setLock("pessimistic_write") + .where("1=1") + .getQuery(); + + console.log(selectQuery); + expect(countInstances(selectQuery, lock)).to.equal(2); + + await connection.query(selectQuery); + }) + ); + }); + + function countInstances(str: string, word: string) { + return str.split(word).length - 1; + } +});