fix(core): support sql fragments in custom types with joined strategy

Closes #1594

packages/core/src/hydration/ObjectHydrator.ts

@@ -116,19 +116,19 @@ export class ObjectHydrator extends Hydrator {
       if (prop.mapToPk) {
         ret.push(`      entity.${prop.name} = data.${prop.name};`);
       } else if (prop.wrappedReference) {
-        ret.push(`      entity.${prop.name} = new Reference(factory.createReference('${prop.type}', data.${prop.name}, { merge: true }));`);
+        ret.push(`      entity.${prop.name} = new Reference(factory.createReference('${prop.type}', data.${prop.name}, { merge: true, convertCustomTypes }));`);
       } else {
-        ret.push(`      entity.${prop.name} = factory.createReference('${prop.type}', data.${prop.name}, { merge: true });`);
+        ret.push(`      entity.${prop.name} = factory.createReference('${prop.type}', data.${prop.name}, { merge: true, convertCustomTypes });`);
       }
 
       ret.push(`    } else if (data.${prop.name} && typeof data.${prop.name} === 'object') {`);
 
       if (prop.mapToPk) {
         ret.push(`      entity.${prop.name} = data.${prop.name};`);
       } else if (prop.wrappedReference) {
-        ret.push(`      entity.${prop.name} = new Reference(factory.create('${prop.type}', data.${prop.name}, { initialized: true, merge: true, newEntity }));`);
+        ret.push(`      entity.${prop.name} = new Reference(factory.create('${prop.type}', data.${prop.name}, { initialized: true, merge: true, newEntity, convertCustomTypes }));`);
       } else {
-        ret.push(`      entity.${prop.name} = factory.create('${prop.type}', data.${prop.name}, { initialized: true, merge: true, newEntity });`);
+        ret.push(`      entity.${prop.name} = factory.create('${prop.type}', data.${prop.name}, { initialized: true, merge: true, newEntity, convertCustomTypes });`);
       }
 
       ret.push(`    }`);

packages/knex/src/AbstractSqlDriver.ts

@@ -490,10 +490,15 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
    * @internal
    */
   mapPropToFieldNames<T extends AnyEntity<T>>(qb: QueryBuilder<T>, prop: EntityProperty<T>, tableAlias?: string): Field<T>[] {
+    const aliased = qb.ref(tableAlias ? `${tableAlias}__${prop.fieldNames[0]}` : prop.fieldNames[0]).toString();
+
+    if (prop.customType?.convertToJSValueSQL && tableAlias) {
+      const prefixed = qb.ref(prop.fieldNames[0]).withSchema(tableAlias).toString();
+      return [qb.raw(prop.customType.convertToJSValueSQL(prefixed, this.platform) + ' as ' + aliased).toString()];
+    }
+
     if (prop.formula) {
       const alias = qb.ref(tableAlias ?? qb.alias).toString();
-      const aliased = qb.ref(tableAlias ? `${tableAlias}__${prop.fieldNames[0]}` : prop.fieldNames[0]).toString();
-
       return [`${prop.formula!(alias)} as ${aliased}`];
     }
 
@@ -617,7 +622,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
   protected buildFields<T extends AnyEntity<T>>(meta: EntityMetadata<T>, populate: PopulateOptions<T>[], joinedProps: PopulateOptions<T>[], qb: QueryBuilder<T>, fields?: Field<T>[]): Field<T>[] {
     const lazyProps = meta.props.filter(prop => prop.lazy && !populate.some(p => p.field === prop.name || p.all));
     const hasLazyFormulas = meta.props.some(p => p.lazy && p.formula);
-    const requiresSQLConversion = meta.props.some(p => p.customType?.convertToDatabaseValueSQL || p.customType?.convertToJSValueSQL);
+    const requiresSQLConversion = meta.props.some(p => p.customType?.convertToJSValueSQL);
     const hasExplicitFields = !!fields;
     const ret: Field<T>[] = [];
 

packages/knex/src/query/QueryBuilder.ts

@@ -550,7 +550,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
 
     const meta = this.metadata.find(this.entityName);
     /* istanbul ignore next */
-    const requiresSQLConversion = (meta?.props || []).filter(p => p.customType?.convertToDatabaseValueSQL || p.customType?.convertToJSValueSQL);
+    const requiresSQLConversion = (meta?.props || []).filter(p => p.customType?.convertToJSValueSQL);
 
     if (this.flags.has(QueryFlag.CONVERT_CUSTOM_TYPES) && (fields.includes('*') || fields.includes(`${this.alias}.*`)) && requiresSQLConversion.length > 0) {
       requiresSQLConversion.forEach(p => ret.push(this.helper.mapper(p.name, this.type)));

packages/knex/src/query/QueryBuilderHelper.ts

@@ -38,14 +38,15 @@ export class QueryBuilderHelper {
 
     let ret = field;
     const customExpression = QueryBuilderHelper.isCustomExpression(field);
-    const prop = this.getProperty(field, this.alias);
+    const [a, f] = this.splitField(field);
+    const prop = this.getProperty(f, a);
     const noPrefix = prop && prop.persist === false;
 
     if (prop?.fieldNameRaw) {
       return this.knex.raw(this.prefix(field, true));
     }
 
-    if (prop?.customType && 'convertToJSValueSQL' in prop.customType) {
+    if (prop?.customType?.convertToJSValueSQL) {
       const prefixed = this.prefix(field, true, true);
       /* istanbul ignore next */
       return this.knex.raw(prop.customType.convertToJSValueSQL!(prefixed, this.platform) + ' as ' + this.platform.quoteIdentifier(alias ?? prop.fieldNames[0]));

tests/features/custom-types.test.ts

@@ -1,4 +1,4 @@
-import { Entity, Logger, MikroORM, PrimaryKey, Property, Type } from '@mikro-orm/core';
+import { Entity, LoadStrategy, Logger, ManyToOne, MikroORM, PrimaryKey, Property, Type } from '@mikro-orm/core';
 import { MySqlDriver } from '@mikro-orm/mysql';
 
 export class Point {
@@ -58,13 +58,28 @@ export class Location {
 
 }
 
+@Entity()
+export class Address {
+
+  @PrimaryKey()
+  id!: number;
+
+  @ManyToOne()
+  location: Location;
+
+  constructor(location: Location) {
+    this.location = location;
+  }
+
+}
+
 describe('custom types [mysql]', () => {
 
   let orm: MikroORM<MySqlDriver>;
 
   beforeAll(async () => {
     orm = await MikroORM.init<MySqlDriver>({
-      entities: [Location],
+      entities: [Location, Address],
       dbName: `mikro_orm_test_custom_types`,
       type: 'mysql',
       port: 3307,
@@ -75,6 +90,7 @@ describe('custom types [mysql]', () => {
     await orm.getSchemaGenerator().createSchema();
   });
   beforeEach(async () => {
+    await orm.em.nativeDelete(Address, {});
     await orm.em.nativeDelete(Location, {});
   });
   afterAll(async () => orm.close(true));
@@ -85,9 +101,10 @@ describe('custom types [mysql]', () => {
     Object.assign(orm.config, { logger });
 
     const loc = new Location();
+    const addr = new Address(loc);
     loc.point = new Point(1.23, 4.56);
     loc.extendedPoint = new Point(5.23, 9.56);
-    await orm.em.persistAndFlush(loc);
+    await orm.em.persistAndFlush(addr);
     orm.em.clear();
 
     const l1 = await orm.em.findOneOrFail(Location, loc);
@@ -97,30 +114,44 @@ describe('custom types [mysql]', () => {
     expect(l1.extendedPoint).toMatchObject({ latitude: 5.23, longitude: 9.56 });
     expect(mock.mock.calls[0][0]).toMatch('begin');
     expect(mock.mock.calls[1][0]).toMatch('insert into `location` (`extended_point`, `point`) values (ST_PointFromText(\'point(5.23 9.56)\'), ST_PointFromText(\'point(1.23 4.56)\'))');
-    expect(mock.mock.calls[2][0]).toMatch('commit');
-    expect(mock.mock.calls[3][0]).toMatch('select `e0`.*, ST_AsText(`e0`.`point`) as `point`, ST_AsText(`e0`.`extended_point`) as `extended_point` from `location` as `e0` where `e0`.`id` = ? limit ?');
-    expect(mock.mock.calls).toHaveLength(4);
+    expect(mock.mock.calls[2][0]).toMatch('insert into `address` (`location_id`) values (?)');
+    expect(mock.mock.calls[3][0]).toMatch('commit');
+    expect(mock.mock.calls[4][0]).toMatch('select `e0`.*, ST_AsText(`e0`.`point`) as `point`, ST_AsText(`e0`.`extended_point`) as `extended_point` from `location` as `e0` where `e0`.`id` = ? limit ?');
+    expect(mock.mock.calls).toHaveLength(5);
     await orm.em.flush(); // ensure we do not fire queries when nothing changed
-    expect(mock.mock.calls).toHaveLength(4);
+    expect(mock.mock.calls).toHaveLength(5);
 
     l1.point = new Point(2.34, 9.87);
     await orm.em.flush();
-    expect(mock.mock.calls).toHaveLength(7);
-    expect(mock.mock.calls[4][0]).toMatch('begin');
-    expect(mock.mock.calls[5][0]).toMatch('update `location` set `point` = ST_PointFromText(\'point(2.34 9.87)\') where `id` = ?');
-    expect(mock.mock.calls[6][0]).toMatch('commit');
+    expect(mock.mock.calls).toHaveLength(8);
+    expect(mock.mock.calls[5][0]).toMatch('begin');
+    expect(mock.mock.calls[6][0]).toMatch('update `location` set `point` = ST_PointFromText(\'point(2.34 9.87)\') where `id` = ?');
+    expect(mock.mock.calls[7][0]).toMatch('commit');
     orm.em.clear();
 
     const qb1 = orm.em.createQueryBuilder(Location, 'l');
     const res1 = await qb1.select('*').where({ id: loc.id }).getSingleResult();
-    expect(mock.mock.calls[7][0]).toMatch('select `l`.*, ST_AsText(`l`.`point`) as `point`, ST_AsText(`l`.`extended_point`) as `extended_point` from `location` as `l` where `l`.`id` = ?');
+    expect(mock.mock.calls[8][0]).toMatch('select `l`.*, ST_AsText(`l`.`point`) as `point`, ST_AsText(`l`.`extended_point`) as `extended_point` from `location` as `l` where `l`.`id` = ?');
     expect(res1).toMatchObject(l1);
     orm.em.clear();
 
     const qb2 = orm.em.createQueryBuilder(Location);
     const res2 = await qb2.select(['e0.*']).where({ id: loc.id }).getSingleResult();
-    expect(mock.mock.calls[8][0]).toMatch('select `e0`.*, ST_AsText(`e0`.`point`) as `point`, ST_AsText(`e0`.`extended_point`) as `extended_point` from `location` as `e0` where `e0`.`id` = ?');
+    expect(mock.mock.calls[9][0]).toMatch('select `e0`.*, ST_AsText(`e0`.`point`) as `point`, ST_AsText(`e0`.`extended_point`) as `extended_point` from `location` as `e0` where `e0`.`id` = ?');
     expect(res2).toMatchObject(l1);
+    mock.mock.calls.length = 0;
+    orm.em.clear();
+
+    // custom types with SQL fragments with joined strategy (GH #1594)
+    const a2 = await orm.em.findOneOrFail(Address, addr, { populate: { location: LoadStrategy.JOINED } });
+    expect(mock.mock.calls[0][0]).toMatch('select `e0`.`id`, `e0`.`location_id`, `l1`.`id` as `l1__id`, ST_AsText(`l1`.`point`) as `l1__point`, ST_AsText(`l1`.`extended_point`) as `l1__extended_point` from `address` as `e0` left join `location` as `l1` on `e0`.`location_id` = `l1`.`id` where `e0`.`id` = ?');
+    expect(a2.location.point).toBeInstanceOf(Point);
+    expect(a2.location.point).toMatchObject({ latitude: 2.34, longitude: 9.87 });
+    expect(a2.location.extendedPoint).toBeInstanceOf(Point);
+    expect(a2.location.extendedPoint).toMatchObject({ latitude: 5.23, longitude: 9.56 });
+    expect(mock.mock.calls).toHaveLength(1);
+    await orm.em.flush(); // ensure we do not fire queries when nothing changed
+    expect(mock.mock.calls).toHaveLength(1);
   });
 
   test('extending custom types (gh issue 1442)', async () => {