aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2019-02-21 17:30:27 +0900
committerRyuta Kamizono <kamipo@gmail.com>2019-03-04 21:00:03 +0900
commit9def05385f1cfa41924bb93daa187615e88c95b9 (patch)
tree32ff637ba806db7bb15b32abf99b685fb2bae525 /activerecord
parentcbedbdef0708e00968d66feab89c86a40f2292e0 (diff)
downloadrails-9def05385f1cfa41924bb93daa187615e88c95b9.tar.gz
rails-9def05385f1cfa41924bb93daa187615e88c95b9.tar.bz2
rails-9def05385f1cfa41924bb93daa187615e88c95b9.zip
Deprecate mismatched collation comparison for uniquness validator
In MySQL, the default collation is case insensitive. Since the uniqueness validator enforces case sensitive comparison by default, it frequently causes mismatched collation issues (performance, weird behavior, etc) to MySQL users. https://grosser.it/2009/12/11/validates_uniqness_of-mysql-slow/ https://github.com/rails/rails/issues/1399 https://github.com/rails/rails/pull/13465 https://github.com/gitlabhq/gitlabhq/commit/c1dddf8c7d947691729f6d64a8ea768b5c915855 https://github.com/huginn/huginn/pull/1330#discussion_r55152573 I'd like to deprecate the implicit default enforcing since I frequently experienced the problems in code reviews. Note that this change has no effect to sqlite3, postgresql, and oracle-enhanced adapters which are implemented as case sensitive by default, only affect to mysql2 adapter (I can take a work if sqlserver adapter will support Rails 6.0).
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md8
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract_adapter.rb2
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb14
-rw-r--r--activerecord/lib/active_record/validations/uniqueness.rb2
-rw-r--r--activerecord/test/cases/validations/uniqueness_validation_test.rb45
-rw-r--r--activerecord/test/schema/schema.rb27
6 files changed, 86 insertions, 12 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 42c0e6a190..ec412c2a9d 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,11 @@
+* Deprecate mismatched collation comparison for uniquness validator.
+
+ Uniqueness validator will no longer enforce case sensitive comparison in Rails 6.1.
+ To continue case sensitive comparison on the case insensitive column,
+ pass `case_sensitive: true` option explicitly to the uniqueness validator.
+
+ *Ryuta Kamizono*
+
* Add `reselect` method. This is a short-hand for `unscope(:select).select(fields)`.
Fixes #27340.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index f205d77ddb..823efee301 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -506,7 +506,7 @@ module ActiveRecord
@connection
end
- def default_uniqueness_comparison(attribute, value) # :nodoc:
+ def default_uniqueness_comparison(attribute, value, klass) # :nodoc:
case_sensitive_comparison(attribute, value)
end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 37506a97e2..96f902792e 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -453,6 +453,20 @@ module ActiveRecord
SQL
end
+ def default_uniqueness_comparison(attribute, value, klass) # :nodoc:
+ column = column_for_attribute(attribute)
+
+ if column.collation && !column.case_sensitive?
+ ActiveSupport::Deprecation.warn(<<~MSG.squish)
+ Uniqueness validator will no longer enforce case sensitive comparison in Rails 6.1.
+ To continue case sensitive comparison on the :#{attribute.name} attribute in #{klass} model,
+ pass `case_sensitive: true` option explicitly to the uniqueness validator.
+ MSG
+ end
+
+ super
+ end
+
def case_sensitive_comparison(attribute, value) # :nodoc:
column = column_for_attribute(attribute)
diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb
index 88ae62c681..dc1be73a60 100644
--- a/activerecord/lib/active_record/validations/uniqueness.rb
+++ b/activerecord/lib/active_record/validations/uniqueness.rb
@@ -63,7 +63,7 @@ module ActiveRecord
if bind.nil?
attr.eq(bind)
elsif !options.key?(:case_sensitive)
- klass.connection.default_uniqueness_comparison(attr, bind)
+ klass.connection.default_uniqueness_comparison(attr, bind, klass)
elsif options[:case_sensitive]
klass.connection.case_sensitive_comparison(attr, bind)
else
diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb
index 8f6f47e5fb..301e552642 100644
--- a/activerecord/test/cases/validations/uniqueness_validation_test.rb
+++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb
@@ -314,6 +314,51 @@ class UniquenessValidationTest < ActiveRecord::TestCase
assert t3.save, "Should save t3 as unique"
end
+ if current_adapter?(:Mysql2Adapter)
+ def test_deprecate_validate_uniqueness_mismatched_collation
+ Topic.validates_uniqueness_of(:author_email_address)
+
+ topic1 = Topic.new(author_email_address: "david@loudthinking.com")
+ topic2 = Topic.new(author_email_address: "David@loudthinking.com")
+
+ assert_equal 1, Topic.where(author_email_address: "david@loudthinking.com").count
+
+ assert_deprecated do
+ assert_not topic1.valid?
+ assert_not topic1.save
+ assert topic2.valid?
+ assert topic2.save
+ end
+
+ assert_equal 2, Topic.where(author_email_address: "david@loudthinking.com").count
+ assert_equal 2, Topic.where(author_email_address: "David@loudthinking.com").count
+ end
+ end
+
+ def test_validate_case_sensitive_uniqueness_by_default
+ Topic.validates_uniqueness_of(:author_email_address)
+
+ topic1 = Topic.new(author_email_address: "david@loudthinking.com")
+ topic2 = Topic.new(author_email_address: "David@loudthinking.com")
+
+ assert_equal 1, Topic.where(author_email_address: "david@loudthinking.com").count
+
+ ActiveSupport::Deprecation.silence do
+ assert_not topic1.valid?
+ assert_not topic1.save
+ assert topic2.valid?
+ assert topic2.save
+ end
+
+ if current_adapter?(:Mysql2Adapter)
+ assert_equal 2, Topic.where(author_email_address: "david@loudthinking.com").count
+ assert_equal 2, Topic.where(author_email_address: "David@loudthinking.com").count
+ else
+ assert_equal 1, Topic.where(author_email_address: "david@loudthinking.com").count
+ assert_equal 1, Topic.where(author_email_address: "David@loudthinking.com").count
+ end
+ end
+
def test_validate_case_sensitive_uniqueness
Topic.validates_uniqueness_of(:title, case_sensitive: true, allow_nil: true)
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 1dfea0f506..5d46bd47d9 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -8,6 +8,13 @@ ActiveRecord::Schema.define do
# #
# ------------------------------------------------------------------- #
+ case_sensitive_options =
+ if current_adapter?(:Mysql2Adapter)
+ { collation: "utf8mb4_bin" }
+ else
+ {}
+ end
+
create_table :accounts, force: true do |t|
t.references :firm, index: false
t.string :firm_name
@@ -266,7 +273,7 @@ ActiveRecord::Schema.define do
end
create_table :dashboards, force: true, id: false do |t|
- t.string :dashboard_id
+ t.string :dashboard_id, **case_sensitive_options
t.string :name
end
@@ -330,7 +337,7 @@ ActiveRecord::Schema.define do
end
create_table :essays, force: true do |t|
- t.string :name
+ t.string :name, **case_sensitive_options
t.string :writer_id
t.string :writer_type
t.string :category_id
@@ -338,7 +345,7 @@ ActiveRecord::Schema.define do
end
create_table :events, force: true do |t|
- t.string :title, limit: 5
+ t.string :title, limit: 5, **case_sensitive_options
end
create_table :eyes, force: true do |t|
@@ -380,7 +387,7 @@ ActiveRecord::Schema.define do
end
create_table :guids, force: true do |t|
- t.column :key, :string
+ t.column :key, :string, **case_sensitive_options
end
create_table :guitars, force: true do |t|
@@ -388,8 +395,8 @@ ActiveRecord::Schema.define do
end
create_table :inept_wizards, force: true do |t|
- t.column :name, :string, null: false
- t.column :city, :string, null: false
+ t.column :name, :string, null: false, **case_sensitive_options
+ t.column :city, :string, null: false, **case_sensitive_options
t.column :type, :string
end
@@ -876,8 +883,8 @@ ActiveRecord::Schema.define do
end
create_table :topics, force: true do |t|
- t.string :title, limit: 250
- t.string :author_name
+ t.string :title, limit: 250, **case_sensitive_options
+ t.string :author_name, **case_sensitive_options
t.string :author_email_address
if subsecond_precision_supported?
t.datetime :written_on, precision: 6
@@ -889,10 +896,10 @@ ActiveRecord::Schema.define do
# use VARCHAR2(4000) instead of CLOB datatype as CLOB data type has many limitations in
# Oracle SELECT WHERE clause which causes many unit test failures
if current_adapter?(:OracleAdapter)
- t.string :content, limit: 4000
+ t.string :content, limit: 4000, **case_sensitive_options
t.string :important, limit: 4000
else
- t.text :content
+ t.text :content, **case_sensitive_options
t.text :important
end
t.boolean :approved, default: true