Skip to content

Commit

Permalink
Change validates_uniqueness_of :case_sensitive option default back to…
Browse files Browse the repository at this point in the history 
… true (from [9160]). Love your database columns, don't LOWER them. [rick]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9248 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information
@technoweenie
technoweenie committed Apr 10, 2008
1 parent c67e985 commit ed99dda
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
2 changes: 2 additions & 0 deletions activerecord/CHANGELOG
View file
@@ -1,5 +1,7 @@
*SVN*

* Change validates_uniqueness_of :case_sensitive option default back to true (from [9160]). Love your database columns, don't LOWER them. [rick]

* Ensure that save on child object fails for invalid belongs_to association. Closes #11555. [rubyruy]

* Add support for interleaving migrations by storing which migrations have run in the new schema_migrations table. Closes #11493 [jordi]
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/validations.rb
View file
Expand Up @@ -603,7 +603,7 @@ def validates_length_of(*attrs)
# Configuration options:
# * <tt>message</tt> - Specifies a custom error message (default is: "has already been taken")
# * <tt>scope</tt> - One or more columns by which to limit the scope of the uniqueness constraint.
# * <tt>case_sensitive</tt> - Looks for an exact match. Ignored by non-text columns (false by default).
# * <tt>case_sensitive</tt> - Looks for an exact match. Ignored by non-text columns (true by default).
# * <tt>allow_nil</tt> - If set to true, skips this validation if the attribute is null (default is: false)
# * <tt>allow_blank</tt> - If set to true, skips this validation if the attribute is blank (default is: false)
# * <tt>if</tt> - Specifies a method, proc or string to call to determine if the validation should
Expand All @@ -613,7 +613,7 @@ def validates_length_of(*attrs)
# not occur (e.g. :unless => :skip_validation, or :unless => Proc.new { |user| user.signup_step <= 2 }). The
# method, proc or string should return or evaluate to a true or false value.
def validates_uniqueness_of(*attr_names)
configuration = { :message => ActiveRecord::Errors.default_error_messages[:taken] }
configuration = { :message => ActiveRecord::Errors.default_error_messages[:taken], :case_sensitive => true }
configuration.update(attr_names.extract_options!)

validates_each(attr_names,configuration) do |record, attr_name, value|
Expand Down

5 comments on commit ed99dda

@kemiller
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s an issue which has cropped up in this method, which is that there’s no need to LOWER, and yet MySQL is penalized in the case insensitive mode with unnecessary and utter destruction of its index utilitization. It wasn’t an issue before 2.1 because validates_uniqueness_of was effectively insensitive by default on MySQL (oops).

@remigabillet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I second kemiller’s comment. It’s a big issue on large tables with millions or rows getting scanned.

@remigabillet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a temporary fix, I overrode validations.rb at line 627 with:

```
if value.nil? || (configuration[:case_sensitive] || !is_text_column) || self.connection.class.is_a?(ActiveRecord::ConnectionAdapters::MysqlAdapter)
….
```

@remigabillet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, what I just wrote was wrong, I meant:

if value.nil? || (configuration[:case_sensitive] || !is_text_column) || self.connection.is_a?(ActiveRecord::ConnectionAdapters::MysqlAdapter)

@NZKoz
Copy link
Member

@NZKoz NZKoz commented on ed99dda Oct 19, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open a thread on the core mailing list http://groups.google.com/group/rubyonrails-core rather than discussing this here, otherwise it’s likely to get forgotten and not be fixed in the next point release.

Please sign in to comment.