Skip to content
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

Update find or create by #35633

Closed

Conversation

alexcameron89
Copy link
Member

Summary

I've come around to the idea that find_or_create_or_find_by is a better behavior in our case (though I'd never suggest rails having that method).

I'd like to consider making that the behaviour of find_or_create_by, if you're interested in PRing it.

To me the ideal would be that they both have similar[ly rare] failure scenarios, and the choice of find_or_create_by vs create_or_find_by would boil down to whether you, on average, expect the row to exist.. and thus which one is more likely to save you a query.

I believe these are the changes needed to make find_or_create_by fail in similar ways to create_or_find_by, but I'm struggling to find an appropriate way to validate that through tests.

@rails-bot
Copy link

rails-bot bot commented Dec 18, 2019

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.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 18, 2019
@ghiculescu
Copy link
Member

@alexcameron89 was there ever any more discussion on this one? Just read over #35543 and this does still feel like a good idea.

ChanChar added a commit to ChanChar/rails that referenced this pull request Apr 13, 2022
Updates `#create_or_find_by` to accept an optional `options` key to
house a new `find_first` flag. This flag specifies that the method runs
a `#find_by` before attempting the create.

This can help mitigate a few potential issues:
* (Impact) Description
* (Medium) Reduces write load. In most cases, this method is useful to
  create a record once with all subsequent calls using `#find_by` as a
fallback. The proactive `#find_by` can skip the create/rescue loop and
mostly rely on the initial fetch.
* (Low) Reduces the risk of unneccesary PK increments when an existing record
  exists (ie. some versions of MySQL increment the PK on INSERT,
regardless of whether or not it succeeds). References rails#35543

[Previous implementation](rails#35633)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants