Skip to content

Commit

Permalink
fix: handle brackets when only one condition is passed (#8048)
Browse files Browse the repository at this point in the history
  • Loading branch information
imnotjames committed Aug 6, 2021
1 parent f0e40f6 commit ab39066
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 12 deletions.
13 changes: 10 additions & 3 deletions src/query-builder/QueryBuilder.ts
Expand Up @@ -819,7 +819,7 @@ export abstract class QueryBuilder<Entity> {
/**
* Computes given where argument - transforms to a where string all forms it can take.
*/
protected createWhereConditionExpression(condition: WhereClauseCondition): string {
protected createWhereConditionExpression(condition: WhereClauseCondition, alwaysWrap: boolean = false): string {
if (typeof condition === "string")
return condition;

Expand All @@ -828,7 +828,9 @@ export abstract class QueryBuilder<Entity> {
return "1=1";
}

if (condition.length === 1) {
// In the future we should probably remove this entire condition
// but for now to prevent any breaking changes it exists.
if (condition.length === 1 && !alwaysWrap) {
return this.createWhereClausesExpression(condition);
}

Expand Down Expand Up @@ -872,6 +874,8 @@ export abstract class QueryBuilder<Entity> {

case "not":
return `NOT(${this.createWhereConditionExpression(condition.condition)})`;
case "brackets":
return `${this.createWhereConditionExpression(condition.condition, true)}`;
}

throw new TypeError(`Unsupported FindOperator ${FindOperator.constructor.name}`);
Expand Down Expand Up @@ -1184,7 +1188,10 @@ export abstract class QueryBuilder<Entity> {

where.whereFactory(whereQueryBuilder as any);

return whereQueryBuilder.expressionMap.wheres;
return {
operator: "brackets",
condition: whereQueryBuilder.expressionMap.wheres
};
}

if (where instanceof Function) {
Expand Down
2 changes: 1 addition & 1 deletion src/query-builder/WhereClause.ts
@@ -1,4 +1,4 @@
type WrappingOperator = "not";
type WrappingOperator = "not" | "brackets";

type PredicateOperator = "lessThan" | "lessThanOrEqual" | "moreThan" | "moreThanOrEqual" | "equal" | "notEqual" | "ilike" | "like" | "between" | "in" | "any" | "isNull";

Expand Down
34 changes: 32 additions & 2 deletions test/functional/query-builder/brackets/query-builder-brackets.ts
Expand Up @@ -6,14 +6,44 @@ import {User} from "./entity/User";
import {Brackets} from "../../../../src/query-builder/Brackets";

describe("query builder > brackets", () => {

let connections: Connection[];
before(async () => connections = await createTestingConnections({
entities: [__dirname + "/entity/*{.js,.ts}"],
enabledDrivers: [ "sqlite" ],
}));
beforeEach(() => reloadTestingDatabases(connections));
after(() => closeTestingConnections(connections));

it("should put parentheses in the SQL", () => Promise.all(connections.map(async connection => {
const sql = await connection.createQueryBuilder(User, "user")
.where("user.isAdmin = :isAdmin", { isAdmin: true })
.orWhere(new Brackets(qb => {
qb.where("user.firstName = :firstName1", { firstName1: "Hello" })
.andWhere("user.lastName = :lastName1", { lastName1: "Mars" });
}))
.orWhere(new Brackets(qb => {
qb.where("user.firstName = :firstName2", { firstName2: "Hello" })
.andWhere("user.lastName = :lastName2", { lastName2: "Earth" });
}))
.andWhere(new Brackets(qb => {
qb.where("user.firstName = :firstName3 AND foo = bar", { firstName3: "Hello" })

}))
.disableEscaping()
.getSql()

expect(sql).to.be.equal(
"SELECT user.id AS user_id, user.firstName AS user_firstName, " +
"user.lastName AS user_lastName, user.isAdmin AS user_isAdmin " +
"FROM user user " +
"WHERE user.isAdmin = ? " +
"OR (user.firstName = ? AND user.lastName = ?) " +
"OR (user.firstName = ? AND user.lastName = ?) " +
"AND (user.firstName = ? AND foo = bar)"
)
})));

it("should put brackets correctly into WHERE expression", () => Promise.all(connections.map(async connection => {

const user1 = new User();
Expand Down Expand Up @@ -50,4 +80,4 @@ describe("query builder > brackets", () => {

})));

});
});
12 changes: 6 additions & 6 deletions test/functional/query-builder/select/query-builder-select.ts
Expand Up @@ -412,9 +412,9 @@ describe("query builder > select", () => {

expect(sql).to.equal(
'SELECT post.id AS post_id FROM external_post post WHERE ' +
'((post.outlet = ? AND post.id = ?) OR ' +
'(post.outlet = ? AND post.id = ?) OR ' +
'(post.outlet = ? AND post.id = ?))'
'(((post.outlet = ? AND post.id = ?)) OR ' +
'((post.outlet = ? AND post.id = ?)) OR ' +
'((post.outlet = ? AND post.id = ?)))'
)
expect(params).to.eql([ "foo", 1, "bar", 2, "baz", 5 ])
})))
Expand All @@ -428,9 +428,9 @@ describe("query builder > select", () => {

expect(sql).to.equal(
'SELECT post.id AS post_id FROM external_post post WHERE ' +
'((post.outlet = ? AND post.id = ?) OR ' +
'(post.outlet = ? AND post.id = ?) OR ' +
'post.id = ?)'
'(((post.outlet = ? AND post.id = ?)) OR ' +
'((post.outlet = ? AND post.id = ?)) OR ' +
'(post.id = ?))'
)
expect(params).to.eql([ "foo", 1, "bar", 2, 5 ])
})))
Expand Down

0 comments on commit ab39066

Please sign in to comment.