Skip to content

Commit

Permalink
fix: add lock clause for MSSQL select, select with join clause
Browse files Browse the repository at this point in the history
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
  • Loading branch information
h16rkim committed Jan 5, 2022
1 parent cefddd9 commit 4d6d19f
Show file tree
Hide file tree
Showing 5 changed files with 264 additions and 21 deletions.
49 changes: 28 additions & 21 deletions src/query-builder/SelectQueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1437,21 +1437,6 @@ export class SelectQueryBuilder<Entity> extends QueryBuilder<Entity> 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) {
Expand All @@ -1473,7 +1458,7 @@ export class SelectQueryBuilder<Entity> extends QueryBuilder<Entity> 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.createSqlServerSelectLockExpression() + useIndex;
}

/**
Expand Down Expand Up @@ -1528,7 +1513,7 @@ export class SelectQueryBuilder<Entity> extends QueryBuilder<Entity> 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.createSqlServerSelectLockExpression() +
(joinAttr.condition ? " ON " + this.replacePropertyNames(joinAttr.condition) : "");
}

Expand All @@ -1541,7 +1526,7 @@ export class SelectQueryBuilder<Entity> extends QueryBuilder<Entity> 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.createSqlServerSelectLockExpression() + " ON " + this.replacePropertyNames(condition + appendedCondition);

} else if (relation.isOneToMany || relation.isOneToOneNotOwner) {

Expand All @@ -1555,7 +1540,7 @@ export class SelectQueryBuilder<Entity> extends QueryBuilder<Entity> 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.createSqlServerSelectLockExpression() + " ON " + this.replacePropertyNames(condition + appendedCondition);

} else { // means many-to-many
const junctionTableName = relation.junctionEntityMetadata!.tablePath;
Expand Down Expand Up @@ -1587,8 +1572,8 @@ export class SelectQueryBuilder<Entity> extends QueryBuilder<Entity> 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.createSqlServerSelectLockExpression() + " ON " + this.replacePropertyNames(junctionCondition) +
" " + joinAttr.direction + " JOIN " + this.getTableName(destinationTableName) + " " + this.escape(destinationTableAlias) + this.createSqlServerSelectLockExpression() + " ON " + this.replacePropertyNames(destinationCondition + appendedCondition);

}
});
Expand Down Expand Up @@ -1693,6 +1678,28 @@ export class SelectQueryBuilder<Entity> extends QueryBuilder<Entity> implements
return "";
}

/**
* Creates "LOCK" part of SELECT Query
*/
private createSqlServerSelectLockExpression(): string {
let lock = "";
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;
}
}

return lock;
}

/**
* Creates "LOCK" part of SQL query.
*/
Expand Down
19 changes: 19 additions & 0 deletions test/github-issues/4764/entity/AdminUser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { Column, Entity, PrimaryGeneratedColumn } from "../../../../src";

@Entity()
export class AdminUser {
@PrimaryGeneratedColumn()
id!: number;

@Column()
email!: number;

@Column()
scopes!: string;

@Column()
name!: string;

@Column()
unid!: number;
}
29 changes: 29 additions & 0 deletions test/github-issues/4764/entity/Cart.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { Column, Entity, OneToMany, PrimaryGeneratedColumn } from "../../../../src";
import { CartItems } from "./CartItems";

@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[];
}
30 changes: 30 additions & 0 deletions test/github-issues/4764/entity/CartItems.ts
Original file line number Diff line number Diff line change
@@ -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;
}
158 changes: 158 additions & 0 deletions test/github-issues/4764/issue-4764.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
import { expect } from "chai";
import "reflect-metadata";
import { Connection } from "../../../src/index";
import {
closeTestingConnections,
createTestingConnections,
reloadTestingDatabases,
} from "../../utils/test-utils";
import { Cart } from "./entity/Cart";

describe("mssql > add lock clause for MSSQL select, 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 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;
}
});

0 comments on commit 4d6d19f

Please sign in to comment.