nestjs-typeorm-paginate
nestjs-typeorm-paginate copied to clipboard
Wrong totalCount when using joins in QB
In this commit https://github.com/nestjsx/nestjs-typeorm-paginate/commit/890ee164e15a1495782917b8443f0cd4cca0b7c0#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
I can provide failing test, but the issue is pretty obvious. I'm curious what was the motivation for not using TypeORMs qb.getCount ?
sure, I haven't looked into this yet, don't suppose you'd know a way in typeorm to get the primary key column?
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.
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!
@bashleigh I've added the failing test as promised. Hope it's helpful.
https://github.com/nestjsx/nestjs-typeorm-paginate/compare/master...rudolfmedula:issues/627
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
sure https://github.com/nestjsx/nestjs-typeorm-paginate/pull/634/files
is there any chance to fix it?
I ran into the same problem. @bashleigh, do you have any information about solving this problem?
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!
@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
@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, 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) ofitems, butmeta.totalItemswas incorrect. For example,itemsis an array of 5 elements (which is true), butmeta.totalItemsis 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.
Same problem :(
@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" }})
Same problem with NikolayYakovenko :(
Same issue here.
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.
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.
Same issue in my case :(
any news about this ? I have same issue with 3.2.0
we stays on 2.x version because of this bug
@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) ofitems, butmeta.totalItemswas incorrect. For example,itemsis an array of 5 elements (which is true), butmeta.totalItemsis 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.
@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) ofitems, butmeta.totalItemswas incorrect. For example,itemsis an array of 5 elements (which is true), butmeta.totalItemsis 10.My solution with this problem, was return the function
metaTransformerand modifying the return oftotalItemsandtotalPages.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.
@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) ofitems, butmeta.totalItemswas incorrect. For example,itemsis an array of 5 elements (which is true), butmeta.totalItemsis 10.My solution with this problem, was return the function
metaTransformerand modifying the return oftotalItemsandtotalPages.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.roundshould be replaced byMath.ceil.
Thank you, that's right!
made a quick release today,
3.2.0removing 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
loadRelationCountAndMapwhich 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.
I'm also facing the issue, any workarounds?
Any news about this ?
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 ?
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)
})
})