Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Only use LOWER for mysql uniqueness check when column is case sensitive #561

Closed
wants to merge 1 commit into from

5 participants

@jpalermo

This addresses this two year old ticket:
https://rails.lighthouseapp.com/projects/8994/tickets/2503

I have one comment in the code looking for suggestions on how to reduce the number of arguments to the new method:
jpalermo@47e6809#commitcomment-383890

@metavida

Thanks for bringing this ticket back to life! I'll do my best to find time to look over your patch.

@rheaton

Merge this in!

@jake3030 jake3030 referenced this pull request from a commit in jake3030/rails
@samoli samoli Added prompt options to date helpers [#561 state:resolved]
Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
389534c
@jake3030 jake3030 referenced this pull request from a commit in jake3030/rails
@samoli samoli Use I18n for date/time select helpers prompt text [#561 state:resolved]
Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
70456ae
@jake3030 jake3030 referenced this pull request from a commit in jake3030/rails
@samoli samoli Stops date select helpers from defaulting the selected date to today …
…if :prompt option has been used

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#561 state:resolved]
f04346d
@jeremy
Owner

Interesting. Thanks for the implementation and tests, @jpalermo. This would be a nice improvement for 3.2. Could you rebase against master?

Joseph Palermo Only use LOWER for mysql case insensitive uniqueness check when colum…
…n has a case sensitive collation.
01ae984
@jpalermo

Rebase against master is done.

@jeremy
Owner
@jeremy jeremy closed this
@kenn

@jpalermo Thank you for this patch!

How do you define a collation on a column in migration, though? Maybe I'm missing something, but I've been doing this:

execute 'ALTER TABLE users MODIFY `token` VARCHAR(32) CHARACTER SET utf8 COLLATE utf8_bin'

While what I'd really like to do instead is:

create_table :users do |t|
  t.string :token, limit: 32, collation: 'utf8_bin'
end

But it doesn't seem possible.

@jpalermo

Yeah, column definitions don't have an option for collation.
http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/TableDefinition.html#method-i-column

It's in a totally unrelated part of the code compared to this patch.

You can see the column definition code in ActiveRecord::ConnectionAdapters::column

There is not a lot of code there, so it could be easily expanded. Of course, you might need to touch code in the individual connection adapters.

You'll probably want to search old issues and tickets to see if this has been attempted before. It could be that it was attempted and rejected previously.

@kenn

Hmm okay, thanks for the info anyway! I'll try to write a patch myself and hopefully turn into a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 9, 2011
  1. Only use LOWER for mysql case insensitive uniqueness check when colum…

    Joseph Palermo authored
    …n has a case sensitive collation.
This page is out of date. Refresh to see the latest.
View
4 activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -238,6 +238,10 @@ def case_sensitive_modifier(node)
node
end
+ def case_insensitive_comparison(table, attribute, column, value)
+ table[attribute].lower.eq(table.lower(value))
+ end
+
def current_savepoint_name
"active_record_#{open_transactions}"
end
View
28 activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -4,6 +4,14 @@ module ActiveRecord
module ConnectionAdapters
class AbstractMysqlAdapter < AbstractAdapter
class Column < ConnectionAdapters::Column # :nodoc:
+
+ attr_reader :collation
+
+ def initialize(name, default, sql_type = nil, null = true, collation = nil)
+ super(name, default, sql_type, null)
+ @collation = collation
+ end
+
def extract_default(default)
if sql_type =~ /blob/i || type == :text
if default.blank?
@@ -28,6 +36,10 @@ def adapter
raise NotImplementedError
end
+ def case_sensitive?
+ collation && !collation.match(/_ci$/)
+ end
+
private
def simplified_type(field_type)
@@ -157,8 +169,8 @@ def each_hash(result) # :nodoc:
end
# Overridden by the adapters to instantiate their specific Column type.
- def new_column(field, default, type, null) # :nodoc:
- Column.new(field, default, type, null)
+ def new_column(field, default, type, null, collation) # :nodoc:
+ Column.new(field, default, type, null, collation)
end
# Must return the Mysql error number from the exception, if the exception has an
@@ -393,10 +405,10 @@ def indexes(table_name, name = nil) #:nodoc:
# Returns an array of +Column+ objects for the table specified by +table_name+.
def columns(table_name, name = nil)#:nodoc:
- sql = "SHOW FIELDS FROM #{quote_table_name(table_name)}"
+ sql = "SHOW FULL FIELDS FROM #{quote_table_name(table_name)}"
execute_and_free(sql, 'SCHEMA') do |result|
each_hash(result).map do |field|
- new_column(field[:Field], field[:Default], field[:Type], field[:Null] == "YES")
+ new_column(field[:Field], field[:Default], field[:Type], field[:Null] == "YES", field[:Collation])
end
end
end
@@ -501,6 +513,14 @@ def case_sensitive_modifier(node)
Arel::Nodes::Bin.new(node)
end
+ def case_insensitive_comparison(table, attribute, column, value)
+ if column.case_sensitive?
+ super
+ else
+ table[attribute].eq(value)
+ end
+ end
+
def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key)
where_sql
end
View
4 activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
@@ -47,8 +47,8 @@ def each_hash(result) # :nodoc:
end
end
- def new_column(field, default, type, null) # :nodoc:
- Column.new(field, default, type, null)
+ def new_column(field, default, type, null, collation) # :nodoc:
+ Column.new(field, default, type, null, collation)
end
def error_number(exception)
View
4 activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -150,8 +150,8 @@ def each_hash(result) # :nodoc:
end
end
- def new_column(field, default, type, null) # :nodoc:
- Column.new(field, default, type, null)
+ def new_column(field, default, type, null, collation) # :nodoc:
+ Column.new(field, default, type, null, collation)
end
def error_number(exception) # :nodoc:
View
4 activerecord/lib/active_record/validations/uniqueness.rb
@@ -57,8 +57,8 @@ def build_relation(klass, table, attribute, value) #:nodoc:
value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s if column.text?
if !options[:case_sensitive] && value && column.text?
- # will use SQL LOWER function before comparison
- relation = table[attribute].lower.eq(table.lower(value))
+ # will use SQL LOWER function before comparison, unless it detects a case insensitive collation
+ relation = klass.connection.case_insensitive_comparison(table, attribute, column, value)
else
value = klass.connection.case_sensitive_modifier(value)
relation = table[attribute].eq(value)
View
38 activerecord/test/cases/adapters/mysql/case_sensitivity_test.rb
@@ -0,0 +1,38 @@
+require "cases/helper"
+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
+
+ 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
+ end
+
+ def test_case_sensitive
+ assert !CollationTest.columns_hash['string_ci_column'].case_sensitive?
+ assert CollationTest.columns_hash['string_cs_column'].case_sensitive?
+ end
+
+ def test_case_insensitive_comparison_for_ci_column
+ 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_no_match(/lower/i, ci_uniqueness_query)
+ end
+
+ def test_case_insensitive_comparison_for_cs_column
+ 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
+
+
+end
View
38 activerecord/test/cases/adapters/mysql2/case_sensitivity_test.rb
@@ -0,0 +1,38 @@
+require "cases/helper"
+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
+
+ 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
+ end
+
+ def test_case_sensitive
+ assert !CollationTest.columns_hash['string_ci_column'].case_sensitive?
+ assert CollationTest.columns_hash['string_cs_column'].case_sensitive?
+ end
+
+ def test_case_insensitive_comparison_for_ci_column
+ 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_no_match(/lower/i, ci_uniqueness_query)
+ end
+
+ def test_case_insensitive_comparison_for_cs_column
+ 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
+
+
+end
View
13 activerecord/test/schema/mysql2_specific_schema.rb
@@ -21,4 +21,15 @@
END
SQL
-end
+ ActiveRecord::Base.connection.execute <<-SQL
+DROP TABLE IF EXISTS collation_tests;
+SQL
+
+ ActiveRecord::Base.connection.execute <<-SQL
+CREATE TABLE collation_tests (
+ string_cs_column VARCHAR(1) COLLATE utf8_bin,
+ string_ci_column VARCHAR(1) COLLATE utf8_general_ci
+) CHARACTER SET utf8 COLLATE utf8_general_ci
+SQL
+
+end
View
11 activerecord/test/schema/mysql_specific_schema.rb
@@ -32,4 +32,15 @@
END
SQL
+ ActiveRecord::Base.connection.execute <<-SQL
+DROP TABLE IF EXISTS collation_tests;
+SQL
+
+ ActiveRecord::Base.connection.execute <<-SQL
+CREATE TABLE collation_tests (
+ string_cs_column VARCHAR(1) COLLATE utf8_bin,
+ string_ci_column VARCHAR(1) COLLATE utf8_general_ci
+) CHARACTER SET utf8 COLLATE utf8_general_ci
+SQL
+
end
Something went wrong with that request. Please try again.