search

lykahb / groundhog

This library maps datatypes to a relational model, in a way similar to what ORM libraries do in OOP. See the tutorial https://www.schoolofhaskell.com/user/lykahb/groundhog for introduction
http://hackage.haskell.org/package/groundhog
176 stars 39 forks source link

insertByAll race condition? #64

Open MichaelXavier opened 7 years ago

MichaelXavier commented 7 years ago

I implemented some Haskell logic based on the docs of insertByAll saying if there was a constraint violation that it would return Left and then an arbitrary id matching the constraint. I assumed it was rescuing the db-specific error code for a constraint violation. Instead it looks like its selecting ids that match the constraint (no doubt necessary because the db vendor's error message won't have this info). I think the problem here is there is a race condition between the check and the insert:

  1. Thread 1 selects ids matching constraints, it gets none.
  2. Thread 2 selects ids matching constraints, it gets none.
  3. Thread 2 inserts and commits the transaction.
  4. Thread 1 tries to insert, but gets a unique constraint violation. The transaction rolls back and rather than returning a conflicting ID it throws a postgres (in my case) exception.

I'm not certain what the correct solution is here but this feels like it violates the spirit of the documentation. I'd expect not to ever receive a constraint exception fron this function.

In one option you could just insert blindly, rescue a uniqueness error (which means you can't fully get away with one RDBMS-agnostic insertByAll) and then issue your conflicting id query. Problem here is will abort the larger transaction and I'm not familiar enough with transaction semantics in groundhog to know if this is okay.

In another option, you create a subtransaction for the insert. You could still check, as the postgres IRC channel seems to believe the subtransaction is expensive enough that you'd want to optimally avoid it with the pre-insert check. You'd still have to catch the constraint violation.

There may be other solutions, such as this crazy upsert in postgresql.

andrewthad commented 7 years ago

I just got bit by this as well. At the least, the docs should be improved to indicate that this function has a race condition.