fix(sql): use __ when aliasing fetch-joined properties

Previously fetch-joining failed when there was a conflict in aliased property name
and parent entity property name and the alias is same as the parent property name
(e.g. property `Book.author` and alias `author`).

Related #1171

packages/knex/src/AbstractSqlDriver.ts

@@ -110,7 +110,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
       // If the primary key value for the relation is null, we know we haven't joined to anything
       // and therefore we don't return any record (since all values would be null)
       const hasPK = meta2.primaryKeys.every(pk => meta2.properties[pk].fieldNames.every(name => {
-        return Utils.isDefined(root![`${relationAlias}_${name}`], true);
+        return Utils.isDefined(root![`${relationAlias}__${name}`], true);
       }));
 
       if (!hasPK) {
@@ -122,9 +122,9 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
         .forEach(prop => {
           if (prop.fieldNames.length > 1) { // composite keys
             relationPojo[prop.name] = prop.fieldNames.map(name => root![`${relationAlias}_${name}`]);
-            prop.fieldNames.map(name => delete root![`${relationAlias}_${name}`]);
+            prop.fieldNames.map(name => delete root![`${relationAlias}__${name}`]);
           } else {
-            const alias = `${relationAlias}_${prop.fieldNames[0]}`;
+            const alias = `${relationAlias}__${prop.fieldNames[0]}`;
             relationPojo[prop.name] = root![alias];
             delete root![alias];
           }
@@ -480,13 +480,13 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
   mapPropToFieldNames<T extends AnyEntity<T>>(qb: QueryBuilder<T>, prop: EntityProperty<T>, tableAlias?: string): Field<T>[] {
     if (prop.formula) {
       const alias = qb.ref(tableAlias ?? qb.alias).toString();
-      const aliased = qb.ref(tableAlias ? `${tableAlias}_${prop.fieldNames[0]}` : prop.fieldNames[0]).toString();
+      const aliased = qb.ref(tableAlias ? `${tableAlias}__${prop.fieldNames[0]}` : prop.fieldNames[0]).toString();
 
       return [`${prop.formula!(alias)} as ${aliased}`];
     }
 
     if (tableAlias) {
-      return prop.fieldNames.map(fieldName => qb.ref(fieldName).withSchema(tableAlias).as(`${tableAlias}_${fieldName}`));
+      return prop.fieldNames.map(fieldName => qb.ref(fieldName).withSchema(tableAlias).as(`${tableAlias}__${fieldName}`));
     }
 
     return prop.fieldNames;

packages/knex/src/query/QueryBuilder.ts

@@ -144,8 +144,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
 
   protected getFieldsForJoinedLoad<U extends AnyEntity<U>>(prop: EntityProperty<U>, alias: string): Field<U>[] {
     const fields: Field<U>[] = [];
-    const meta2 = this.metadata.find<U>(prop.type)!;
-    meta2.props
+    prop.targetMeta!.props
       .filter(prop => this.driver.shouldHaveColumn(prop, this._populate))
       .forEach(prop => fields.push(...this.driver.mapPropToFieldNames<U>(this as unknown as QueryBuilder<U>, prop, alias)));
 

tests/EntityManager.sqlite2.test.ts

@@ -924,8 +924,8 @@ describe('EntityManagerSqlite2', () => {
       .leftJoinAndSelect('b.tags', 't')
       .where({ 't.name': ['sick', 'sexy'] });
     const sql = 'select `a`.*, ' +
-      '`b`.`id` as `b_id`, `b`.`created_at` as `b_created_at`, `b`.`updated_at` as `b_updated_at`, `b`.`title` as `b_title`, `b`.`author_id` as `b_author_id`, `b`.`publisher_id` as `b_publisher_id`, `b`.`meta` as `b_meta`, ' +
-      '`t`.`id` as `t_id`, `t`.`created_at` as `t_created_at`, `t`.`updated_at` as `t_updated_at`, `t`.`name` as `t_name`, `t`.`version` as `t_version` ' +
+      '`b`.`id` as `b__id`, `b`.`created_at` as `b__created_at`, `b`.`updated_at` as `b__updated_at`, `b`.`title` as `b__title`, `b`.`author_id` as `b__author_id`, `b`.`publisher_id` as `b__publisher_id`, `b`.`meta` as `b__meta`, ' +
+      '`t`.`id` as `t__id`, `t`.`created_at` as `t__created_at`, `t`.`updated_at` as `t__updated_at`, `t`.`name` as `t__name`, `t`.`version` as `t__version` ' +
       'from `author4` as `a` ' +
       'left join `book4` as `b` on `a`.`id` = `b`.`author_id` ' +
       'left join `tags_ordered` as `e1` on `b`.`id` = `e1`.`book4_id` ' +

tests/QueryBuilder.test.ts

@@ -204,8 +204,8 @@ describe('QueryBuilder', () => {
       .where({ 'fz.name': 'baz' })
       .limit(1);
     const sql = 'select `fb1`.*, ' +
-      '`fz`.`id` as `fz_id`, `fz`.`name` as `fz_name`, `fz`.`version` as `fz_version`, ' +
-      '`fb2`.`id` as `fb2_id`, `fb2`.`name` as `fb2_name`, `fb2`.`baz_id` as `fb2_baz_id`, `fb2`.`foo_bar_id` as `fb2_foo_bar_id`, `fb2`.`version` as `fb2_version`, `fb2`.`blob` as `fb2_blob`, `fb2`.`array` as `fb2_array`, `fb2`.`object` as `fb2_object`, (select 123) as `fb2_random`, ' +
+      '`fz`.`id` as `fz__id`, `fz`.`name` as `fz__name`, `fz`.`version` as `fz__version`, ' +
+      '`fb2`.`id` as `fb2__id`, `fb2`.`name` as `fb2__name`, `fb2`.`baz_id` as `fb2__baz_id`, `fb2`.`foo_bar_id` as `fb2__foo_bar_id`, `fb2`.`version` as `fb2__version`, `fb2`.`blob` as `fb2__blob`, `fb2`.`array` as `fb2__array`, `fb2`.`object` as `fb2__object`, (select 123) as `fb2__random`, ' +
       '(select 123) as `random` from `foo_bar2` as `fb1` ' +
       'inner join `foo_baz2` as `fz` on `fb1`.`baz_id` = `fz`.`id` ' +
       'left join `foo_bar2` as `fb2` on `fz`.`id` = `fb2`.`baz_id` ' +

tests/issues/GH1041.test.ts

@@ -97,7 +97,7 @@ describe('GH issue 1041, 1043', () => {
   test('joined strategy: find by many-to-many relation ID', async () => {
     const findPromise = orm.em.findOne(User, { apps: 1 }, { populate: { apps: LoadStrategy.JOINED } });
     await expect(findPromise).resolves.toBeInstanceOf(User);
-    expect(log.mock.calls[0][0]).toMatch('select `e0`.`id`, `e0`.`name`, `a1`.`id` as `a1_id`, `a1`.`name` as `a1_name` from `user` as `e0` left join `user_apps` as `e2` on `e0`.`id` = `e2`.`user_id` left join `app` as `a1` on `e2`.`app_id` = `a1`.`id` where `e2`.`app_id` = 1');
+    expect(log.mock.calls[0][0]).toMatch('select `e0`.`id`, `e0`.`name`, `a1`.`id` as `a1__id`, `a1`.`name` as `a1__name` from `user` as `e0` left join `user_apps` as `e2` on `e0`.`id` = `e2`.`user_id` left join `app` as `a1` on `e2`.`app_id` = `a1`.`id` where `e2`.`app_id` = 1');
   });
 
   test('select-in strategy: find by many-to-many relation IDs', async () => {
@@ -110,7 +110,7 @@ describe('GH issue 1041, 1043', () => {
   test('joined strategy: find by many-to-many relation IDs', async () => {
     const findPromise = orm.em.findOne(User, { apps: [1, 2, 3] }, { populate: { apps: LoadStrategy.JOINED } });
     await expect(findPromise).resolves.toBeInstanceOf(User);
-    expect(log.mock.calls[0][0]).toMatch('select `e0`.`id`, `e0`.`name`, `a1`.`id` as `a1_id`, `a1`.`name` as `a1_name` from `user` as `e0` left join `user_apps` as `e2` on `e0`.`id` = `e2`.`user_id` left join `app` as `a1` on `e2`.`app_id` = `a1`.`id` where `e2`.`app_id` in (1, 2, 3)');
+    expect(log.mock.calls[0][0]).toMatch('select `e0`.`id`, `e0`.`name`, `a1`.`id` as `a1__id`, `a1`.`name` as `a1__name` from `user` as `e0` left join `user_apps` as `e2` on `e0`.`id` = `e2`.`user_id` left join `app` as `a1` on `e2`.`app_id` = `a1`.`id` where `e2`.`app_id` in (1, 2, 3)');
   });
 
 });

tests/issues/GH1171.test.ts

@@ -50,11 +50,12 @@ describe('GH issue 1171', () => {
     const orderedAs = await orm.em
       .createQueryBuilder(A)
       .select('*')
-      .leftJoinAndSelect('b', 'b0')
-      .orderBy({ 'b0.name': 'asc' })
+      .leftJoinAndSelect('b', 'b')
+      .orderBy({ 'b.name': 'asc' })
       .getResult();
 
     expect(orderedAs.map(e => e.id)).toEqual([a3.id, a2.id, a1.id]);
+    expect(orderedAs.map(e => e.b.name)).toEqual([b3.name, b2.name, b1.name]);
   });
 
 });