Wrong totalCount when using joins in QB

[rudolfmedula]

In this commit 890ee16#diff-c58e144dadf00250381ed55e6ce83245cda76aca84131ad494fd4911932f656f. a custom total count query was introduced instead of TypeORMs qb.getCount();

this query being SELECT COUNT(*) FROM (..) returns number of all rows returned by query which would be like (m*n* k...) depending on how many rows were joined

the original implementation of TypeORMs getCount does SELECT COUNT(DISTINCT("table"."id")) AS "cnt" FROM (..)
which returns correct number of entities even when using JOINs

[rudolfmedula]

I can provide failing test, but the issue is pretty obvious. I'm curious what was the motivation for not using TypeORMs qb.getCount ?

[bashleigh]

sure, I haven't looked into this yet, don't suppose you'd know a way in typeorm to get the primary key column?

[rudolfmedula]

well in TypeORM they are able to get the primary columns from the metadata https://github.com/typeorm/typeorm/blob/dd94c9d38d62c98f7afd5396abbc7e32ba689aad/src/query-builder/SelectQueryBuilder.ts#L1833
I think they have the count query figured out and available in the QB API. If you could help me understand the motivation for the custom count query, would help, thanks.

[bashleigh]

Yea we need a more advanced query which I was trying to implement for others which doesn't have the distinct func so adding that will solve your problem and enable more advanced queries for others!

[rudolfmedula]

@bashleigh I've added the failing test as promised. Hope it's helpful.

master...rudolfmedula:issues/627

[bashleigh]

Haven't forgotten, been pretty ill. Don't suppose you could make a PR for your changes? That way I might as well merge them into master and solve the problem there

[rudolfmedula]

sure https://github.com/nestjsx/nestjs-typeorm-paginate/pull/634/files

[gulaandrij]

is there any chance to fix it?

[NikolayYakovenko]

I ran into the same problem.
@bashleigh, do you have any information about solving this problem?

[bashleigh]

Honestly, I haven't had a lot of time to look into this unfortunately. Please feel free to have a go yourselves! Really sorry I've left it so long!

[ayeen]

@rudolfmedula @NikolayYakovenko use the below code snippet, I hope this will resolve your problem, this helped me with the same issue I had faced in my application
options.paginationType = PaginationTypeEnum.TAKE_AND_SKIP

[NikolayYakovenko]

@ayeen, thank you for your advice.

I know about this option and used it.
But in my case the result was not completely correct. When I set paginationType: PaginationTypeEnum.TAKE_AND_SKIP,
I've got the correct value (count) of items, but meta.totalItems was incorrect.
For example, items is an array of 5 elements (which is true), but meta.totalItems is 10.

[ayeen]

@ayeen, thank you for your advice.

I know about this option and used it. But in my case the result was not completely correct. When I set paginationType: PaginationTypeEnum.TAKE_AND_SKIP, I've got the correct value (count) of items, but meta.totalItems was incorrect. For example, items is an array of 5 elements (which is true), but meta.totalItems is 10.

@NikolayYakovenko this means you are missing something like you are query on the wrong table which shows the total of 10 items and in a result that shows 5 items from join table.

[FelipeGerolomo]

Same problem :(

[ayeen]

@FelipeGerolomo I face this issue when I am trying to order by the data from inner relation! I found one solution I set the order by entity definition and remove it from the query then its worked fine for me.
Entity( {
orderBy: {
id: "DESC"
}})

[thgiang-ctp]

Same problem with NikolayYakovenko :(

[celianvdb]

Same issue here.

[martinsotirov]

Not only is the total count wrong with the most recent @nest/typeorm but also the actual items themselves are wrong, because it seems to count the LEFT JOINed items towards the total count. This doesn't happen with Nest 7.

E.g.

    const qb = this.vendorsRepo
      .createQueryBuilder('vendor')
      .leftJoinAndSelect('vendor.features', 'features')

    return paginate<VendorEntity>(qb, { limit: 5, page: 1 })

Gives me 1 vendor with 5 features instead of 5 vendors with 5 features each. Even though I only have 10 vendors with 5 features each, the meta gives me:

{
  "items": [
    {
      "id": 31,
      "name": "Schowalter, Hegmann and Wilkinson",
      "slug": "Schowalter-Hegmann-and-Wilkinson",
      "visible": false,
      "summary": null,
      "description": "Ergonomic executive chair upholstered in bonded black leather and PVC padded seat and back for all-day comfort and support",
      "features": [
        {
          "id": 1,
          "value": "lavender"
        },
        {
          "id": 2,
          "value": "maroon"
        },
        {
          "id": 3,
          "value": "cyan"
        },
        {
          "id": 4,
          "value": "azure"
        },
        {
          "id": 5,
          "value": "blue"
        }
      ]
    }
  ],
  "meta": {
    "totalItems": 50,
    "itemCount": 1,
    "itemsPerPage": 5,
    "totalPages": 10,
    "currentPage": 1
  }
}

@ayeen's solution fixes the items but the totalItems in the meta block remain wrong.

[bashleigh]

made a quick release today, 3.2.0 removing order by from the count query #706. I was given an example of an issue with total counts and I managed to resolve it without changing the pagination function which is really annoying :/

I'll give it another go today (got a few hours spare currently 🤞🏼) what I need to do to solve this once and forall would be to add a groupBy on the parent table with the primary key however not everyone's setup is the same so this would break in some cases. Plus I would need to resolve this myself without adding a new property config (because who wants to specify their primary key) which shouldn't be an issue. It's the fact that not everyone will have a primary key that's making me feel it's not a good enough solution.

Have also noticed this function loadRelationCountAndMap which I want to investigate to see if it'll mitigate issues with left join but not holding out hope tbh.

[krzysztofsaja]

Same issue in my case :(

[zarnautovic]

any news about this ? I have same issue with 3.2.0

[gulaandrij]

we stays on 2.x version because of this bug

[WalleksMR]

@ayeen, thank you for your advice.

I know about this option and used it. But in my case the result was not completely correct. When I set paginationType: PaginationTypeEnum.TAKE_AND_SKIP, I've got the correct value (count) of items, but meta.totalItems was incorrect. For example, items is an array of 5 elements (which is true), but meta.totalItems is 10.

My solution with this problem, was return the function metaTransformer and modifying the return of totalItems and totalPages.

async listAll({
    page,
    limit,
  }: IListAllProps): Promise<Pagination<PeopleM>> {
    const peoples = this.peopleEntityRepository
      .createQueryBuilder('p')
      .leftJoinAndSelect('p.lead_step', 'lead')
      .leftJoinAndSelect('p.schedules', 's')
      .select(['p', 'lead', 's.id', 's.started_at', 's.finished_at']);

    // Here I get total items of table peoples
    const totalItems = await peoples.getCount();
    
    return await paginate<People>(peoples, {
      limit,
      page,
      paginationType: PaginationTypeEnum.TAKE_AND_SKIP,
      metaTransformer: ({ currentPage, itemCount, itemsPerPage }) => {

     // Calculating the total of pages
        const totalPages = Math.round(totalItems / itemsPerPage);
        return {
          currentPage,
          itemCount,
          itemsPerPage,

         // Returning in this two row
          totalItems,
          totalPages: totalPages === 0 ? 1 : totalPages,
        };
      },
    });
  }

I know it’s not right this way, but was that I found at moment.

[Anisi]

@ayeen, thank you for your advice.
I know about this option and used it. But in my case the result was not completely correct. When I set paginationType: PaginationTypeEnum.TAKE_AND_SKIP, I've got the correct value (count) of items, but meta.totalItems was incorrect. For example, items is an array of 5 elements (which is true), but meta.totalItems is 10.

My solution with this problem, was return the function metaTransformer and modifying the return of totalItems and totalPages.

async listAll({
    page,
    limit,
  }: IListAllProps): Promise<Pagination<PeopleM>> {
    const peoples = this.peopleEntityRepository
      .createQueryBuilder('p')
      .leftJoinAndSelect('p.lead_step', 'lead')
      .leftJoinAndSelect('p.schedules', 's')
      .select(['p', 'lead', 's.id', 's.started_at', 's.finished_at']);

    // Here I get total items of table peoples
    const totalItems = await peoples.getCount();
    
    return await paginate<People>(peoples, {
      limit,
      page,
      paginationType: PaginationTypeEnum.TAKE_AND_SKIP,
      metaTransformer: ({ currentPage, itemCount, itemsPerPage }) => {

     // Calculating the total of pages
        const totalPages = Math.round(totalItems / itemsPerPage);
        return {
          currentPage,
          itemCount,
          itemsPerPage,

         // Returning in this two row
          totalItems,
          totalPages: totalPages === 0 ? 1 : totalPages,
        };
      },
    });
  }

I know it’s not right this way, but was that I found at moment.

Math.round should be replaced by Math.ceil.

[WalleksMR]

@ayeen, thank you for your advice.
I know about this option and used it. But in my case the result was not completely correct. When I set paginationType: PaginationTypeEnum.TAKE_AND_SKIP, I've got the correct value (count) of items, but meta.totalItems was incorrect. For example, items is an array of 5 elements (which is true), but meta.totalItems is 10.

My solution with this problem, was return the function metaTransformer and modifying the return of totalItems and totalPages.

async listAll({
    page,
    limit,
  }: IListAllProps): Promise<Pagination<PeopleM>> {
    const peoples = this.peopleEntityRepository
      .createQueryBuilder('p')
      .leftJoinAndSelect('p.lead_step', 'lead')
      .leftJoinAndSelect('p.schedules', 's')
      .select(['p', 'lead', 's.id', 's.started_at', 's.finished_at']);

    // Here I get total items of table peoples
    const totalItems = await peoples.getCount();
    
    return await paginate<People>(peoples, {
      limit,
      page,
      paginationType: PaginationTypeEnum.TAKE_AND_SKIP,
      metaTransformer: ({ currentPage, itemCount, itemsPerPage }) => {

     // Calculating the total of pages
        const totalPages = Math.round(totalItems / itemsPerPage);
        return {
          currentPage,
          itemCount,
          itemsPerPage,

         // Returning in this two row
          totalItems,
          totalPages: totalPages === 0 ? 1 : totalPages,
        };
      },
    });
  }

I know it’s not right this way, but was that I found at moment.

Math.round should be replaced by Math.ceil.

Thank you, that's right!

[bnmlynx]

made a quick release today, 3.2.0 removing order by from the count query #706. I was given an example of an issue with total counts and I managed to resolve it without changing the pagination function which is really annoying :/

I'll give it another go today (got a few hours spare currently 🤞🏼) what I need to do to solve this once and forall would be to add a groupBy on the parent table with the primary key however not everyone's setup is the same so this would break in some cases. Plus I would need to resolve this myself without adding a new property config (because who wants to specify their primary key) which shouldn't be an issue. It's the fact that not everyone will have a primary key that's making me feel it's not a good enough solution.

Have also noticed this function loadRelationCountAndMap which I want to investigate to see if it'll mitigate issues with left join but not holding out hope tbh.

what did you change to resolve it? Still seems to be an issues for me.

[b0nbon1]

I'm also facing the issue, any workarounds?

[zarnautovic]

Any news about this ?

[francois-codes]

For now the only workaround we've found is to remove the join from the query, but that wouldn't work everytime.

has this fallen off the radar ?

[psam44]

Inspired by the other comments, my current workaround is:

    const totalItems = await queryBuilder.clone().getCount()
    return paginate<MyEntity>(queryBuilder, {
      ...options, countQueries: false,
      metaTransformer: (meta: IPaginationMeta): IPaginationMeta => ({
        ...meta,
        totalItems,
        totalPages: Math.ceil(totalItems / meta.itemsPerPage)
      })
    })

[psam44]

@bashleigh, if your concern is about the presence of primary keys that getCount() seems to rely on, here is a proposal for a workaround (not tested).
At least it makes the count workable again for our very usual use case, that is: some joins with the entity table having a primary key.
It is not a full fix as it doesn't cover the case of: some joins, with no primary key on the entity table.

@@ -1,10 +1,12 @@
 const countQuery = async <T>(
   queryBuilder: SelectQueryBuilder<T>,
   cacheOption: TypeORMCacheType,
 ): Promise<number> => {
   const totalQueryBuilder = queryBuilder.clone();
+  if (totalQueryBuilder.expressionMap.mainAlias?.metadata.primaryColumns.length > 0)
+    return totalQueryBuilder.getCount()
 
   totalQueryBuilder
     .skip(undefined)
     .limit(undefined)
     .offset(undefined)

[jeffrey008]

Hi team, any update on this issue? I am having a query with join where I limit is 20, but the actual record only returns 10.

[andrescjhl]

Inspired by the other comments, my current workaround is:

    const totalItems = await queryBuilder.clone().getCount()
    return paginate<MyEntity>(queryBuilder, {
      ...options, countQueries: false,
      metaTransformer: (meta: IPaginationMeta): IPaginationMeta => ({
        ...meta,
        totalItems,
        totalPages: Math.ceil(totalItems / meta.itemsPerPage)
      })
    })

This fixes the problem, but It stops returning the "links" property for the next, previous page etc. Is there a way to keep the links ? @psam44

[andrescjhl]

It stops returning the "links" property because it needs que countQuery information to build the property and countQueries is set to false since it does the count wrong . For anyone that gets here , the logic of how the links property is build is on the createPaginationObject :

nestjs-typeorm-paginate/src/create-pagination.ts

Line 9 in 774bbd9

export function createPaginationObject<

The function is exported so you can use it with the correct totalItems count

[psam44]

@andrescjhl Right, my project doesn't use the links object, so probably I didn't pay attention to this part. Once again, it's just a workaround, not a fix. Thanks to point towards the complement.

[bashleigh]

@psam44 there's a few issues really. The difficult part about this problem is a "one shoe fits all" solution for everyone's individual needs with different queries. Yes the getCount function on typeorm is the immediate problem as it removes havings from the query therefore if we change the overall usage to getCount we lose the ability to generate count queries with havings.
I've come away from open source for a bit as I've not been well really so I'm sorry it's been a while. However, I'm starting to feel a lot better etc.
Am considering merging this PR #805 as it would provide an option to change the count query method. However it would mean knowing when to change it to which count query.
But yes it would appear the main breaker is that typeorm's relationships are counted differently to the fetch method which dehydrates the extra values from what I remember? It's been a long time (sorry guys)