3

I have the following code:

rating = user.recipe_ratings.where(:recipe_id => recipe.id).where(:delivery_id => delivery.id).first_or_create

Yet somehow we get occasional PG::Error: ERROR: duplicate key value violates unique constraint errors from this. I can't think of any reason that should happen, since the whole point of first_or_create is to prevent those.

Is this just a crazy race-condition? How can I solve this without a maddening series of begin...rescue blocks?

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
AlexQueue
  • 6,353
  • 5
  • 35
  • 44
  • 2
    Don't use `first_or_create` at all, do it by hand. Even your own version that did the naive SELECT/INSERT inside an exception handling wrapper would be better than AR's `first_or_create`. – mu is too short Aug 19 '14 at 01:37

2 Answers2

5

This seems to stem from a typical race condition for the "SELECT or INSERT" case.

Ruby seems to choose performance over safety in its implementation. Quoting "Ruby on Rails Guides":

The first_or_create method checks whether first returns nil or not. If it does return nil, then create is called.

...

The SQL generated by this method looks like this:

SELECT * FROM clients WHERE (clients.first_name = 'Andy') LIMIT 1
BEGIN
INSERT INTO clients (created_at, first_name, locked, orders_count, updated_at)
VALUES ('2011-08-30 05:22:57', 'Andy', 0, NULL, '2011-08-30 05:22:57')
COMMIT

If that's the actual implementation (?), it seems completely open for race conditions. Another transaction can easily SELECT between the first transaction's SELECT and INSERT. And then try its own INSERT, which would raise the error you reported, since the first transaction has inserted the row in the meantime.

The time frame for a race condition could be drastically reduced with a data-modifying CTE. Even a safe version would not cost that much more. But I guess they have their reasons.
Compare this safe implementation:

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
2

Rails 6 adds a new create_or_find_by method that alleviates a possible race condition, with a few drawbacks:

  • The underlying table must have the relevant columns defined with unique constraints.
  • A unique constraint violation may be triggered by only one, or at least less than all, of the given attributes. This means that the subsequent find_by! may fail to find a matching record, which will then raise an ActiveRecord::RecordNotFound exception, rather than a record with the given attributes.
  • While we avoid the race condition between SELECT -> INSERT from find_or_create_by, we actually have another race condition between INSERT -> SELECT, which can be triggered if a DELETE between those two statements is run by another client. But for most applications, that's a significantly less likely condition to hit.
  • It relies on exception handling to handle control flow, which may be marginally slower.
def create_or_find_by(attributes, &block)
  transaction(requires_new: true) { create(attributes, &block) }
rescue ActiveRecord::RecordNotUnique
  find_by!(attributes)
end

Using your example:

rating = user.recipe_ratings.create_or_find_by(
  recipe_id: recipe.id,
  delivery_id: delivery.id
)
AKN
  • 196
  • 3
  • 13