Skip to content

Commit

Permalink
fix(core): rework pivot table joining (#4438)
Browse files Browse the repository at this point in the history
Closes #4423
  • Loading branch information
B4nan committed Nov 5, 2023
1 parent 9162418 commit 0506d36
Show file tree
Hide file tree
Showing 16 changed files with 222 additions and 152 deletions.
18 changes: 1 addition & 17 deletions packages/core/src/drivers/DatabaseDriver.ts
Expand Up @@ -27,7 +27,7 @@ import type {
import type { MetadataStorage } from '../metadata';
import type { Connection, QueryResult, Transaction } from '../connections';
import { EntityComparator, Utils, type Configuration, type ConnectionOptions, Cursor } from '../utils';
import { QueryOrder, ReferenceKind, type QueryOrderMap, type QueryOrderKeys, QueryOrderNumeric } from '../enums';
import { type QueryOrder, ReferenceKind, type QueryOrderKeys, QueryOrderNumeric } from '../enums';
import type { Platform } from '../platforms';
import type { Collection } from '../entity/Collection';
import { EntityManager } from '../EntityManager';
Expand Down Expand Up @@ -315,22 +315,6 @@ export abstract class DatabaseDriver<C extends Connection> implements IDatabaseD
});
}

protected getPivotOrderBy<T>(prop: EntityProperty<T>, orderBy?: OrderDefinition<T>): QueryOrderMap<T>[] {
if (!Utils.isEmpty(orderBy)) {
return orderBy as QueryOrderMap<T>[];
}

if (!Utils.isEmpty(prop.orderBy)) {
return Utils.asArray(prop.orderBy);
}

if (prop.fixedOrder) {
return [{ [`${prop.pivotEntity}.${prop.fixedOrderColumn}`]: QueryOrder.ASC } as QueryOrderMap<T>];
}

return [];
}

protected getPrimaryKeyFields(entityName: string): string[] {
const meta = this.metadata.find(entityName);
return meta ? Utils.flatten(meta.getPrimaryProps().map(pk => pk.fieldNames)) : [this.config.getNamingStrategy().referenceColumnName()];
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/entity/BaseEntity.ts
Expand Up @@ -2,8 +2,7 @@ import { Reference, type Ref } from './Reference';
import type { AutoPath, EntityData, EntityDTO, Loaded, LoadedReference, AddEager, EntityKey } from '../typings';
import { EntityAssigner, type AssignOptions } from './EntityAssigner';
import type { EntityLoaderOptions } from './EntityLoader';
import type { SerializeOptions } from '../serialization/EntitySerializer';
import { EntitySerializer } from '../serialization/EntitySerializer';
import { EntitySerializer, type SerializeOptions } from '../serialization/EntitySerializer';
import { helper } from './wrap';

export abstract class BaseEntity {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/entity/EntityValidator.ts
Expand Up @@ -130,7 +130,7 @@ export class EntityValidator {
}

private validateCollection<T extends object>(entity: T, prop: EntityProperty): void {
if (helper(entity).__initialized && !entity[prop.name as keyof T]) {
if (prop.hydrate !== false && helper(entity).__initialized && !entity[prop.name as keyof T]) {
throw ValidationError.fromCollectionNotInitialized(entity, prop);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/logging/DefaultLogger.ts
Expand Up @@ -79,7 +79,7 @@ export class DefaultLogger implements Logger {

if (context.took != null) {
if (context.results != null) {
msg += colors.grey(` [took ${context.took} ms, ${context.results} result${context.results > 1 ? 's' : ''}]`);
msg += colors.grey(` [took ${context.took} ms, ${context.results} result${context.results === 0 || context.results > 1 ? 's' : ''}]`);
} else {
msg += colors.grey(` [took ${context.took} ms]`);
}
Expand Down
20 changes: 20 additions & 0 deletions packages/core/src/metadata/MetadataDiscovery.ts
Expand Up @@ -595,6 +595,26 @@ export class MetadataDiscovery {
private definePivotTableEntity(meta: EntityMetadata, prop: EntityProperty): EntityMetadata {
const pivotMeta = this.metadata.find(prop.pivotEntity);

// ensure inverse side exists so we can join it when populating via pivot tables
if (!prop.inversedBy && prop.targetMeta) {
const inverseName = `${meta.className}_${prop.name}__inverse`;
prop.inversedBy = inverseName;
const inverseProp = {
name: inverseName,
kind: ReferenceKind.MANY_TO_MANY,
type: meta.className,
mappedBy: prop.name,
pivotEntity: prop.pivotEntity,
pivotTable: prop.pivotTable,
persist: false,
hydrate: false,
} as EntityProperty;
this.applyNamingStrategy(prop.targetMeta, inverseProp);
this.initCustomType(prop.targetMeta, inverseProp);
this.initRelation(inverseProp);
prop.targetMeta!.properties[inverseName] = inverseProp;
}

if (pivotMeta) {
this.ensureCorrectFKOrderInPivotEntity(pivotMeta, prop);
return pivotMeta;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils/Utils.ts
Expand Up @@ -656,7 +656,7 @@ export class Utils {
* Checks whether given object is a scalar reference.
*/
static isScalarReference<T = unknown>(data: any, allowReference = false): data is ScalarReference<any> & {} {
return typeof data === 'object' && data != null && data.__scalarReference;
return typeof data === 'object' && data?.__scalarReference;
}

/**
Expand Down
63 changes: 46 additions & 17 deletions packages/knex/src/AbstractSqlDriver.ts
Expand Up @@ -49,6 +49,7 @@ import {
type UpsertOptions,
Utils,
type OrderDefinition,
QueryOrder,
} from '@mikro-orm/core';
import type { AbstractSqlConnection } from './AbstractSqlConnection';
import type { AbstractSqlPlatform } from './AbstractSqlPlatform';
Expand Down Expand Up @@ -703,9 +704,20 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection

override async loadFromPivotTable<T extends object, O extends object>(prop: EntityProperty, owners: Primary<O>[][], where: FilterQuery<any> = {} as FilterQuery<any>, orderBy?: OrderDefinition<T>, ctx?: Transaction, options?: FindOptions<T, any, any>): Promise<Dictionary<T[]>> {
const pivotProp2 = this.getPivotInverseProperty(prop);
const prop2 = prop.targetMeta!.properties[prop.mappedBy ?? prop.inversedBy];
const ownerMeta = this.metadata.find(pivotProp2.type)!;
const pivotMeta = this.metadata.find(prop.pivotEntity)!;
const cond = { [`${prop.pivotEntity}.${pivotProp2.name}`]: { $in: ownerMeta.compositePK ? owners : owners.map(o => o[0]) } };
options = { ...options };
const qb = this.createQueryBuilder<T>(prop.type, ctx, options.connectionType)
.withSchema(this.getSchemaName(prop.targetMeta, options))
.indexHint(options.indexHint!)
.comment(options.comments!)
.hintComment(options.hintComments!);
const pivotAlias = qb.getNextAlias(pivotMeta.tableName);
const pivotKey = pivotProp2.joinColumns.map(column => `${pivotAlias}.${column}`).join(Utils.PK_SEPARATOR);
const cond = {
[pivotKey]: { $in: ownerMeta.compositePK ? owners : owners.map(o => o[0]) },
};

/* istanbul ignore if */
if (!Utils.isEmpty(where) && Object.keys(where as Dictionary).every(k => Utils.isOperator(k, false))) {
Expand All @@ -714,19 +726,23 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
where = { ...(where as Dictionary), ...cond } as FilterQuery<T>;
}

/* istanbul ignore if */
options = { ...options };

orderBy = this.getPivotOrderBy(prop, orderBy);
const qb = this.createQueryBuilder<T>(prop.type, ctx, options.connectionType)
.indexHint(options.indexHint!)
.comment(options.comments!)
.hintComment(options.hintComments!)
.unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES)
.withSchema(this.getSchemaName(prop.targetMeta, options));
orderBy = this.getPivotOrderBy(prop, pivotAlias, orderBy);
// FIXME is this fine?
const populate = this.autoJoinOneToOneOwner(prop.targetMeta!, [{ field: prop.pivotEntity }]);
const fields = this.buildFields(prop.targetMeta!, (options.populate ?? []) as unknown as PopulateOptions<T>[], [], qb, options.fields as unknown as Field<T>[]);
qb.select(fields).populate(populate).where(where).orderBy(orderBy!).setLockMode(options.lockMode, options.lockTableAliases);
const k1 = !prop.owner ? 'joinColumns' : 'inverseJoinColumns';
const k2 = prop.owner ? 'joinColumns' : 'inverseJoinColumns';
const cols = [
...prop[k1].map(col => `${pivotAlias}.${col} as fk__${col}`),
...prop[k2].map(col => `${pivotAlias}.${col} as fk__${col}`),
];
fields.push(...cols as string[]);
qb.select(fields)
.join(prop2.name, pivotAlias, {}, 'pivotJoin', undefined, options.schema)
.populate(populate)
.where(where)
.orderBy(orderBy!)
.setLockMode(options.lockMode, options.lockTableAliases);

if (owners.length === 1 && (options.offset != null || options.limit != null)) {
qb.limit(options.limit, options.offset);
Expand Down Expand Up @@ -766,6 +782,22 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
return map;
}

private getPivotOrderBy<T>(prop: EntityProperty<T>, pivotAlias: string, orderBy?: OrderDefinition<T>): QueryOrderMap<T>[] {
if (!Utils.isEmpty(orderBy)) {
return orderBy as QueryOrderMap<T>[];
}

if (!Utils.isEmpty(prop.orderBy)) {
return Utils.asArray(prop.orderBy);
}

if (prop.fixedOrder) {
return [{ [`${pivotAlias}.${prop.fixedOrderColumn}`]: QueryOrder.ASC } as QueryOrderMap<T>];
}

return [];
}

async execute<T extends QueryResult | EntityData<AnyEntity> | EntityData<AnyEntity>[] = EntityData<AnyEntity>[]>(queryOrKnex: string | Knex.QueryBuilder | Knex.Raw, params: any[] = [], method: 'all' | 'get' | 'run' = 'all', ctx?: Transaction): Promise<T> {
return this.rethrow(this.connection.execute(queryOrKnex, params, method, ctx));
}
Expand Down Expand Up @@ -982,9 +1014,7 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
const pivotMeta = this.metadata.find(prop.pivotEntity)!;

if (deleteDiff === true || deleteDiff.length > 0) {
const qb1 = this.createQueryBuilder(prop.pivotEntity, options?.ctx, 'write')
.withSchema(this.getSchemaName(pivotMeta, options))
.unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES);
const qb1 = this.createQueryBuilder(prop.pivotEntity, options?.ctx, 'write').withSchema(this.getSchemaName(pivotMeta, options));
const knex = qb1.getKnex();

if (Array.isArray(deleteDiff)) {
Expand Down Expand Up @@ -1017,7 +1047,6 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection
} else {
await Utils.runSerial(items, item => {
return this.createQueryBuilder(prop.pivotEntity, options?.ctx, 'write')
.unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES)
.withSchema(this.getSchemaName(pivotMeta, options))
.insert(item)
.execute('run', false);
Expand All @@ -1027,7 +1056,7 @@ export abstract class AbstractSqlDriver<Connection extends AbstractSqlConnection

override async lockPessimistic<T extends object>(entity: T, options: LockOptions): Promise<void> {
const meta = helper(entity).__meta;
const qb = this.createQueryBuilder((entity as object).constructor.name, options.ctx).unsetFlag(QueryFlag.CONVERT_CUSTOM_TYPES).withSchema(options.schema ?? meta.schema);
const qb = this.createQueryBuilder((entity as object).constructor.name, options.ctx).withSchema(options.schema ?? meta.schema);
const cond = Utils.getPrimaryKeyCond(entity, meta.primaryKeys);
qb.select(raw('1')).where(cond!).setLockMode(options.lockMode, options.lockTableAliases);
await this.rethrow(qb.execute());
Expand Down
52 changes: 21 additions & 31 deletions packages/knex/src/query/QueryBuilder.ts
Expand Up @@ -209,21 +209,21 @@ export class QueryBuilder<T extends object = AnyEntity> {
return this.init(QueryType.COUNT) as CountQueryBuilder<T>;
}

join(field: string, alias: string, cond: QBFilterQuery = {}, type: 'leftJoin' | 'innerJoin' | 'pivotJoin' = 'innerJoin', path?: string): this {
this.joinReference(field, alias, cond, type, path);
join(field: string, alias: string, cond: QBFilterQuery = {}, type: 'leftJoin' | 'innerJoin' | 'pivotJoin' = 'innerJoin', path?: string, schema?: string): this {
this.joinReference(field, alias, cond, type, path, schema);
return this;
}

leftJoin(field: string, alias: string, cond: QBFilterQuery = {}): this {
return this.join(field, alias, cond, 'leftJoin');
leftJoin(field: string, alias: string, cond: QBFilterQuery = {}, schema?: string): this {
return this.join(field, alias, cond, 'leftJoin', undefined, schema);
}

joinAndSelect(field: string, alias: string, cond: QBFilterQuery = {}, type: 'leftJoin' | 'innerJoin' | 'pivotJoin' = 'innerJoin', path?: string, fields?: string[]): SelectQueryBuilder<T> {
joinAndSelect(field: string, alias: string, cond: QBFilterQuery = {}, type: 'leftJoin' | 'innerJoin' | 'pivotJoin' = 'innerJoin', path?: string, fields?: string[], schema?: string): SelectQueryBuilder<T> {
if (!this.type) {
this.select('*');
}

const prop = this.joinReference(field, alias, cond, type, path);
const prop = this.joinReference(field, alias, cond, type, path, schema);
this.addSelect(this.getFieldsForJoinedLoad(prop, alias, fields));
const [fromAlias] = this.helper.splitField(field as EntityKey<T>);
const populate = this._joinedProps.get(fromAlias);
Expand All @@ -240,12 +240,12 @@ export class QueryBuilder<T extends object = AnyEntity> {
return this as SelectQueryBuilder<T>;
}

leftJoinAndSelect(field: string, alias: string, cond: QBFilterQuery = {}, fields?: string[]): SelectQueryBuilder<T> {
return this.joinAndSelect(field, alias, cond, 'leftJoin', undefined, fields);
leftJoinAndSelect(field: string, alias: string, cond: QBFilterQuery = {}, fields?: string[], schema?: string): SelectQueryBuilder<T> {
return this.joinAndSelect(field, alias, cond, 'leftJoin', undefined, fields, schema);
}

innerJoinAndSelect(field: string, alias: string, cond: QBFilterQuery = {}, fields?: string[]): SelectQueryBuilder<T> {
return this.joinAndSelect(field, alias, cond, 'innerJoin', undefined, fields);
innerJoinAndSelect(field: string, alias: string, cond: QBFilterQuery = {}, fields?: string[], schema?: string): SelectQueryBuilder<T> {
return this.joinAndSelect(field, alias, cond, 'innerJoin', undefined, fields, schema);
}

protected getFieldsForJoinedLoad(prop: EntityProperty<T>, alias: string, explicitFields?: string[]): Field<T>[] {
Expand Down Expand Up @@ -852,7 +852,7 @@ export class QueryBuilder<T extends object = AnyEntity> {
return qb;
}

private joinReference(field: string, alias: string, cond: Dictionary, type: 'leftJoin' | 'innerJoin' | 'pivotJoin', path?: string): EntityProperty<T> {
private joinReference(field: string, alias: string, cond: Dictionary, type: 'leftJoin' | 'innerJoin' | 'pivotJoin', path?: string, schema?: string): EntityProperty<T> {
this.ensureNotFinalized();
const [fromAlias, fromField] = this.helper.splitField(field as EntityKey<T>);
const q = (str: string) => `'${str}'`;
Expand Down Expand Up @@ -882,7 +882,7 @@ export class QueryBuilder<T extends object = AnyEntity> {
path ??= `${(Object.values(this._joins).find(j => j.alias === fromAlias)?.path ?? entityName)}.${prop.name}`;

if (prop.kind === ReferenceKind.ONE_TO_MANY) {
this._joins[aliasedName] = this.helper.joinOneToReference(prop, fromAlias, alias, type, cond);
this._joins[aliasedName] = this.helper.joinOneToReference(prop, fromAlias, alias, type, cond, schema);
} else if (prop.kind === ReferenceKind.MANY_TO_MANY) {
let pivotAlias = alias;

Expand All @@ -892,13 +892,14 @@ export class QueryBuilder<T extends object = AnyEntity> {
aliasedName = `${fromAlias}.${prop.name}#${pivotAlias}`;
}

const joins = this.helper.joinManyToManyReference(prop, fromAlias, alias, pivotAlias, type, cond, path);
const joins = this.helper.joinManyToManyReference(prop, fromAlias, alias, pivotAlias, type, cond, path, schema);

Object.assign(this._joins, joins);
this.createAlias(prop.pivotEntity, pivotAlias);
} else if (prop.kind === ReferenceKind.ONE_TO_ONE) {
this._joins[aliasedName] = this.helper.joinOneToReference(prop, fromAlias, alias, type, cond);
this._joins[aliasedName] = this.helper.joinOneToReference(prop, fromAlias, alias, type, cond, schema);
} else { // MANY_TO_ONE
this._joins[aliasedName] = this.helper.joinManyToOneReference(prop, fromAlias, alias, type, cond);
this._joins[aliasedName] = this.helper.joinManyToOneReference(prop, fromAlias, alias, type, cond, schema);
}

if (!this._joins[aliasedName].path && path) {
Expand Down Expand Up @@ -1136,9 +1137,7 @@ export class QueryBuilder<T extends object = AnyEntity> {
return;
}

if (this.metadata.find(field)?.pivotTable) { // pivot table entity
this.autoJoinPivotTable(field);
} else if (meta && this.helper.isOneToOneInverse(fromField)) {
if (meta && this.helper.isOneToOneInverse(fromField)) {
const prop = meta.properties[fromField as EntityKey<T>];
const alias = this.getNextAlias(prop.pivotEntity ?? prop.type);
const aliasedName = `${fromAlias}.${prop.name}#${alias}`;
Expand Down Expand Up @@ -1310,19 +1309,6 @@ export class QueryBuilder<T extends object = AnyEntity> {
});
}

private autoJoinPivotTable(field: string): void {
const pivotMeta = this.metadata.find(field)!;
const owner = pivotMeta.relations[0];
const inverse = pivotMeta.relations[1];
const prop = this._cond[pivotMeta.name + '.' + owner.name] || (this._orderBy as Dictionary)[pivotMeta.name + '.' + owner.name] ? inverse : owner;
const pivotAlias = this.getNextAlias(pivotMeta.name!);

this._joins[field] = this.helper.joinPivotTable(field, prop, this.mainAlias.aliasName, pivotAlias, 'leftJoin');
Utils.renameKey(this._cond, `${field}.${owner.name}`, Utils.getPrimaryKeyHash(owner.fieldNames.map(fieldName => `${pivotAlias}.${fieldName}`)), true);
Utils.renameKey(this._cond, `${field}.${inverse.name}`, Utils.getPrimaryKeyHash(inverse.fieldNames.map(fieldName => `${pivotAlias}.${fieldName}`)), true);
this._populateMap[field] = this._joins[field].alias;
}

private getSchema(alias: Alias<any>): string | undefined {
const { metadata } = alias;
const metaSchema = metadata?.schema && metadata.schema !== '*' ? metadata.schema : undefined;
Expand Down Expand Up @@ -1386,6 +1372,10 @@ export class QueryBuilder<T extends object = AnyEntity> {
object.data = this._data;
}

if (this._schema) {
object.schema = this._schema;
}

if (!Utils.isEmpty(this._cond)) {
object.where = this._cond;
}
Expand Down

0 comments on commit 0506d36

Please sign in to comment.