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
Use take
instead of first
in ActiveRecord::Relation#first_or_create
#25173
Conversation
…ate`, `#first_or_create!` and `#first_or_initialize` `first` adds an order by clause to query that may make it slower.
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @eileencodes (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
@rafaelfranca @sgrif given they're private, shouldn't we rename them to match the new implementation? If we want to backport to 5-0-stable as a bug fix, I agree we should keep the existing name there. |
They're private, but we know they have pretty wide usage. I think renaming them would be too much unnecessary breakage. |
How about add |
I don't think we should be removing On Sat, May 28, 2016 at 9:42 AM Guilherme Goettems Schneider <
|
Any further thoughts on this change? Just ran into it and discovered the ORDER BY defeated our index on the fields in the |
@sgrif I'm not sure which change you were suggesting above. If we're not prepared to rename |
Just to note, these methods were documented in Rails 3.2. So many code bases still depend on them (not to mention developers). What about adding new |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
@rafaelfranca @matthewd I still think this is relevant. I think going with new methods It makes sense for me to keep Order is usually unnecessary from my experience on |
This change was suggested in discussion of #24847.
first
adds an order by clause to query that may make it slower.