Skip to content

Commit

Permalink
fix(sql): pivot joining of m:n when no target entity needed directly
Browse files Browse the repository at this point in the history 
Closes #549
  • Loading branch information
@B4nan
B4nan committed Aug 9, 2020
1 parent 3b05a59 commit 2b0bb72
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 19 deletions.
10 changes: 5 additions & 5 deletions packages/core/src/EntityManager.ts
View file
Expand Up @@ -79,7 +79,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
*/
async find<T>(entityName: EntityName<T>, where: FilterQuery<T>, populate?: Populate<T> | FindOptions<T>, orderBy?: QueryOrderMap, limit?: number, offset?: number): Promise<T[]> {
entityName = Utils.className(entityName);
where = SmartQueryHelper.processWhere(where, entityName, this.metadata.get(entityName));
where = SmartQueryHelper.processWhere(where, entityName, this.metadata);
this.validator.validateParams(where);
const options = Utils.isObject<FindOptions<T>>(populate) ? populate : { populate, orderBy, limit, offset };
options.orderBy = options.orderBy || {};
Expand Down Expand Up @@ -146,7 +146,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
const options = Utils.isObject<FindOneOptions<T>>(populate) ? populate : { populate, orderBy };
const meta = this.metadata.get<T>(entityName);
this.validator.validateEmptyWhere(where);
where = SmartQueryHelper.processWhere(where as FilterQuery<T>, entityName, meta);
where = SmartQueryHelper.processWhere(where as FilterQuery<T>, entityName, this.metadata);
this.checkLockRequirements(options.lockMode, meta);
let entity = this.getUnitOfWork().tryGetById<T>(entityName, where);
const isOptimisticLocking = !Utils.isDefined(options.lockMode) || options.lockMode === LockMode.OPTIMISTIC;
Expand Down Expand Up @@ -240,7 +240,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
async nativeUpdate<T>(entityName: EntityName<T>, where: FilterQuery<T>, data: EntityData<T>): Promise<number> {
entityName = Utils.className(entityName);
data = SmartQueryHelper.processParams(data);
where = SmartQueryHelper.processWhere(where as FilterQuery<T>, entityName, this.metadata.get(entityName, false, false));
where = SmartQueryHelper.processWhere(where as FilterQuery<T>, entityName, this.metadata);
this.validator.validateParams(data, 'update data');
this.validator.validateParams(where, 'update condition');
const res = await this.driver.nativeUpdate(entityName, where, data, this.transactionContext);
Expand All @@ -253,7 +253,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
*/
async nativeDelete<T>(entityName: EntityName<T>, where: FilterQuery<T>): Promise<number> {
entityName = Utils.className(entityName);
where = SmartQueryHelper.processWhere(where as FilterQuery<T>, entityName, this.metadata.get(entityName, false, false));
where = SmartQueryHelper.processWhere(where as FilterQuery<T>, entityName, this.metadata);
this.validator.validateParams(where, 'delete condition');
const res = await this.driver.nativeDelete(entityName, where, this.transactionContext);

Expand Down Expand Up @@ -366,7 +366,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
*/
async count<T>(entityName: EntityName<T>, where: FilterQuery<T> = {}): Promise<number> {
entityName = Utils.className(entityName);
where = SmartQueryHelper.processWhere(where, entityName, this.metadata.get(entityName));
where = SmartQueryHelper.processWhere(where, entityName, this.metadata);
this.validator.validateParams(where);

return this.driver.count(entityName, where, this.transactionContext);
Expand Down
42 changes: 39 additions & 3 deletions packages/core/src/utils/SmartQueryHelper.ts
View file
Expand Up @@ -2,6 +2,7 @@ import { Reference, wrap } from '../entity';
import { Utils } from './Utils';
import { AnyEntity, Dictionary, EntityMetadata, FilterQuery } from '../typings';
import { GroupOperator } from '../enums';
import { MetadataStorage } from '../metadata';

export class SmartQueryHelper {

Expand Down Expand Up @@ -33,12 +34,47 @@ export class SmartQueryHelper {
return params;
}

static processWhere<T extends AnyEntity<T>>(where: FilterQuery<T>, entityName: string, meta?: EntityMetadata<T>): FilterQuery<T> {
static inlinePrimaryKeyObjects<T extends AnyEntity<T>>(where: Dictionary, meta: EntityMetadata<T>, metadata: MetadataStorage, key?: string): boolean {
if (Array.isArray(where)) {
where.forEach((item, i) => {
if (this.inlinePrimaryKeyObjects(item, meta, metadata, key)) {
where[i] = Utils.getPrimaryKeyValues(item, meta.primaryKeys, false);
}
});
}

if (!Utils.isPlainObject(where)) {
return false;
}

if (meta.primaryKeys.every(pk => pk in where) && Object.keys(where).length === meta.primaryKeys.length) {
return !GroupOperator[key as string] && Object.keys(where).every(k => !Utils.isPlainObject(where[k]) || Object.keys(where[k]).every(v => !Utils.isOperator(v, false)));
}

Object.keys(where).forEach(k => {
const meta2 = metadata.get(meta.properties[k]?.type, false, false) || meta;

if (this.inlinePrimaryKeyObjects(where[k], meta2, metadata, k)) {
where[k] = Utils.getPrimaryKeyValues(where[k], meta2.primaryKeys, true);
}
});

return false;
}

static processWhere<T extends AnyEntity<T>>(where: FilterQuery<T>, entityName: string, metadata: MetadataStorage): FilterQuery<T> {
const meta = metadata.get(entityName, false, false);

// inline PK-only objects in M:N queries so we don't join the target entity when not needed
if (meta) {
SmartQueryHelper.inlinePrimaryKeyObjects(where as Dictionary, meta, metadata);
}

where = SmartQueryHelper.processParams(where, true) || {};

if (Array.isArray(where)) {
const rootPrimaryKey = meta ? Utils.getPrimaryKeyHash(meta.primaryKeys) : entityName;
return { [rootPrimaryKey]: { $in: (where as FilterQuery<T>[]).map(sub => SmartQueryHelper.processWhere(sub, entityName, meta)) } } as FilterQuery<T>;
return { [rootPrimaryKey]: { $in: (where as FilterQuery<T>[]).map(sub => SmartQueryHelper.processWhere(sub, entityName, metadata)) } } as FilterQuery<T>;
}

if (!Utils.isPlainObject(where) || Utils.isPrimaryKey(where)) {
Expand All @@ -50,7 +86,7 @@ export class SmartQueryHelper {
const composite = meta?.properties[key]?.joinColumns?.length > 1;

if (key in GroupOperator) {
o[key] = value.map((sub: any) => SmartQueryHelper.processWhere(sub, entityName, meta));
o[key] = value.map((sub: any) => SmartQueryHelper.processWhere(sub, entityName, metadata));
return o;
}

Expand Down
5 changes: 3 additions & 2 deletions packages/knex/src/query/QueryBuilder.ts
View file
Expand Up @@ -104,7 +104,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
where(cond: QBFilterQuery<T>, operator?: keyof typeof GroupOperator): this;
where(cond: string, params?: any[], operator?: keyof typeof GroupOperator): this;
where(cond: QBFilterQuery<T> | string, params?: keyof typeof GroupOperator | any[], operator?: keyof typeof GroupOperator): this {
cond = SmartQueryHelper.processWhere(cond as Dictionary, this.entityName, this.metadata.get(this.entityName, false, false))!;
cond = SmartQueryHelper.processWhere(cond as Dictionary, this.entityName, this.metadata)!;

if (Utils.isString(cond)) {
cond = { [`(${cond})`]: Utils.asArray(params) };
Expand Down Expand Up @@ -146,6 +146,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
}

orderBy(orderBy: QueryOrderMap): this {
orderBy = SmartQueryHelper.processWhere(orderBy as Dictionary, this.entityName, this.metadata) as QueryOrderMap;
this._orderBy = CriteriaNode.create(this.metadata, this.entityName, orderBy).process(this);
return this;
}
Expand Down Expand Up @@ -336,7 +337,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
const entityName = this._aliasMap[fromAlias];
const prop = this.metadata.get(entityName).properties[fromField];
this._aliasMap[alias] = prop.type;
cond = SmartQueryHelper.processWhere(cond, this.entityName, this.metadata.get(this.entityName))!;
cond = SmartQueryHelper.processWhere(cond, this.entityName, this.metadata)!;
const aliasedName = `${fromAlias}.${prop.name}`;

if (prop.reference === ReferenceType.ONE_TO_MANY) {
Expand Down
40 changes: 39 additions & 1 deletion tests/QueryBuilder.test.ts
View file
Expand Up @@ -77,7 +77,7 @@ describe('QueryBuilder', () => {
test('select query with auto-joined composite key entity', async () => {
const qb1 = orm.em.createQueryBuilder(CarOwner2);
qb1.select('*').where({ car: { name: 'Audi A8', year: 2010 } });
expect(qb1.getQuery()).toEqual('select `e0`.* from `car_owner2` as `e0` left join `car2` as `e1` on `e0`.`car_name` = `e1`.`name` and `e0`.`car_year` = `e1`.`year` where `e1`.`name` = ? and `e1`.`year` = ?');
expect(qb1.getQuery()).toEqual('select `e0`.* from `car_owner2` as `e0` where (`e0`.`car_name`, `e0`.`car_year`) = (?, ?)');
expect(qb1.getParams()).toEqual(['Audi A8', 2010]);

const qb2 = orm.em.createQueryBuilder(CarOwner2);
Expand Down Expand Up @@ -1328,6 +1328,44 @@ describe('QueryBuilder', () => {
expect(qb4.getParams()).toEqual([]);
});

test('pivot joining of m:n when no target entity needed directly (GH issue 549)', async () => {
const qb1 = await orm.em.createQueryBuilder(Book2, 'b').select('b.*').where({ tags: { id: 1 } });
expect(qb1.getQuery()).toMatch('select `b`.*, `b`.price * 1.19 as `price_taxed` ' +
'from `book2` as `b` ' +
'left join `book2_tags` as `e1` on `b`.`uuid_pk` = `e1`.`book2_uuid_pk` ' +
'where `e1`.`book_tag2_id` = ?');
expect(qb1.getParams()).toEqual([1]);

const qb11 = await orm.em.createQueryBuilder(User2, 'u').select('u.*').where({ cars: { name: 'n', year: 1 } });
expect(qb11.getQuery()).toMatch('select `u`.* ' +
'from `user2` as `u` ' +
'left join `user2_cars` as `e1` on `u`.`first_name` = `e1`.`user2_first_name` and `u`.`last_name` = `e1`.`user2_last_name` ' +
'where (`e1`.`car2_name`, `e1`.`car2_year`) = (?, ?)');
expect(qb11.getParams()).toEqual(['n', 1]);

const qb12 = await orm.em.createQueryBuilder(User2, 'u').select('u.*').where({ cars: { $in: [{ name: 'n', year: 1 }, { name: 'n', year: 2 }] } });
expect(qb12.getQuery()).toMatch('select `u`.* ' +
'from `user2` as `u` ' +
'left join `user2_cars` as `e1` on `u`.`first_name` = `e1`.`user2_first_name` and `u`.`last_name` = `e1`.`user2_last_name` ' +
'where (`e1`.`car2_name`, `e1`.`car2_year`) in ((?, ?), (?, ?))');
expect(qb12.getParams()).toEqual(['n', 1, 'n', 2]);

const qb2 = await orm.em.createQueryBuilder(Book2, 'b').select('b.*').where({ $or: [{ tags: { id: null } }, { tags: { $ne: 1 } }] });
expect(qb2.getQuery()).toMatch('select `b`.*, `b`.price * 1.19 as `price_taxed` ' +
'from `book2` as `b` ' +
'left join `book2_tags` as `e1` on `b`.`uuid_pk` = `e1`.`book2_uuid_pk` ' +
'where (`e1`.`book_tag2_id` is null or `e1`.`book_tag2_id` != ?)');
expect(qb2.getParams()).toEqual([1]);

const qb4 = await orm.em.createQueryBuilder(Author2, 'a').select('a.*').where({ friends: 1 }).orderBy({ friends: { id: QueryOrder.ASC } });
expect(qb4.getQuery()).toMatch('select `a`.* ' +
'from `author2` as `a` ' +
'left join `author_to_friend` as `e1` on `a`.`id` = `e1`.`author2_1_id` ' +
'where `e1`.`author2_2_id` = ? ' +
'order by `e1`.`author2_2_id` asc');
expect(qb4.getParams()).toEqual([1]);
});

afterAll(async () => orm.close(true));

});
16 changes: 8 additions & 8 deletions tests/SmartQueryHelper.test.ts
View file
Expand Up @@ -18,7 +18,7 @@ describe('SmartQueryHelper', () => {
'key4<=': 123,
'key5!=': 123,
'key6!': 123,
}, 'id')).toEqual({
}, 'id', orm.getMetadata())).toEqual({
key1: { $gt: 123 },
key2: { $lt: 123 },
key3: { $gte: 123 },
Expand All @@ -33,7 +33,7 @@ describe('SmartQueryHelper', () => {
'key4 <=': 123,
'key5 !=': 123,
'key6 !': 123,
}, 'id')).toEqual({
}, 'id', orm.getMetadata())).toEqual({
key1: { $gt: 123 },
key2: { $lt: 123 },
key3: { $gte: 123 },
Expand All @@ -53,7 +53,7 @@ describe('SmartQueryHelper', () => {
'key6:not': 123,
'key7:in': [123],
'key8:nin': [123],
}, 'id')).toEqual({
}, 'id', orm.getMetadata())).toEqual({
key1: { $gt: 123 },
key2: { $lt: 123 },
key3: { $gte: 123 },
Expand All @@ -66,7 +66,7 @@ describe('SmartQueryHelper', () => {
});

test('processWhere returns empty object for undefined condition', async () => {
expect(SmartQueryHelper.processWhere(undefined as any, 'id')).toEqual({});
expect(SmartQueryHelper.processWhere(undefined as any, 'id', orm.getMetadata())).toEqual({});
});

test('test entity conversion to PK', async () => {
Expand Down Expand Up @@ -102,10 +102,10 @@ describe('SmartQueryHelper', () => {
const book1 = new Book2('b1', author);
const book2 = new Book2('b2', author);
const book3 = new Book2('b3', author);
expect(SmartQueryHelper.processWhere<Author2>([1, 2, 3], 'uuid')).toEqual({ uuid: { $in: [1, 2, 3] } });
expect(SmartQueryHelper.processWhere<Book2>([book1, book2, book3], 'uuid')).toEqual({ uuid: { $in: [book1.uuid, book2.uuid, book3.uuid] } });
expect(SmartQueryHelper.processWhere<Author2>({ favouriteBook: ['1', '2', '3'] }, 'id')).toEqual({ favouriteBook: { $in: ['1', '2', '3'] } });
expect(SmartQueryHelper.processWhere<Book2>({ $or: [{ author: [1, 2, 3] }, { author: [7, 8, 9] }] }, 'id')).toEqual({
expect(SmartQueryHelper.processWhere<Author2>([1, 2, 3], 'uuid', orm.getMetadata())).toEqual({ uuid: { $in: [1, 2, 3] } });
expect(SmartQueryHelper.processWhere<Book2>([book1, book2, book3], 'uuid', orm.getMetadata())).toEqual({ uuid: { $in: [book1.uuid, book2.uuid, book3.uuid] } });
expect(SmartQueryHelper.processWhere<Author2>({ favouriteBook: ['1', '2', '3'] }, 'id', orm.getMetadata())).toEqual({ favouriteBook: { $in: ['1', '2', '3'] } });
expect(SmartQueryHelper.processWhere<Book2>({ $or: [{ author: [1, 2, 3] }, { author: [7, 8, 9] }] }, 'id', orm.getMetadata())).toEqual({
$or: [{ author: { $in: [1, 2, 3] } }, { author: { $in: [7, 8, 9] } }],
});
});
Expand Down

0 comments on commit 2b0bb72

Please sign in to comment.