From 16629c099ca30895c90661d5cbce40e218f0984e Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Sat, 7 Feb 2015 17:23:30 -0700 Subject: [PATCH] rm `Type#text?` This predicate was only to figure out if it's safe to do case insensitive comparison, which is only a problem on PG. Turns out, PG can just tell us whether we are able to do it or not. If the query turns out to be a problem, let's just replace that method with checking the SQL type for `text` or `character`. I'd rather not burden the type objects with adapter specific knowledge. The *real* solution, is to deprecate this behavior entirely. The only reason we need it is because the `:case_sensitive` option for `validates_uniqueness_of` is documented as "this option is ignored for non-strings". It makes no sense for us to do that. If the type can't be compared in a case insensitive way, the user shouldn't tell us to do case insensitive comparison. --- .../connection_adapters/abstract_adapter.rb | 11 ++++++++++- .../postgresql/oid/specialized_string.rb | 4 ---- .../connection_adapters/postgresql_adapter.rb | 18 ++++++++++++++++++ activerecord/lib/active_record/type/string.rb | 4 ---- activerecord/lib/active_record/type/value.rb | 4 ---- .../active_record/validations/uniqueness.rb | 2 +- 6 files changed, 29 insertions(+), 14 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 67c8f438e2..9392bcb473 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -367,9 +367,18 @@ module ActiveRecord end def case_insensitive_comparison(table, attribute, column, value) - table[attribute].lower.eq(table.lower(value)) + if can_perform_case_insensitive_comparison_for?(column) + table[attribute].lower.eq(table.lower(value)) + else + case_sensitive_comparison(table, attribute, column, value) + end end + def can_perform_case_insensitive_comparison_for?(column) + true + end + private :can_perform_case_insensitive_comparison_for? + def current_savepoint_name current_transaction.savepoint_name end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/specialized_string.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/specialized_string.rb index b2a42e9ebb..2d2fede4e8 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/specialized_string.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/specialized_string.rb @@ -8,10 +8,6 @@ module ActiveRecord def initialize(type) @type = type end - - def text? - false - end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index ab970e183a..e8ecaffcab 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -762,6 +762,24 @@ module ActiveRecord def create_table_definition(name, temporary = false, options = nil, as = nil) # :nodoc: PostgreSQL::TableDefinition.new native_database_types, name, temporary, options, as end + + def can_perform_case_insensitive_comparison_for?(column) + @case_insensitive_cache ||= {} + @case_insensitive_cache[column.sql_type] ||= begin + sql = <<-end_sql + SELECT exists( + SELECT * FROM pg_proc + INNER JOIN pg_cast + ON casttarget::text::oidvector = proargtypes + WHERE proname = 'lower' + AND castsource = '#{column.sql_type}'::regtype::oid + ) + end_sql + execute_and_clear(sql, "SCHEMA", []) do |result| + result.getvalue(0, 0) == 't' + end + end + end end end end diff --git a/activerecord/lib/active_record/type/string.rb b/activerecord/lib/active_record/type/string.rb index cf95e25be0..fbc0af2c5a 100644 --- a/activerecord/lib/active_record/type/string.rb +++ b/activerecord/lib/active_record/type/string.rb @@ -21,10 +21,6 @@ module ActiveRecord end end - def text? - true - end - private def cast_value(value) diff --git a/activerecord/lib/active_record/type/value.rb b/activerecord/lib/active_record/type/value.rb index 9027925072..7f384bee68 100644 --- a/activerecord/lib/active_record/type/value.rb +++ b/activerecord/lib/active_record/type/value.rb @@ -52,10 +52,6 @@ module ActiveRecord # These predicates are not documented, as I need to look further into # their use, and see if they can be removed entirely. - def text? # :nodoc: - false - end - def number? # :nodoc: false end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index ad56f637e3..a766d77e88 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -69,7 +69,7 @@ module ActiveRecord value = Arel::Nodes::Quoted.new(value) - comparison = if !options[:case_sensitive] && value && cast_type.text? + comparison = if !options[:case_sensitive] && !value.nil? # will use SQL LOWER function before comparison, unless it detects a case insensitive collation klass.connection.case_insensitive_comparison(table, attribute, column, value) else