-
Notifications
You must be signed in to change notification settings - Fork 90
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
feat: added take and last methods to query #867
base: master
Are you sure you want to change the base?
feat: added take and last methods to query #867
Conversation
…d when we want to apply custom order by.
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@saqijaan Please run |
Sure I'll add |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #867 +/- ##
=======================================
Coverage 67.51% 67.51%
=======================================
Files 151 151
Lines 10355 10355
=======================================
Hits 6991 6991
Misses 2996 2996
Partials 368 368 ☔ View full report in Codecov by Sentry. |
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.
LGTM, could you add some test cases in tests/queue_test.go
for the logic?
Sure, I will. I’m just adding this whenever I have time so I don’t miss this feature 😉. It caused me a major performance issue. I have a table with around 2 million records, and using first with orderBy was making the query take around 7-9 seconds. Later, when checking the query log, I found out that the issue was caused by the first method. |
@saqijaan Or try to add some indexes? |
Indexes are already there on required columns. But I don't want to order by id and this still requires me to add composite index on id and other column I'm using for ordering my records. So adding indexes is not a proper solution because it should only include columns in OrderBy which are provided by user. |
📑 Description
Its new Feature.
The First method provided by orm.Query automatically adds an ORDER BY clause. This meant that if someone wanted to retrieve a single record using a custom order, there was no built-in method available. To address this, GORM provides the Take method, which fetches a single record without applying any ORDER BY condition.
Additionally, the Last method was missing in the query, so I added it to quickly fetch the last record without requiring an ORDER BY clause. This method is also provided by GORM.
Using ORDER BY with First can lead to performance issues because it automatically includes the primary key in the ORDER BY clause along with any other fields specified for ordering. In the absence of a composite index, this can result in a significantly slow query.
@coderabbitai summary
✅ Checks