-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Allow limit queries without random ordering #12598
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
base: 4.20
Are you sure you want to change the base?
Conversation
framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java
Outdated
Show resolved
Hide resolved
5acff5b to
0ec2ced
Compare
0ec2ced to
b57892b
Compare
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
@sureshanaparti , what are we using this for? It is a framework level change without usage, I am really wondering. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12598 +/- ##
=========================================
Coverage 16.26% 16.27%
- Complexity 13429 13432 +3
=========================================
Files 5661 5661
Lines 500010 500014 +4
Branches 60715 60716 +1
=========================================
+ Hits 81331 81361 +30
+ Misses 409606 409579 -27
- Partials 9073 9074 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
random sorting may not be required in all cases. It requires the use of a temporary table for the sort, which is taking more time for a simple lookup by name of a VM. |
| if (randomize) { | ||
| _orderBy = " ORDER BY RAND()" ; | ||
| _limit = limit; | ||
| } else { | ||
| _limit = limit; | ||
| } |
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.
| if (randomize) { | |
| _orderBy = " ORDER BY RAND()" ; | |
| _limit = limit; | |
| } else { | |
| _limit = limit; | |
| } | |
| _limit = limit; | |
| if (randomize) { | |
| _orderBy = " ORDER BY RAND()" ; | |
| } |
@DaanHoogland there are some usages for it. I used an alternative approach in 02d8f18 for batch expunge. That one line in |
|
@sureshanaparti btw, I think that it may be good to change the usages of |
|
ok, guys, I see how it could have been useful for instance in @winterhazel ’s change. In this PR however it is not used at all. Are we planning to …? |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16714 |
Description
This PR allows the limit queries without random ordering.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?