-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrong count query for findAndCountAll with include #7225
Comments
If you specify an include, it will be passed to the count query, that's how it works. If you don't want the JOIN, don't pass the include in the findAndCountAll |
Pass |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment 🙂 |
Personally I think LEFT JOIN's should not be here as well. Count of rows cannot be changed if we use this type of JOIN or just omit it, but result query will be faster then. |
I think making this into a separate query would be a better approach since you might not want to use |
For posterity: Using a separate query here seems like the right thing to do IF you're concerned about performance. with findAndCountAll, Sequelize is already performing two queries (a async function findAndCountApples(appleType)
const where = { apply_type: appleType }
const asyncRows = appleModel.findAll({
where,
include: ...,
offset: ...
limit: ...
})
const asyncCount = model.count({where})
return Promise.all([asyncRows, asyncCount]).then(([rows, count]) => ({rows, count}))
} Of course, if the full query with all joins is not an expensive/slow query, using |
Hi guys, I updated to the version
3.30.2
and when I try to dofindAndCountAll
with include, the count query is wrong, example:I get:
expected:
if I have a product with more than 2 features, it will be counted twice
Sequelize:
3.30.2
dialect:
pg
The text was updated successfully, but these errors were encountered: