Skip to content

Commit

Permalink
fix(core): apply convertToJSValueSQL on composite FKs too
Browse files Browse the repository at this point in the history
Closes #4843
  • Loading branch information
B4nan committed Oct 21, 2023
1 parent d03528d commit 41425cb
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 14 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/entity/EntityLoader.ts
Expand Up @@ -237,7 +237,7 @@ export class EntityLoader {

if (pk) {
const key = helper(this.em.getReference(prop.type, pk)).getSerializedPrimaryKey();
map[key].push(child as T);
map[key]?.push(child as T);
}
});
} else {
Expand All @@ -246,7 +246,7 @@ export class EntityLoader {

if (entity) {
const key = helper(entity).getSerializedPrimaryKey();
map[key].push(child as T);
map[key]?.push(child as T);
}
});
}
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/metadata/MetadataDiscovery.ts
Expand Up @@ -1049,6 +1049,18 @@ export class MetadataDiscovery {
prop.type = prop.customType.constructor.name;
}

if (!prop.customType && [ReferenceType.ONE_TO_ONE, ReferenceType.MANY_TO_ONE].includes(prop.reference) && this.metadata.get(prop.type).compositePK) {
prop.customTypes = [];

for (const pk of this.metadata.get(prop.type).getPrimaryProps()) {
if (pk.customType) {
prop.customTypes.push(pk.customType);
prop.hasConvertToJSValueSQL ||= !!pk.customType.convertToJSValueSQL && pk.customType.convertToJSValueSQL('', this.platform) !== '';
prop.hasConvertToDatabaseValueSQL ||= !!pk.customType.convertToDatabaseValueSQL && pk.customType.convertToDatabaseValueSQL('', this.platform) !== '';
}
}
}

if (prop.reference === ReferenceType.SCALAR) {
const mappedType = this.getMappedType(prop);
prop.columnTypes ??= [mappedType.getColumnType(prop, this.platform)];
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/typings.ts
Expand Up @@ -261,6 +261,7 @@ export interface EntityProperty<T = any> {
targetMeta?: EntityMetadata;
columnTypes: string[];
customType: Type<any>;
customTypes: Type<any>[];
hasConvertToJSValueSQL: boolean;
hasConvertToDatabaseValueSQL: boolean;
autoincrement?: boolean;
Expand Down
45 changes: 33 additions & 12 deletions packages/knex/src/query/QueryBuilderHelper.ts
Expand Up @@ -50,8 +50,11 @@ export class QueryBuilderHelper {
for (const p of fields) {
const [a, f] = this.splitField(p);
const prop = this.getProperty(f, a);
const fkIdx2 = prop?.fieldNames.findIndex(name => name === f) ?? -1;

if (prop) {
if (fkIdx2 !== -1) {
parts.push(this.mapper(a !== this.alias ? `${a}.${prop!.fieldNames[fkIdx2]}` : prop!.fieldNames[fkIdx2], type, value, alias));
} else if (prop) {
parts.push(...prop.fieldNames.map(f => this.mapper(a !== this.alias ? `${a}.${f}` : f, type, value, alias)));
} else {
parts.push(this.mapper(a !== this.alias ? `${a}.${f}` : f, type, value, alias));
Expand All @@ -77,10 +80,12 @@ export class QueryBuilderHelper {
const customExpression = QueryBuilderHelper.isCustomExpression(field, !!alias);
const [a, f] = this.splitField(field);
const prop = this.getProperty(f, a);
const fkIdx2 = prop?.fieldNames.findIndex(name => name === f) ?? -1;
const fkIdx = fkIdx2 === -1 ? 0 : fkIdx2;

// embeddable nested path instead of a regular property with table alias, reset alias
if (prop?.name === a && prop.embeddedProps[f]) {
return this.alias + '.' + prop.fieldNames[0];
return this.alias + '.' + prop.fieldNames[fkIdx];
}

const noPrefix = prop && prop.persist === false;
Expand All @@ -98,19 +103,27 @@ export class QueryBuilderHelper {
}

if (prop?.hasConvertToJSValueSQL) {
const prefixed = this.prefix(field, isTableNameAliasRequired, true);
const valueSQL = prop.customType.convertToJSValueSQL!(prefixed, this.platform);
let valueSQL: string;

if (prop.fieldNames.length > 1 && fkIdx !== -1) {
const fk = prop.targetMeta!.getPrimaryProps()[fkIdx];
const prefixed = this.prefix(field, isTableNameAliasRequired, true, fkIdx);
valueSQL = fk.customType.convertToJSValueSQL!(prefixed, this.platform);
} else {
const prefixed = this.prefix(field, isTableNameAliasRequired, true);
valueSQL = prop.customType.convertToJSValueSQL!(prefixed, this.platform);
}

if (alias === null) {
return this.knex.raw(valueSQL);
}

return this.knex.raw(`${valueSQL} as ${this.platform.quoteIdentifier(alias ?? prop.fieldNames[0])}`);
return this.knex.raw(`${valueSQL} as ${this.platform.quoteIdentifier(alias ?? prop.fieldNames[fkIdx])}`);
}

// do not wrap custom expressions
if (!customExpression) {
ret = this.prefix(field);
ret = this.prefix(field, false, false, fkIdx);
}

if (alias) {
Expand Down Expand Up @@ -688,12 +701,12 @@ export class QueryBuilderHelper {
return !!field.match(re);
}

private prefix(field: string, always = false, quote = false): string {
private prefix(field: string, always = false, quote = false, idx?: number): string {
let ret: string;

if (!this.isPrefixed(field)) {
const alias = always ? (quote ? this.alias : this.platform.quoteIdentifier(this.alias)) + '.' : '';
const fieldName = this.fieldName(field, this.alias, always);
const fieldName = this.fieldName(field, this.alias, always, idx);

if (QueryBuilderHelper.isCustomExpression(fieldName)) {
return fieldName;
Expand All @@ -703,7 +716,7 @@ export class QueryBuilderHelper {
} else {
const [a, ...rest] = field.split('.');
const f = rest.join('.');
ret = a + '.' + this.fieldName(f, a);
ret = a + '.' + this.fieldName(f, a, always, idx);
}

if (quote) {
Expand Down Expand Up @@ -737,7 +750,7 @@ export class QueryBuilderHelper {
return !!field.match(/[\w`"[\]]+\./);
}

private fieldName(field: string, alias?: string, always?: boolean): string {
private fieldName(field: string, alias?: string, always?: boolean, idx = 0): string {
const prop = this.getProperty(field, alias);

if (!prop) {
Expand All @@ -760,7 +773,7 @@ export class QueryBuilderHelper {
}

/* istanbul ignore next */
return prop.fieldNames?.[0] ?? field;
return prop.fieldNames?.[idx] ?? field;
}

getProperty(field: string, alias?: string): EntityProperty | undefined {
Expand All @@ -783,7 +796,15 @@ export class QueryBuilderHelper {
}
}

return meta?.properties[field];
if (meta) {
if (meta.properties[field]) {
return meta.properties[field];
}

return meta.relations.find(prop => prop.fieldNames?.some(name => field === name));
}

return undefined;
}

isTableNameAliasRequired(type?: QueryType): boolean {
Expand Down
111 changes: 111 additions & 0 deletions tests/issues/GH4843.test.ts
@@ -0,0 +1,111 @@
import {
Collection,
Entity,
ManyToOne,
PrimaryKey,
OneToMany,
TextType,
PrimaryKeyType,
} from '@mikro-orm/core';
import { MikroORM } from '@mikro-orm/postgresql';

class CitextType extends TextType {

getColumnType() {
return 'citext';
}

convertToJSValueSQL(key: string): string {
return `lower(${key})`;
}

}

@Entity()
class A {

[PrimaryKeyType]?: [string, string];

@PrimaryKey({ type: CitextType })
id1!: string;

@PrimaryKey({ type: CitextType })
id2!: string;

@OneToMany(() => C, c => c.a)
c = new Collection<C>(this);

}

@Entity()
class B {

@PrimaryKey({ type: CitextType })
id!: string;

@OneToMany(() => C, c => c.b)
c = new Collection<C>(this);

}

@Entity()
class C {

@PrimaryKey()
id!: number;

@ManyToOne(() => A)
a!: A;

@ManyToOne(() => B)
b!: B;

}

let orm: MikroORM;

describe('populate with citext', () => {

beforeAll(async () => {
orm = await MikroORM.init({
entities: [A, B, C],
dbName: 'test-4843',
});
await orm.schema.ensureDatabase();
await orm.schema.execute('create extension if not exists citext;');
await orm.schema.refreshDatabase();

const knex = orm.em.getKnex();

await knex.queryBuilder()
.insert([{ id1: 'test1', id2: 'test2' }, { id1: 'asDf', id2: 'teST' }])
.into('a');

await knex.queryBuilder()
.insert([{ id: 'test3' }, { id: 'TEst' }])
.into('b');

await knex.queryBuilder()
.insert([{ a_id1: 'test1', a_id2: 'test2', b_id: 'test3' }, { a_id1: 'ASdF', a_id2: 'TEst', b_id: 'TEST' }])
.into('c');
});

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

test('composite FK', async () => {
const a1 = await orm.em.fork().find(A, ['test1', 'test2'], { populate: ['c'] });
expect(a1[0].c.$.toArray()).toEqual([{ id: 1, a: { id1: 'test1', id2: 'test2' }, b: 'test3' }]);

const a2 = await orm.em.fork().find(A, ['asdf', 'test'], { populate: ['c'] });
expect(a2[0].c.$.toArray()).toEqual([{ id: 2, a: { id1: 'asdf', id2: 'test' }, b: 'test' }]);
});

test('simple FK', async () => {
const b1 = await orm.em.fork().find(B, 'test3', { populate: ['c'] });
expect(b1[0].c.$.toArray()).toEqual([{ id: 1, a: { id1: 'test1', id2: 'test2' }, b: 'test3' }]);

const b2 = await orm.em.fork().find(B, 'test', { populate: ['c'] });
expect(b2[0].c.$.toArray()).toEqual([{ id: 2, a: { id1: 'asdf', id2: 'test' }, b: 'test' }]);
});

});

0 comments on commit 41425cb

Please sign in to comment.