-
Notifications
You must be signed in to change notification settings - Fork 2.2k
improve PaginatedOrders query speed by rearranging pipeline #4274
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
improve PaginatedOrders query speed by rearranging pipeline #4274
Conversation
|
The snyk-security test failed with this error: Is this something I need to fix? |
|
@zrisher thanks for this PR. The snyk failure is because this PR is coming from your fork of the project I think, and you shouldn't need to worry about that. |
|
@zrisher I'd like to get this performance improvement in the next release but it looks like you need to resolve some conflicts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not do any testing, but the code change LGTM
|
One conflict here. Would love to get this merged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need conflicts resolved
c02097c to
fee9791
Compare
fee9791 to
04d723d
Compare
|
@zenweasel Rebased to latest |
|
Still LGTM |
|
@zrisher I'm still not able to fast-forward a local branch to test this locally. Could you possibly update this again? |
|
I made a new PR for this change: #4555 |
Resolves #4271
Impact: minor
Type: performance
Issue
The query produced by
createAggregate()performs its projection before limiting the result set, and its sort can't use indices. See #4271Solution
Reorder the query.
Breaking changes
None.
Testing