Skip to content

Commit

Permalink
fix(core): do not alias JSON conditions on update/delete queries
Browse files Browse the repository at this point in the history
Closes #2839
  • Loading branch information
B4nan committed Mar 12, 2022
1 parent 67806cb commit 5c0674e
Show file tree
Hide file tree
Showing 16 changed files with 225 additions and 66 deletions.
9 changes: 7 additions & 2 deletions packages/better-sqlite/src/BetterSqlitePlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,14 @@ export class BetterSqlitePlatform extends AbstractSqlPlatform {
return escape(value, true, this.timezone);
}

getSearchJsonPropertyKey(path: string[], type: string): string {
getSearchJsonPropertyKey(path: string[], type: string, aliased: boolean): string {
const [a, ...b] = path;
return expr(alias => `json_extract(${this.quoteIdentifier(`${alias}.${a}`)}, '$.${b.join('.')}')`);

if (aliased) {
return expr(alias => `json_extract(${this.quoteIdentifier(`${alias}.${a}`)}, '$.${b.join('.')}')`);
}

return `json_extract(${this.quoteIdentifier(a)}, '$.${b.join('.')}')`;
}

getDefaultIntegrityRule(): string {
Expand Down
17 changes: 15 additions & 2 deletions packages/core/src/EntityManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,14 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
}

protected async processWhere<T extends AnyEntity<T>, P extends string = never>(entityName: string, where: FilterQuery<T>, options: FindOptions<T, P> | FindOneOptions<T, P>, type: 'read' | 'update' | 'delete'): Promise<FilterQuery<T>> {
where = QueryHelper.processWhere(where as FilterQuery<T>, entityName, this.metadata, this.driver.getPlatform(), options.convertCustomTypes);
where = QueryHelper.processWhere({
where: where as FilterQuery<T>,
entityName,
metadata: this.metadata,
platform: this.driver.getPlatform(),
convertCustomTypes: options.convertCustomTypes,
aliased: type === 'read',
});
where = await this.applyFilters(entityName, where, options.filters ?? {}, type);
where = await this.applyDiscriminatorCondition(entityName, where);

Expand Down Expand Up @@ -296,7 +303,13 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
cond = filter.cond;
}

ret.push(QueryHelper.processWhere(cond, entityName, this.metadata, this.driver.getPlatform()));
ret.push(QueryHelper.processWhere({
where: cond,
entityName,
metadata: this.metadata,
platform: this.driver.getPlatform(),
aliased: type === 'read',
}));
}

const conds = [...ret, where as Dictionary].filter(c => Utils.hasObjectKeys(c)) as FilterQuery<T>[];
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/decorators/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* istanbul ignore file */
export * from './PrimaryKey';
export * from './Entity';
export * from './OneToOne';
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/entity/EntityLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export class EntityLoader {
}

private mergePrimaryCondition<T>(ids: T[], pk: string, options: EntityLoaderOptions<T>, meta: EntityMetadata, metadata: MetadataStorage, platform: Platform): FilterQuery<T> {
const cond1 = QueryHelper.processWhere({ [pk]: { $in: ids } }, meta.name!, metadata, platform, !options.convertCustomTypes);
const cond1 = QueryHelper.processWhere({ where: { [pk]: { $in: ids } }, entityName: meta.name!, metadata, platform, convertCustomTypes: !options.convertCustomTypes });

return options.where![pk]
? { $and: [cond1, options.where] } as FilterQuery<any>
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/metadata/MetadataDiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ export class MetadataDiscovery {

path.push(prop.name);
meta.properties[name].fieldNames = [path.join('.')]; // store path for ObjectHydrator
meta.properties[name].fieldNameRaw = this.platform.getSearchJsonPropertySQL(path.join('->'), prop.type); // for querying in SQL drivers
meta.properties[name].fieldNameRaw = this.platform.getSearchJsonPropertySQL(path.join('->'), prop.type, true); // for querying in SQL drivers
meta.properties[name].persist = false; // only virtual as we store the whole object
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/platforms/Platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,11 @@ export abstract class Platform {
return 'json';
}

getSearchJsonPropertySQL(path: string, type: string): string {
getSearchJsonPropertySQL(path: string, type: string, aliased: boolean): string {
return path;
}

getSearchJsonPropertyKey(path: string[], type: string): string {
getSearchJsonPropertyKey(path: string[], type: string, aliased: boolean): string {
return path.join('.');
}

Expand Down
39 changes: 28 additions & 11 deletions packages/core/src/utils/QueryHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,12 @@ export class QueryHelper {
return false;
}

static processWhere<T>(where: FilterQuery<T>, entityName: string, metadata: MetadataStorage, platform: Platform, convertCustomTypes = true, root = true): FilterQuery<T> {
static processWhere<T>(options: ProcessWhereOptions<T>): FilterQuery<T> {
// eslint-disable-next-line prefer-const
let { where, entityName, metadata, platform, aliased = true, convertCustomTypes = true, root = true } = options;
const meta = metadata.find<T>(entityName);

// inline PK-only objects in M:N queries so we don't join the target entity when not needed
// inline PK-only objects in M:N queries, so we don't join the target entity when not needed
if (meta && root) {
QueryHelper.inlinePrimaryKeyObjects(where as Dictionary, meta, metadata);
}
Expand All @@ -107,7 +109,7 @@ export class QueryHelper {
const rootPrimaryKey = meta ? Utils.getPrimaryKeyHash(meta.primaryKeys) : entityName;
const cond = { [rootPrimaryKey]: { $in: where } } as ObjectQuery<T>;

return QueryHelper.processWhere(cond, entityName, metadata, platform, convertCustomTypes, false);
return QueryHelper.processWhere({ ...options, where: cond, root: false });
}

if (!Utils.isPlainObject(where)) {
Expand All @@ -121,23 +123,23 @@ export class QueryHelper {
const composite = keys > 1;

if (key in GroupOperator) {
o[key] = value.map((sub: any) => QueryHelper.processWhere<T>(sub, entityName, metadata, platform, convertCustomTypes, false));
o[key] = value.map((sub: any) => QueryHelper.processWhere<T>({ ...options, where: sub, root: false }));
return o;
}

// wrap top level operators (except $not) with PK
if (Utils.isOperator(key) && root && meta && key !== '$not') {
const rootPrimaryKey = Utils.getPrimaryKeyHash(meta.primaryKeys);
o[rootPrimaryKey] = { [key]: QueryHelper.processWhere<T>(value, entityName, metadata, platform, convertCustomTypes, false) };
o[rootPrimaryKey] = { [key]: QueryHelper.processWhere<T>({ ...options, where: value, root: false }) };
return o;
}

if (prop?.customType && convertCustomTypes && !platform.isRaw(value)) {
value = QueryHelper.processCustomType<T>(prop, value, platform, undefined, true);
}

if (prop?.customType instanceof JsonType && Utils.isPlainObject(value) && !platform.isRaw(value)) {
return this.processJsonCondition(o, value, [prop.fieldNames[0]], platform);
if (prop?.customType instanceof JsonType && Utils.isPlainObject(value) && !platform.isRaw(value) && Object.keys(value)[0] !== '$eq') {
return this.processJsonCondition(o, value, [prop.fieldNames[0]], platform, aliased);
}

if (Array.isArray(value) && !Utils.isOperator(key) && !QueryHelper.isSupportedOperator(key) && !key.includes('?')) {
Expand All @@ -160,7 +162,12 @@ export class QueryHelper {
const operatorExpression = new RegExp(re).exec(key);

if (Utils.isPlainObject(value)) {
o[key] = QueryHelper.processWhere(value as ObjectQuery<T>, prop?.type ?? entityName, metadata, platform, convertCustomTypes, false);
o[key] = QueryHelper.processWhere({
...options,
where: value as ObjectQuery<T>,
entityName: prop?.type ?? entityName,
root: false,
});
} else if (!QueryHelper.isSupportedOperator(key)) {
o[key] = value;
} else if (operatorExpression) {
Expand Down Expand Up @@ -248,25 +255,35 @@ export class QueryHelper {
return !!QueryHelper.SUPPORTED_OPERATORS.find(op => key.includes(op));
}

private static processJsonCondition<T>(o: ObjectQuery<T>, value: Dictionary, path: string[], platform: Platform) {
private static processJsonCondition<T>(o: ObjectQuery<T>, value: Dictionary, path: string[], platform: Platform, alias: boolean) {
if (Utils.isPlainObject(value) && !Object.keys(value).some(k => Utils.isOperator(k))) {
Object.keys(value).forEach(k => {
this.processJsonCondition(o, value[k], [...path, k], platform);
this.processJsonCondition(o, value[k], [...path, k], platform, alias);
});

return o;
}

const operatorObject = Utils.isPlainObject(value) && Object.keys(value).every(k => Utils.isOperator(k));
const type = operatorObject ? typeof Object.values(value)[0] : typeof value;
const k = platform.getSearchJsonPropertyKey(path, type);
const k = platform.getSearchJsonPropertyKey(path, type, alias);
o[k] = value;

return o;
}

}

interface ProcessWhereOptions<T> {
where: FilterQuery<T>;
entityName: string;
metadata: MetadataStorage;
platform: Platform;
aliased: boolean;
convertCustomTypes?: boolean;
root?: boolean;
}

/**
* Helper for escaping string types, e.g. `keyof T -> string`.
* We can also pass array of strings to allow tuple comparison in SQL drivers.
Expand Down
4 changes: 2 additions & 2 deletions packages/knex/src/AbstractSqlPlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ export abstract class AbstractSqlPlatform extends Platform {
return ret;
}

getSearchJsonPropertySQL(path: string, type: string): string {
return this.getSearchJsonPropertyKey(path.split('->'), type);
getSearchJsonPropertySQL(path: string, type: string, aliased: boolean): string {
return this.getSearchJsonPropertyKey(path.split('->'), type, aliased);
}

isRaw(value: any): boolean {
Expand Down
67 changes: 58 additions & 9 deletions packages/knex/src/query/QueryBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
import type { Knex } from 'knex';
import type {
AnyEntity, ConnectionType, Dictionary, EntityData, EntityMetadata, EntityProperty, FlatQueryOrderMap, RequiredEntityData,
GroupOperator, MetadataStorage, PopulateOptions, QBFilterQuery, QueryOrderMap, QueryResult, FlushMode, FilterQuery, QBQueryOrderMap,
AnyEntity,
ConnectionType,
Dictionary,
EntityData,
EntityMetadata,
EntityProperty,
FilterQuery,
FlatQueryOrderMap,
FlushMode,
GroupOperator,
MetadataStorage,
PopulateOptions,
QBFilterQuery,
QBQueryOrderMap,
QueryOrderMap,
QueryResult,
RequiredEntityData,
} from '@mikro-orm/core';
import { LoadStrategy, LockMode, QueryFlag, QueryHelper, ReferenceType, Utils, ValidationError } from '@mikro-orm/core';
import { QueryType } from './enums';
Expand Down Expand Up @@ -188,7 +203,14 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
cond = { [`(${cond})`]: Utils.asArray(params) };
operator = operator || '$and';
} else {
cond = QueryHelper.processWhere(cond, this.entityName, this.metadata, this.platform, this.flags.has(QueryFlag.CONVERT_CUSTOM_TYPES)) as FilterQuery<T>;
cond = QueryHelper.processWhere({
where: cond,
entityName: this.entityName,
metadata: this.metadata,
platform: this.platform,
aliased: !this.type || [QueryType.SELECT, QueryType.COUNT].includes(this.type),
convertCustomTypes: this.flags.has(QueryFlag.CONVERT_CUSTOM_TYPES),
}) as FilterQuery<T>;
}

const op = operator || params as keyof typeof GroupOperator;
Expand Down Expand Up @@ -233,7 +255,14 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
orderBy(orderBy: QBQueryOrderMap<T> | QBQueryOrderMap<T>[]): this {
this._orderBy = [];
Utils.asArray(orderBy).forEach(o => {
const processed = QueryHelper.processWhere(o as Dictionary, this.entityName, this.metadata, this.platform, false)!;
const processed = QueryHelper.processWhere({
where: o as Dictionary,
entityName: this.entityName,
metadata: this.metadata,
platform: this.platform,
aliased: !this.type || [QueryType.SELECT, QueryType.COUNT].includes(this.type),
convertCustomTypes: false,
})!;
this._orderBy.push(CriteriaNodeFactory.createNode(this.metadata, this.entityName, processed).process(this));
});

Expand Down Expand Up @@ -602,7 +631,13 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
}

this._aliasMap[alias] = prop.type;
cond = QueryHelper.processWhere(cond, this.entityName, this.metadata, this.platform)!;
cond = QueryHelper.processWhere({
where: cond,
entityName: this.entityName,
metadata: this.metadata,
platform: this.platform,
aliased: !this.type || [QueryType.SELECT, QueryType.COUNT].includes(this.type),
})!;
let aliasedName = `${fromAlias}.${prop.name}#${alias}`;
path ??= `${(Object.values(this._joins).find(j => j.alias === fromAlias)?.path ?? entityName)}.${prop.name}`;

Expand Down Expand Up @@ -894,30 +929,44 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {

export interface RunQueryBuilder<T> extends Omit<QueryBuilder<T>, 'getResult' | 'getSingleResult' | 'getResultList' | 'where'> {
where(cond: QBFilterQuery<T> | string, params?: keyof typeof GroupOperator | any[], operator?: keyof typeof GroupOperator): this;

execute<U = QueryResult<T>>(method?: 'all' | 'get' | 'run', mapResults?: boolean): Promise<U>;

then<TResult1 = QueryResult<T>, TResult2 = never>(onfulfilled?: ((value: QueryResult<T>) => TResult1 | PromiseLike<TResult1>) | undefined | null, onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null): Promise<QueryResult<T>>;
}

export interface SelectQueryBuilder<T> extends QueryBuilder<T> {
execute<U = T[]>(method?: 'all' | 'get' | 'run', mapResults?: boolean): Promise<U>;

execute<U = T[]>(method: 'all', mapResults?: boolean): Promise<U>;

execute<U = T>(method: 'get', mapResults?: boolean): Promise<U>;

execute<U = QueryResult<T>>(method: 'run', mapResults?: boolean): Promise<U>;

then<TResult1 = T[], TResult2 = never>(onfulfilled?: ((value: T[]) => TResult1 | PromiseLike<TResult1>) | undefined | null, onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null): Promise<T[]>;
}

export interface CountQueryBuilder<T> extends QueryBuilder<T> {
execute<U = { count: number }[]>(method?: 'all' | 'get' | 'run', mapResults?: boolean): Promise<U>;

execute<U = { count: number }[]>(method: 'all', mapResults?: boolean): Promise<U>;

execute<U = { count: number }>(method: 'get', mapResults?: boolean): Promise<U>;

execute<U = QueryResult<{ count: number }>>(method: 'run', mapResults?: boolean): Promise<U>;

then<TResult1 = number, TResult2 = never>(onfulfilled?: ((value: number) => TResult1 | PromiseLike<TResult1>) | undefined | null, onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null): Promise<number>;
}

export interface InsertQueryBuilder<T> extends RunQueryBuilder<T> {}
export interface InsertQueryBuilder<T> extends RunQueryBuilder<T> {
}

export interface UpdateQueryBuilder<T> extends RunQueryBuilder<T> {}
export interface UpdateQueryBuilder<T> extends RunQueryBuilder<T> {
}

export interface DeleteQueryBuilder<T> extends RunQueryBuilder<T> {}
export interface DeleteQueryBuilder<T> extends RunQueryBuilder<T> {
}

export interface TruncateQueryBuilder<T> extends RunQueryBuilder<T> {}
export interface TruncateQueryBuilder<T> extends RunQueryBuilder<T> {
}
9 changes: 7 additions & 2 deletions packages/mariadb/src/MariaDbPlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ export class MariaDbPlatform extends AbstractSqlPlatform {
}

/* istanbul ignore next */
getSearchJsonPropertyKey(path: string[], type: string): string {
getSearchJsonPropertyKey(path: string[], type: string, aliased: boolean): string {
const [a, ...b] = path;
return expr(alias => `${this.quoteIdentifier(`${alias}.${a}`)}->'$.${b.join('.')}'`);

if (aliased) {
return expr(alias => `${this.quoteIdentifier(`${alias}.${a}`)}->'$.${b.join('.')}'`);
}

return `${this.quoteIdentifier(a)}->'$.${b.join('.')}'`;
}

getBooleanTypeDeclarationSQL(): string {
Expand Down
9 changes: 7 additions & 2 deletions packages/mysql/src/MySqlPlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@ export class MySqlPlatform extends AbstractSqlPlatform {
return 'utf8mb4';
}

getSearchJsonPropertyKey(path: string[], type: string): string {
getSearchJsonPropertyKey(path: string[], type: string, aliased: boolean): string {
const [a, ...b] = path;
return expr(alias => `${this.quoteIdentifier(`${alias}.${a}`)}->'$.${b.join('.')}'`);

if (aliased) {
return expr(alias => `${this.quoteIdentifier(`${alias}.${a}`)}->'$.${b.join('.')}'`);
}

return `${this.quoteIdentifier(a)}->'$.${b.join('.')}'`;
}

getBooleanTypeDeclarationSQL(): string {
Expand Down
4 changes: 2 additions & 2 deletions packages/postgresql/src/PostgreSqlPlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ export class PostgreSqlPlatform extends AbstractSqlPlatform {
return 'jsonb';
}

getSearchJsonPropertyKey(path: string[], type: string): string {
getSearchJsonPropertyKey(path: string[], type: string, aliased: boolean): string {
const first = path.shift();
const last = path.pop();
const root = expr(alias => this.quoteIdentifier(`${alias}.${first}`));
const root = aliased ? expr(alias => this.quoteIdentifier(`${alias}.${first}`)) : this.quoteIdentifier(first!);
const types = {
number: 'float8',
boolean: 'bool',
Expand Down
9 changes: 7 additions & 2 deletions packages/sqlite/src/SqlitePlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,14 @@ export class SqlitePlatform extends AbstractSqlPlatform {
return escape(value, true, this.timezone);
}

getSearchJsonPropertyKey(path: string[], type: string): string {
getSearchJsonPropertyKey(path: string[], type: string, aliased: boolean): string {
const [a, ...b] = path;
return expr(alias => `json_extract(${this.quoteIdentifier(`${alias}.${a}`)}, '$.${b.join('.')}')`);

if (aliased) {
return expr(alias => `json_extract(${this.quoteIdentifier(`${alias}.${a}`)}, '$.${b.join('.')}')`);
}

return `json_extract(${this.quoteIdentifier(a)}, '$.${b.join('.')}')`;
}

getDefaultIntegrityRule(): string {
Expand Down
Loading

0 comments on commit 5c0674e

Please sign in to comment.