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

#create_or_find_by: find_first option #44885

Closed
wants to merge 1 commit into from

Conversation

ChanChar
Copy link

Summary

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.

Other Information

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 Using create_or_find_by led to primary keys running out #35543

Previous implementation / inspiration

Open questions for reviewers

  • Is there a better pattern for "options" in AR relation methods?
    • The methods already accept kwargs by default and Ruby doesn't easily allow for multiple hashes as args
      (ie. def method(hash_one, hash_two)
  • Should there be validations against options? ie. options: { foo: :bar} raises an error.

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)
@rafaelfranca
Copy link
Member

How is that different from find_or_create_by?

@ChanChar
Copy link
Author

ChanChar commented Apr 13, 2022

Great question, this change bridges the gap between#find_or_create_by & #create_or_find_by. It's more like a #find_or_create_or_find_by.

#find_or_create_by runs the risk of race conditions between the SELECT & INSERT whereas #create_or_find_by runs (imo) the INSERT -> rescue -> SELECT too often.

#find_or_create_by (provided there's an uniqueness constraint) could also be modified to behave in a similar way to #create_or_find_by:

def find_or_create_by_with_uniqueness_handling!
  Record.find_or_create_by!(attrs)
rescue ActiveRecord::RecordNotUnique
  find_by!(attrs)
end

Behaviorally, this would allow the logic to "optimize" for reads in most cases and skip the create/rescue loop. In the worst case (ie. the record does exist but was missed in the first SELECT), it defaults to creating/rescuing.

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