perf(query-builder): use distinct counts only when joining to-many re…

…lations

Closes #3044

packages/knex/src/AbstractSqlDriver.ts

@@ -188,16 +188,15 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
   }
 
   async count<T extends AnyEntity<T>>(entityName: string, where: any, options: CountOptions<T> = {}): Promise<number> {
-    const meta = this.metadata.find(entityName)!;
-    const pks = meta.primaryKeys;
+    const meta = this.metadata.find(entityName);
     const qb = this.createQueryBuilder(entityName, options.ctx, options.connectionType, false)
       .groupBy(options.groupBy!)
       .having(options.having!)
       .populate(options.populate as unknown as PopulateOptions<T>[] ?? [])
       .withSchema(this.getSchemaName(meta, options))
       .where(where);
 
-    return this.rethrow(qb.getCount(pks, true));
+    return this.rethrow(qb.getCount());
   }
 
   async nativeInsert<T extends AnyEntity<T>>(entityName: string, data: EntityDictionary<T>, options: NativeInsertUpdateOptions<T> = {}): Promise<QueryResult<T>> {

packages/knex/src/query/QueryBuilder.ts

@@ -125,7 +125,13 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
   }
 
   count(field?: string | string[], distinct = false): CountQueryBuilder<T> {
-    this._fields = [...(field ? Utils.asArray(field) : this.metadata.find(this.entityName)!.primaryKeys)];
+    if (field) {
+      this._fields = Utils.asArray(field);
+    } else if (this.hasToManyJoins()) {
+      this._fields = this.metadata.find(this.entityName)!.primaryKeys;
+    } else {
+      this._fields = [this.raw('*')];
+    }
 
     if (distinct) {
       this.flags.add(QueryFlag.DISTINCT);
@@ -520,9 +526,9 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
   /**
    * Executes count query (without offset and limit), returning total count of results
    */
-  async getCount(field?: string | string[], distinct = false): Promise<number> {
+  async getCount(field?: string | string[], distinct?: boolean): Promise<number> {
     const qb = this.clone();
-    qb.count(field, distinct).limit(undefined).offset(undefined).orderBy([]);
+    qb.count(field, distinct ?? qb.hasToManyJoins()).limit(undefined).offset(undefined).orderBy([]);
     const res = await qb.execute<{ count: number }>('get', false);
 
     return res ? +res.count : 0;
@@ -847,7 +853,8 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
     }
   }
 
-  private hasToManyJoins(): boolean {
+  /** @internal */
+  hasToManyJoins(): boolean {
     return Object.values(this._joins).some(join => {
       return [ReferenceType.ONE_TO_MANY, ReferenceType.MANY_TO_MANY].includes(join.prop.reference);
     });

packages/knex/src/query/QueryBuilderHelper.ts

@@ -21,9 +21,13 @@ export class QueryBuilderHelper {
               private readonly knex: Knex,
               private readonly driver: AbstractSqlDriver) { }
 
-  mapper(field: string, type?: QueryType): string;
-  mapper(field: string, type?: QueryType, value?: any, alias?: string | null): string;
-  mapper(field: string, type = QueryType.SELECT, value?: any, alias?: string | null): string | Knex.Raw {
+  mapper(field: string | Knex.Raw, type?: QueryType): string;
+  mapper(field: string | Knex.Raw, type?: QueryType, value?: any, alias?: string | null): string;
+  mapper(field: string | Knex.Raw, type = QueryType.SELECT, value?: any, alias?: string | null): string | Knex.Raw {
+    if (typeof field !== 'string') {
+      return field;
+    }
+
     const isTableNameAliasRequired = this.isTableNameAliasRequired(type);
     const fields = Utils.splitPrimaryKeys(field);
 

tests/EntityManager.sqlite2.test.ts

@@ -1078,10 +1078,10 @@ describe.each(['sqlite', 'better-sqlite'] as const)('EntityManager (%s)', driver
     expect(count3).toBe(2);
     const count4 = await orm.em.createQueryBuilder(Author4).where({ email: '123' }).getCount();
     expect(count4).toBe(0);
-    expect(mock.mock.calls[0][0]).toMatch('select count(`a0`.`id`) as `count` from `author4` as `a0`');
+    expect(mock.mock.calls[0][0]).toMatch('select count(*) as `count` from `author4` as `a0`');
     expect(mock.mock.calls[1][0]).toMatch('select count(`a0`.`terms_accepted`) as `count` from `author4` as `a0`');
     expect(mock.mock.calls[2][0]).toMatch('select count(distinct `a0`.`terms_accepted`) as `count` from `author4` as `a0`');
-    expect(mock.mock.calls[3][0]).toMatch('select count(`a0`.`id`) as `count` from `author4` as `a0` where `a0`.`email` = \'123\'');
+    expect(mock.mock.calls[3][0]).toMatch('select count(*) as `count` from `author4` as `a0` where `a0`.`email` = \'123\'');
   });
 
   test('using collection methods with null/undefined (GH issue #1408)', async () => {

tests/QueryBuilder.test.ts

@@ -874,7 +874,7 @@ describe('QueryBuilder', () => {
   test('select count query', async () => {
     const qb = orm.em.createQueryBuilder(Publisher2);
     qb.count().where({ name: 'test 123', type: PublisherType.GLOBAL });
-    expect(qb.getQuery()).toEqual('select count(`e0`.`id`) as `count` from `publisher2` as `e0` where `e0`.`name` = ? and `e0`.`type` = ?');
+    expect(qb.getQuery()).toEqual('select count(*) as `count` from `publisher2` as `e0` where `e0`.`name` = ? and `e0`.`type` = ?');
     expect(qb.getParams()).toEqual(['test 123', PublisherType.GLOBAL]);
   });
 
@@ -888,7 +888,7 @@ describe('QueryBuilder', () => {
   test('select count with non-standard PK field name (uuid_pk)', async () => {
     const qb = orm.em.createQueryBuilder(Book2);
     qb.count().where({ title: 'test 123' });
-    expect(qb.getQuery()).toEqual('select count(`e0`.`uuid_pk`) as `count` from `book2` as `e0` where `e0`.`title` = ?');
+    expect(qb.getQuery()).toEqual('select count(*) as `count` from `book2` as `e0` where `e0`.`title` = ?');
     expect(qb.getParams()).toEqual(['test 123']);
   });
 
@@ -2527,7 +2527,7 @@ describe('QueryBuilder', () => {
   test('count query with auto-joining (GH issue 858)', async () => {
     // m:1 -> 1:1 inverse -> PK
     const sql1 = orm.em.createQueryBuilder(Author2).count().where({ favouriteBook: { test: { id: 1 } } }).getQuery();
-    expect(sql1).toBe('select count(`e0`.`id`) as `count` ' +
+    expect(sql1).toBe('select count(*) as `count` ' +
       'from `author2` as `e0` ' +
       'left join `book2` as `e1` on `e0`.`favourite_book_uuid_pk` = `e1`.`uuid_pk` ' +
       'left join `test2` as `e2` on `e1`.`uuid_pk` = `e2`.`book_uuid_pk` ' +
@@ -2541,7 +2541,7 @@ describe('QueryBuilder', () => {
       'where `e2`.`id` = ?');
 
     const sql3 = orm.em.createQueryBuilder(Book2).count().where({ test: { id: 1 } }).getQuery();
-    expect(sql3).toBe('select count(`e0`.`uuid_pk`) as `count` ' +
+    expect(sql3).toBe('select count(*) as `count` ' +
       'from `book2` as `e0` ' +
       'left join `test2` as `e1` on `e0`.`uuid_pk` = `e1`.`book_uuid_pk` ' +
       'where `e1`.`id` = ?');

tests/features/filters/filters.mysql.test.ts

@@ -83,7 +83,7 @@ describe('filters [mysql]', () => {
     await orm.em.count(Book2, { author: { name: 'Jon' } }, {
       filters: { hasAuthor: false, long: true },
     });
-    expect(mock.mock.calls[6][0]).toMatch('select count(distinct `b0`.`uuid_pk`) as `count` ' +
+    expect(mock.mock.calls[6][0]).toMatch('select count(*) as `count` ' +
       'from `book2` as `b0` ' +
       'left join `author2` as `a1` on `b0`.`author_id` = `a1`.`id` ' +
       'where length(perex) > 10000 and `a1`.`name` = \'Jon\'');

tests/issues/GH519.test.ts

@@ -76,7 +76,7 @@ describe('GH issue 519', () => {
     const queries: string[] = mock.mock.calls.map(c => c[0]).sort();
     expect(queries).toHaveLength(2);
     expect(queries[0]).toMatch('select "r0".* from "registration" as "r0" where "r0"."competition_id" = $1');
-    expect(queries[1]).toMatch('select count(distinct("r0"."competition_id", "r0"."user_id")) as "count" from "registration" as "r0" where "r0"."competition_id" = $1');
+    expect(queries[1]).toMatch('select count(*) as "count" from "registration" as "r0" where "r0"."competition_id" = $1');
   });
 
 });