Merge pull request #13040 from kamipo/case_sensitive_comparison

Only use BINARY for mysql case sensitive uniqueness check when column has a case insensitive collation.

Conflicts:
	activerecord/CHANGELOG.md

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Only use BINARY for mysql case sensitive uniqueness check when column has a case insensitive collation.
+
+    *Ryuta Kamizono*
+
 *   Support for Mysql 5.6 Fractional Seconds.
 
     *arthurnn*, *Tatsuhiko Miyagawa*

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -340,6 +340,11 @@ module ActiveRecord
         node
       end
 
+      def case_sensitive_comparison(table, attribute, column, value)
+        value = case_sensitive_modifier(value) unless value.nil?
+        table[attribute].eq(value)
+      end
+
       def case_insensitive_comparison(table, attribute, column, value)
         table[attribute].lower.eq(table.lower(value))
       end

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -592,6 +592,14 @@ module ActiveRecord
         Arel::Nodes::Bin.new(node)
       end
 
+      def case_sensitive_comparison(table, attribute, column, value)
+        if column.case_sensitive?
+          table[attribute].eq(value)
+        else
+          super
+        end
+      end
+
       def case_insensitive_comparison(table, attribute, column, value)
         if column.case_sensitive?
           super

activerecord/lib/active_record/validations/uniqueness.rb

@@ -68,8 +68,7 @@ module ActiveRecord
           # will use SQL LOWER function before comparison, unless it detects a case insensitive collation
           klass.connection.case_insensitive_comparison(table, attribute, column, value)
         else
-          value = klass.connection.case_sensitive_modifier(value) unless value.nil?
+          klass.connection.case_sensitive_comparison(table, attribute, column, value)
-          table[attribute].eq(value)
         end
       end
 

activerecord/test/cases/adapters/mysql/case_sensitivity_test.rb

@@ -3,10 +3,10 @@ require 'models/person'
 
 class MysqlCaseSensitivityTest < ActiveRecord::TestCase
   class CollationTest < ActiveRecord::Base
-    validates_uniqueness_of :string_cs_column, :case_sensitive => false
-    validates_uniqueness_of :string_ci_column, :case_sensitive => false
   end
 
+  repair_validations(CollationTest)
+
   def test_columns_include_collation_different_from_table
     assert_equal 'utf8_bin', CollationTest.columns_hash['string_cs_column'].collation
     assert_equal 'utf8_general_ci', CollationTest.columns_hash['string_ci_column'].collation
@@ -18,6 +18,7 @@ class MysqlCaseSensitivityTest < ActiveRecord::TestCase
   end
 
   def test_case_insensitive_comparison_for_ci_column
+    CollationTest.validates_uniqueness_of(:string_ci_column, :case_sensitive => false)
     CollationTest.create!(:string_ci_column => 'A')
     invalid = CollationTest.new(:string_ci_column => 'a')
     queries = assert_sql { invalid.save }
@@ -26,10 +27,29 @@ class MysqlCaseSensitivityTest < ActiveRecord::TestCase
   end
 
   def test_case_insensitive_comparison_for_cs_column
+    CollationTest.validates_uniqueness_of(:string_cs_column, :case_sensitive => false)
     CollationTest.create!(:string_cs_column => 'A')
     invalid = CollationTest.new(:string_cs_column => 'a')
     queries = assert_sql { invalid.save }
     cs_uniqueness_query = queries.detect { |q| q.match(/string_cs_column/) }
     assert_match(/lower/i, cs_uniqueness_query)
   end
+
+  def test_case_sensitive_comparison_for_ci_column
+    CollationTest.validates_uniqueness_of(:string_ci_column, :case_sensitive => true)
+    CollationTest.create!(:string_ci_column => 'A')
+    invalid = CollationTest.new(:string_ci_column => 'A')
+    queries = assert_sql { invalid.save }
+    ci_uniqueness_query = queries.detect { |q| q.match(/string_ci_column/) }
+    assert_match(/binary/i, ci_uniqueness_query)
+  end
+
+  def test_case_sensitive_comparison_for_cs_column
+    CollationTest.validates_uniqueness_of(:string_cs_column, :case_sensitive => true)
+    CollationTest.create!(:string_cs_column => 'A')
+    invalid = CollationTest.new(:string_cs_column => 'A')
+    queries = assert_sql { invalid.save }
+    cs_uniqueness_query = queries.detect { |q| q.match(/string_cs_column/) }
+    assert_no_match(/binary/i, cs_uniqueness_query)
+  end
 end

activerecord/test/cases/adapters/mysql2/case_sensitivity_test.rb

@@ -3,10 +3,10 @@ require 'models/person'
 
 class Mysql2CaseSensitivityTest < ActiveRecord::TestCase
   class CollationTest < ActiveRecord::Base
-    validates_uniqueness_of :string_cs_column, :case_sensitive => false
-    validates_uniqueness_of :string_ci_column, :case_sensitive => false
   end
 
+  repair_validations(CollationTest)
+
   def test_columns_include_collation_different_from_table
     assert_equal 'utf8_bin', CollationTest.columns_hash['string_cs_column'].collation
     assert_equal 'utf8_general_ci', CollationTest.columns_hash['string_ci_column'].collation
@@ -18,6 +18,7 @@ class Mysql2CaseSensitivityTest < ActiveRecord::TestCase
   end
 
   def test_case_insensitive_comparison_for_ci_column
+    CollationTest.validates_uniqueness_of(:string_ci_column, :case_sensitive => false)
     CollationTest.create!(:string_ci_column => 'A')
     invalid = CollationTest.new(:string_ci_column => 'a')
     queries = assert_sql { invalid.save }
@@ -26,10 +27,29 @@ class Mysql2CaseSensitivityTest < ActiveRecord::TestCase
   end
 
   def test_case_insensitive_comparison_for_cs_column
+    CollationTest.validates_uniqueness_of(:string_cs_column, :case_sensitive => false)
     CollationTest.create!(:string_cs_column => 'A')
     invalid = CollationTest.new(:string_cs_column => 'a')
     queries = assert_sql { invalid.save }
     cs_uniqueness_query = queries.detect { |q| q.match(/string_cs_column/)}
     assert_match(/lower/i, cs_uniqueness_query)
   end
+
+  def test_case_sensitive_comparison_for_ci_column
+    CollationTest.validates_uniqueness_of(:string_ci_column, :case_sensitive => true)
+    CollationTest.create!(:string_ci_column => 'A')
+    invalid = CollationTest.new(:string_ci_column => 'A')
+    queries = assert_sql { invalid.save }
+    ci_uniqueness_query = queries.detect { |q| q.match(/string_ci_column/) }
+    assert_match(/binary/i, ci_uniqueness_query)
+  end
+
+  def test_case_sensitive_comparison_for_cs_column
+    CollationTest.validates_uniqueness_of(:string_cs_column, :case_sensitive => true)
+    CollationTest.create!(:string_cs_column => 'A')
+    invalid = CollationTest.new(:string_cs_column => 'A')
+    queries = assert_sql { invalid.save }
+    cs_uniqueness_query = queries.detect { |q| q.match(/string_cs_column/) }
+    assert_no_match(/binary/i, cs_uniqueness_query)
+  end
 end