-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
New find_or_create_by behaviour raises RecordNotFound in edge rails #48035
Comments
Yes, transactions are the reasons. Here is what happened.
We need to somehow read the up to date data for this row without restarting a transaction. This is achieved by So, I come up with the following: diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 2e21c22849..f53a94efed 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -215,7 +215,7 @@ def find_or_create_by!(attributes, &block)
def create_or_find_by(attributes, &block)
transaction(requires_new: true) { create(attributes, &block) }
rescue ActiveRecord::RecordNotUnique
- find_by!(attributes)
+ where(attributes).lock.find_by!(attributes)
end
# Like #create_or_find_by, but calls
@@ -224,7 +224,7 @@ def create_or_find_by(attributes, &block)
def create_or_find_by!(attributes, &block)
transaction(requires_new: true) { create!(attributes, &block) }
rescue ActiveRecord::RecordNotUnique
- find_by!(attributes)
+ where(attributes).lock.find_by!(attributes)
end
# Like #find_or_create_by, but calls {new}[rdoc-ref:Core#new] If that looks ok and I am not missing something, I can open a PR. |
Ahh, thanks - I couldn't figure out the REPEATABLE READ step. The patch looks promising... though my actual code ends up doing a second lookup of the tag during a subsequent validation, which ends up raising RecordNotFound again, again due to REPEATABLE READ 😬. I'll see if I can avoid the extra lookup somehow |
Yep, works well once I've removed the unnecessary lookup |
Hum, should we only do that if we're inside a transaction? Feel free to open a PR we can discuss it there. |
Steps to reproduce
Expected behavior
In Rails 7.0, one of the
find_or_create_by
calls raises RecordNotUnique (which we rescue and handle at the application-level).Actual behavior
In 7.1 alpha, it raises RecordNotFound:
Backtrace
though, apparently, only when there's a transaction wrapping the find_or_create call. Without the transaction, the new find_or_create_by improvements successfully return a tag in both threads.
(I'd happily remove our rescue block and leave Rails to handle the unique keys internally, if find_or_create can be fixed to do so)
cc @casperisfine
System configuration
Rails version: 7.1.0 alpha @ 719558c
Ruby version: 3.1.3
The text was updated successfully, but these errors were encountered: