Error in validates_uniqueness_of docs?

Hi,

[My apologies if this isn't the right list for this post.]

The documentation for validates_uniqueness_of says that the validation
can't protect against races that would result in duplicate insertions,
even if the DB is in serializable mode. I don't think this is true:
the SQL serializable isolation mode is supposed to ensure that
transactions appear to have a well-defined execution order. If all
transactions appear to execute in a specific order, than doing a
SELECT to check for the existence of a key followed by an UPDATE
should be enough to protect against duplicate keys.

You can test the race shown at
http://api.rubyonrails.org/classes/ActiveRecord/Validations/ClassMethods.html#M002167
using MySQL 5 to see that serializable isolation does actually prevent
duplicate keys:

[Session A]
create table comments (title varchar(100), content varchar(100))
engine InnoDB;
set transaction isolation level serializable;
begin;
select * from comments where title = "My Post";

[Session B]
set transaction isolation level serializable;
begin;
select * from comments where title = "My Post";

[Session A]
insert into comments (title, content) values ("My Post", "hi!");
[NOTE: We begin to wait on a lock here]

[Session B]
insert into comments (title, content) values ("My Post", "hello!");
[NOTE: Deadlock detected, Session B's transaction is aborted]

[Session A]
[insert has unblocked]
commit;

We can get the same protection even in the default repeatable read
isolation mode by explicitly taking out a lock during the select. So,
with this patch, validates_uniqueness_of should work correctly even if
the DB is running only in repeatable read:

diff --git a/activerecord/lib/active_record/validations/uniqueness.rb
b/activerecord/lib/active_record/validations/uniqueness.rb
index 711086d..bc0f38c 100644
--- a/activerecord/lib/active_record/validations/uniqueness.rb
+++ b/activerecord/lib/active_record/validations/uniqueness.rb
@@ -148,7 +148,7 @@ module ActiveRecord
           end

           finder_class.with_exclusive_scope do
- if finder_class.exists?([condition_sql,
*condition_params])
+ if finder_class.find(:first, :conditions =>
[condition_sql, *condition_params], :lock => true)
               record.errors.add(attr_name, :taken, :default =>
configuration[:message], :value => value)
             end
           end

Actually, all we need is a shared lock, which on MySQL you could get
with :lock => "lock in share mode". Unfortunately, Postgres seems to
have a different syntax, and there's no abstraction for shared locks
in the DB adapters.

-- Andrew

Unfortunately, this is a pointless endeavor. It's easier to prevent
duplicate entries using a unique index. In both the unique index and
your method, the error is raised by the database when the insert is
attempted.

The only reason to use a validation is to report a nice validation
error message before attempting the insert. Your method doesn't do
that, because both select statements return no rows.

Jeremy

Yes.

The patch also keeps the row locked indefinitely, even if we later on
decide not to update the row (e.g. because other validations fail).
Using a unique index is better.