diff --git a/src/query-builder/DeleteQueryBuilder.ts b/src/query-builder/DeleteQueryBuilder.ts index efd732028c..f46b1708a9 100644 --- a/src/query-builder/DeleteQueryBuilder.ts +++ b/src/query-builder/DeleteQueryBuilder.ts @@ -185,21 +185,21 @@ export class DeleteQueryBuilder extends QueryBuilder implements * Adds new AND WHERE with conditions for the given ids. */ whereInIds(ids: any|any[]): this { - return this.where(this.createWhereIdsExpression(ids)); + return this.where(this.getWhereInIdsCondition(ids)); } /** * Adds new AND WHERE with conditions for the given ids. */ andWhereInIds(ids: any|any[]): this { - return this.andWhere(this.createWhereIdsExpression(ids)); + return this.andWhere(this.getWhereInIdsCondition(ids)); } /** * Adds new OR WHERE with conditions for the given ids. */ orWhereInIds(ids: any|any[]): this { - return this.orWhere(this.createWhereIdsExpression(ids)); + return this.orWhere(this.getWhereInIdsCondition(ids)); } /** * Optional returning/output clause. diff --git a/src/query-builder/QueryBuilder.ts b/src/query-builder/QueryBuilder.ts index 45ab1adf2d..031bc17f83 100644 --- a/src/query-builder/QueryBuilder.ts +++ b/src/query-builder/QueryBuilder.ts @@ -813,44 +813,35 @@ export abstract class QueryBuilder { } /** - * Creates "WHERE" expression and variables for the given "ids". + * Creates "WHERE" condition for an in-ids condition. */ - protected createWhereIdsExpression(ids: any|any[]): string { + protected getWhereInIdsCondition(ids: any|any[]): ObjectLiteral | Brackets { const metadata = this.expressionMap.mainAlias!.metadata; const normalized = (Array.isArray(ids) ? ids : [ids]).map(id => metadata.ensureEntityIdMap(id)); // using in(...ids) for single primary key entities - if (!metadata.hasMultiplePrimaryKeys - && metadata.embeddeds.length === 0 - ) { + if (!metadata.hasMultiplePrimaryKeys) { const primaryColumn = metadata.primaryColumns[0]; // getEntityValue will try to transform `In`, it is a bug // todo: remove this transformer check after #2390 is fixed - if (!primaryColumn.transformer) { - return this.computeWhereParameter({ + // This also fails for embedded & relation, so until that is fixed skip it. + if (!primaryColumn.transformer && !primaryColumn.relationMetadata && !primaryColumn.embeddedMetadata) { + return { [primaryColumn.propertyName]: In( normalized.map(id => primaryColumn.getEntityValue(id, false)) ) - }); + }; } } - // create shortcuts for better readability - const alias = this.expressionMap.aliasNamePrefixingEnabled ? this.escape(this.expressionMap.mainAlias!.name) + "." : ""; - const whereStrings = normalized.map((id, index) => { - const whereSubStrings: string[] = []; - metadata.primaryColumns.forEach((primaryColumn, secondIndex) => { - const parameterName = this.createParameter(primaryColumn.getEntityValue(id, true)); - // whereSubStrings.push(alias + this.escape(primaryColumn.databaseName) + "=:id_" + index + "_" + secondIndex); - whereSubStrings.push(alias + this.escape(primaryColumn.databaseName) + " = " + parameterName); - }); - return whereSubStrings.join(" AND "); + return new Brackets((qb) => { + for (const data of normalized) { + qb.orWhere(new Brackets( + (qb) => qb.where(data) + )); + } }); - - return whereStrings.length > 1 - ? "(" + whereStrings.map(whereString => "(" + whereString + ")").join(" OR ") + ")" - : whereStrings[0]; } private findColumnsForPropertyPath(propertyPath: string): [ Alias, string[], ColumnMetadata[] ] { diff --git a/src/query-builder/SelectQueryBuilder.ts b/src/query-builder/SelectQueryBuilder.ts index cac6826f04..6bc6bbc878 100644 --- a/src/query-builder/SelectQueryBuilder.ts +++ b/src/query-builder/SelectQueryBuilder.ts @@ -755,7 +755,7 @@ export class SelectQueryBuilder extends QueryBuilder implements * for example [{ firstId: 1, secondId: 2 }, { firstId: 2, secondId: 3 }, ...] */ whereInIds(ids: any|any[]): this { - return this.where(this.createWhereIdsExpression(ids)); + return this.where(this.getWhereInIdsCondition(ids)); } /** @@ -767,7 +767,7 @@ export class SelectQueryBuilder extends QueryBuilder implements * for example [{ firstId: 1, secondId: 2 }, { firstId: 2, secondId: 3 }, ...] */ andWhereInIds(ids: any|any[]): this { - return this.andWhere(this.createWhereIdsExpression(ids)); + return this.andWhere(this.getWhereInIdsCondition(ids)); } /** @@ -779,7 +779,7 @@ export class SelectQueryBuilder extends QueryBuilder implements * for example [{ firstId: 1, secondId: 2 }, { firstId: 2, secondId: 3 }, ...] */ orWhereInIds(ids: any|any[]): this { - return this.orWhere(this.createWhereIdsExpression(ids)); + return this.orWhere(this.getWhereInIdsCondition(ids)); } /** diff --git a/src/query-builder/SoftDeleteQueryBuilder.ts b/src/query-builder/SoftDeleteQueryBuilder.ts index 09e25af7be..8e24f69adc 100644 --- a/src/query-builder/SoftDeleteQueryBuilder.ts +++ b/src/query-builder/SoftDeleteQueryBuilder.ts @@ -189,21 +189,21 @@ export class SoftDeleteQueryBuilder extends QueryBuilder impleme * Adds new AND WHERE with conditions for the given ids. */ whereInIds(ids: any|any[]): this { - return this.where(this.createWhereIdsExpression(ids)); + return this.where(this.getWhereInIdsCondition(ids)); } /** * Adds new AND WHERE with conditions for the given ids. */ andWhereInIds(ids: any|any[]): this { - return this.andWhere(this.createWhereIdsExpression(ids)); + return this.andWhere(this.getWhereInIdsCondition(ids)); } /** * Adds new OR WHERE with conditions for the given ids. */ orWhereInIds(ids: any|any[]): this { - return this.orWhere(this.createWhereIdsExpression(ids)); + return this.orWhere(this.getWhereInIdsCondition(ids)); } /** * Optional returning/output clause. diff --git a/src/query-builder/UpdateQueryBuilder.ts b/src/query-builder/UpdateQueryBuilder.ts index 811fd4e739..57fe57ea68 100644 --- a/src/query-builder/UpdateQueryBuilder.ts +++ b/src/query-builder/UpdateQueryBuilder.ts @@ -213,21 +213,21 @@ export class UpdateQueryBuilder extends QueryBuilder implements * Adds new AND WHERE with conditions for the given ids. */ whereInIds(ids: any|any[]): this { - return this.where(this.createWhereIdsExpression(ids)); + return this.where(this.getWhereInIdsCondition(ids)); } /** * Adds new AND WHERE with conditions for the given ids. */ andWhereInIds(ids: any|any[]): this { - return this.andWhere(this.createWhereIdsExpression(ids)); + return this.andWhere(this.getWhereInIdsCondition(ids)); } /** * Adds new OR WHERE with conditions for the given ids. */ orWhereInIds(ids: any|any[]): this { - return this.orWhere(this.createWhereIdsExpression(ids)); + return this.orWhere(this.getWhereInIdsCondition(ids)); } /** * Optional returning/output clause. diff --git a/test/functional/query-builder/select/entity/ExternalPost.ts b/test/functional/query-builder/select/entity/ExternalPost.ts new file mode 100644 index 0000000000..a49a3e146a --- /dev/null +++ b/test/functional/query-builder/select/entity/ExternalPost.ts @@ -0,0 +1,17 @@ +import { + Entity, + Column, + PrimaryColumn, +} from "../../../../../src"; + +@Entity() +export class ExternalPost { + @PrimaryColumn() + outlet: string; + + @PrimaryColumn() + id: number; + + @Column() + title: string; +} diff --git a/test/functional/query-builder/select/query-builder-select.ts b/test/functional/query-builder/select/query-builder-select.ts index a46800a9d9..56b3c32cf6 100644 --- a/test/functional/query-builder/select/query-builder-select.ts +++ b/test/functional/query-builder/select/query-builder-select.ts @@ -7,11 +7,12 @@ import { Category } from "./entity/Category"; import {Post} from "./entity/Post"; import { Tag } from "./entity/Tag"; import { HeroImage } from "./entity/HeroImage"; +import { ExternalPost } from "./entity/ExternalPost"; describe("query builder > select", () => { let connections: Connection[]; before(async () => connections = await createTestingConnections({ - entities: [Category, Post, Tag, HeroImage], + entities: [Category, Post, Tag, HeroImage, ExternalPost], enabledDrivers: ["sqlite"], })); beforeEach(() => reloadTestingDatabases(connections)); @@ -390,6 +391,51 @@ describe("query builder > select", () => { }) + describe("where-in-ids", () => { + it("should create expected query with simple primary keys", () => Promise.all(connections.map(async connection => { + const [sql, params] = connection.createQueryBuilder(Post, "post") + .select("post.id") + .whereInIds([ 1, 2, 5, 9 ]) + .disableEscaping() + .getQueryAndParameters(); + + expect(sql).to.equal("SELECT post.id AS post_id FROM post post WHERE post.id IN (?, ?, ?, ?)") + expect(params).to.eql([ 1, 2, 5, 9 ]) + }))) + + it("should create expected query with composite primary keys", () => Promise.all(connections.map(async connection => { + const [sql, params] = connection.createQueryBuilder(ExternalPost, "post") + .select("post.id") + .whereInIds([ { outlet: "foo", id: 1 }, { outlet: "bar", id: 2 }, { outlet: "baz", id: 5 } ]) + .disableEscaping() + .getQueryAndParameters(); + + 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 = ?))' + ) + expect(params).to.eql([ "foo", 1, "bar", 2, "baz", 5 ]) + }))) + + it("should create expected query with composite primary keys with missing value", () => Promise.all(connections.map(async connection => { + const [sql, params] = connection.createQueryBuilder(ExternalPost, "post") + .select("post.id") + .whereInIds([ { outlet: "foo", id: 1 }, { outlet: "bar", id: 2 }, { id: 5 } ]) + .disableEscaping() + .getQueryAndParameters(); + + 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 = ?))' + ) + expect(params).to.eql([ "foo", 1, "bar", 2, 5 ]) + }))) + }) + it("Support max execution time", () => Promise.all(connections.map(async connection => { // MAX_EXECUTION_TIME supports only in MySQL if (!(connection.driver instanceof MysqlDriver)) return