From 111990513b20c045d4af08b20f07f98c7e2c0b92 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Thu, 22 May 2014 13:10:25 -0700 Subject: [PATCH] Push limit to type objects Columns and injected types no longer have any conditionals based on the format of SQL type strings! Hooray! --- .../connection_adapters/abstract_adapter.rb | 43 ++++++++++----- .../abstract_mysql_adapter.rb | 55 +++++++------------ .../connection_adapters/column.rb | 10 +--- .../connection_adapters/postgresql/column.rb | 9 --- .../connection_adapters/postgresql_adapter.rb | 12 +++- .../connection_adapters/type/value.rb | 5 +- .../test/cases/column_definition_test.rb | 6 +- 7 files changed, 67 insertions(+), 73 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index f3162419c7..ea7703c82c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -374,21 +374,23 @@ module ActiveRecord end def initialize_type_map(m) # :nodoc: - m.register_type %r(boolean)i, Type::Boolean.new - m.register_type %r(char)i, Type::String.new - m.register_type %r(binary)i, Type::Binary.new - m.alias_type %r(blob)i, 'binary' - m.register_type %r(text)i, Type::Text.new - m.alias_type %r(clob)i, 'text' - m.register_type %r(date)i, Type::Date.new - m.register_type %r(time)i, Type::Time.new - m.alias_type %r(timestamp)i, 'datetime' - m.register_type %r(datetime)i, Type::DateTime.new - m.alias_type %r(numeric)i, 'decimal' - m.alias_type %r(number)i, 'decimal' - m.register_type %r(float)i, Type::Float.new - m.alias_type %r(double)i, 'float' - m.register_type %r(int)i, Type::Integer.new + register_class_with_limit m, %r(boolean)i, Type::Boolean + register_class_with_limit m, %r(char)i, Type::String + register_class_with_limit m, %r(binary)i, Type::Binary + register_class_with_limit m, %r(text)i, Type::Text + register_class_with_limit m, %r(date)i, Type::Date + register_class_with_limit m, %r(time)i, Type::Time + register_class_with_limit m, %r(datetime)i, Type::DateTime + register_class_with_limit m, %r(float)i, Type::Float + register_class_with_limit m, %r(int)i, Type::Integer + + m.alias_type %r(blob)i, 'binary' + m.alias_type %r(clob)i, 'text' + m.alias_type %r(timestamp)i, 'datetime' + m.alias_type %r(numeric)i, 'decimal' + m.alias_type %r(number)i, 'decimal' + m.alias_type %r(double)i, 'float' + m.register_type(%r(decimal)i) do |sql_type| scale = extract_scale(sql_type) precision = extract_precision(sql_type) @@ -406,6 +408,13 @@ module ActiveRecord initialize_type_map(type_map) end + def register_class_with_limit(mapping, key, klass) # :nodoc: + mapping.register_type(key) do |*args| + limit = extract_limit(args.last) + klass.new(limit: limit) + end + end + def extract_scale(sql_type) # :nodoc: case sql_type when /\((\d+)\)/ then 0 @@ -417,6 +426,10 @@ module ActiveRecord $1.to_i if sql_type =~ /\((\d+)(,\d+)?\)/ end + def extract_limit(sql_type) # :nodoc: + $1.to_i if sql_type =~ /\((.*)\)/ + end + def translate_exception_class(e, sql) message = "#{e.class.name}: #{e.message}: #{sql}" @logger.error message if @logger 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 86eb2a38d8..d3f8470c30 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -86,44 +86,12 @@ module ActiveRecord sql_type =~ /blob/i || type == :text end - # Must return the relevant concrete adapter - def adapter - raise NotImplementedError - end - def case_sensitive? collation && !collation.match(/_ci$/) end private - def extract_limit(sql_type) - case sql_type - when /^enum\((.+)\)/i - $1.split(',').map{|enum| enum.strip.length - 2}.max - when /blob|text/i - case sql_type - when /tiny/i - 255 - when /medium/i - 16777215 - when /long/i - 2147483647 # mysql only allows 2^31-1, not 2^32-1, somewhat inconsistently with the tiny/medium/normal cases - else - super # we could return 65535 here, but we leave it undecorated by default - end - when /^bigint/i; 8 - when /^int/i; 4 - when /^mediumint/i; 3 - when /^smallint/i; 2 - when /^tinyint/i; 1 - when /^float/i; 24 - when /^double/i; 53 - else - super - end - end - # MySQL misreports NOT NULL column default when none is given. # We can't detect this for columns which may have a legitimate '' # default (string) but we can for others (integer, datetime, boolean, @@ -636,10 +604,29 @@ module ActiveRecord protected - def initialize_type_map(m) + def initialize_type_map(m) # :nodoc: super + m.register_type(%r(enum)i) do |sql_type| + limit = sql_type[/^enum\((.+)\)/i, 1] + .split(',').map{|enum| enum.strip.length - 2}.max + Type::String.new(limit: limit) + end + + m.register_type %r(tinytext)i, Type::Text.new(limit: 255) + m.register_type %r(tinyblob)i, Type::Binary.new(limit: 255) + m.register_type %r(mediumtext)i, Type::Text.new(limit: 16777215) + m.register_type %r(mediumblob)i, Type::Binary.new(limit: 16777215) + m.register_type %r(longtext)i, Type::Text.new(limit: 2147483647) + m.register_type %r(longblob)i, Type::Binary.new(limit: 2147483647) + m.register_type %r(^bigint)i, Type::Integer.new(limit: 8) + m.register_type %r(^int)i, Type::Integer.new(limit: 4) + m.register_type %r(^mediumint)i, Type::Integer.new(limit: 3) + m.register_type %r(^smallint)i, Type::Integer.new(limit: 2) + m.register_type %r(^tinyint)i, Type::Integer.new(limit: 1) + m.register_type %r(^float)i, Type::Float.new(limit: 24) + m.register_type %r(^double)i, Type::Float.new(limit: 53) + m.alias_type %r(tinyint\(1\))i, 'boolean' if emulate_booleans - m.alias_type %r(enum)i, 'varchar' m.alias_type %r(set)i, 'varchar' m.alias_type %r(year)i, 'integer' m.alias_type %r(bit)i, 'binary' diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index 10ea25748d..704868c058 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -13,12 +13,12 @@ module ActiveRecord ISO_DATETIME = /\A(\d{4})-(\d\d)-(\d\d) (\d\d):(\d\d):(\d\d)(\.\d+)?\z/ end - attr_reader :name, :default, :cast_type, :limit, :null, :sql_type, :default_function + attr_reader :name, :default, :cast_type, :null, :sql_type, :default_function attr_accessor :primary, :coder alias :encoded? :coder - delegate :type, :precision, :scale, :klass, :text?, :number?, :binary?, :type_cast_for_write, to: :cast_type + delegate :type, :precision, :scale, :limit, :klass, :text?, :number?, :binary?, :type_cast_for_write, to: :cast_type # Instantiates a new column in the table. # @@ -34,7 +34,6 @@ module ActiveRecord @cast_type = cast_type @sql_type = sql_type @null = null - @limit = extract_limit(sql_type) @default = extract_default(default) @default_function = nil @primary = nil @@ -65,11 +64,6 @@ module ActiveRecord def extract_default(default) type_cast(default) end - - private - def extract_limit(sql_type) - $1.to_i if sql_type =~ /\((.*)\)/ - end end end # :startdoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb index 302eb8a114..b55766bde0 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb @@ -110,15 +110,6 @@ module ActiveRecord def has_default_function?(default_value, default) !default_value && (%r{\w+\(.*\)} === default) end - - def extract_limit(sql_type) - case sql_type - when /^bigint/i; 8 - when /^smallint/i; 2 - when /^timestamp/i; nil - else super - end - 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 5b7240b463..e3a2422160 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -569,14 +569,14 @@ module ActiveRecord end def initialize_type_map(m) - m.register_type 'int2', OID::Integer.new + register_class_with_limit m, 'int2', OID::Integer m.alias_type 'int4', 'int2' m.alias_type 'int8', 'int2' m.alias_type 'oid', 'int2' m.register_type 'float4', OID::Float.new m.alias_type 'float8', 'float4' m.register_type 'text', Type::Text.new - m.register_type 'varchar', Type::String.new + register_class_with_limit m, 'varchar', Type::String m.alias_type 'char', 'varchar' m.alias_type 'name', 'varchar' m.alias_type 'bpchar', 'varchar' @@ -629,6 +629,14 @@ module ActiveRecord load_additional_types(m) end + def extract_limit(sql_type) # :nodoc: + case sql_type + when /^bigint/i; 8 + when /^smallint/i; 2 + else super + end + end + def load_additional_types(type_map, oids = nil) if supports_ranges? query = <<-SQL diff --git a/activerecord/lib/active_record/connection_adapters/type/value.rb b/activerecord/lib/active_record/connection_adapters/type/value.rb index 787b51dfa4..54a3e9dd7a 100644 --- a/activerecord/lib/active_record/connection_adapters/type/value.rb +++ b/activerecord/lib/active_record/connection_adapters/type/value.rb @@ -2,12 +2,13 @@ module ActiveRecord module ConnectionAdapters module Type class Value # :nodoc: - attr_reader :precision, :scale + attr_reader :precision, :scale, :limit def initialize(options = {}) - options.assert_valid_keys(:precision, :scale) + options.assert_valid_keys(:precision, :scale, :limit) @precision = options[:precision] @scale = options[:scale] + @limit = options[:limit] end def type; end diff --git a/activerecord/test/cases/column_definition_test.rb b/activerecord/test/cases/column_definition_test.rb index 23e6254577..45e48900ee 100644 --- a/activerecord/test/cases/column_definition_test.rb +++ b/activerecord/test/cases/column_definition_test.rb @@ -34,7 +34,7 @@ module ActiveRecord # Avoid column definitions in create table statements like: # `title` varchar(255) DEFAULT NULL def test_should_not_include_default_clause_when_default_is_null - column = Column.new("title", nil, Type::String.new, "varchar(20)") + column = Column.new("title", nil, Type::String.new(limit: 20)) column_def = ColumnDefinition.new( column.name, "string", column.limit, column.precision, column.scale, column.default, column.null) @@ -42,7 +42,7 @@ module ActiveRecord end def test_should_include_default_clause_when_default_is_present - column = Column.new("title", "Hello", Type::String.new, "varchar(20)") + column = Column.new("title", "Hello", Type::String.new(limit: 20)) column_def = ColumnDefinition.new( column.name, "string", column.limit, column.precision, column.scale, column.default, column.null) @@ -50,7 +50,7 @@ module ActiveRecord end def test_should_specify_not_null_if_null_option_is_false - column = Column.new("title", "Hello", Type::String.new, "varchar(20)", false) + column = Column.new("title", "Hello", Type::String.new(limit: 20), "varchar(20)", false) column_def = ColumnDefinition.new( column.name, "string", column.limit, column.precision, column.scale, column.default, column.null)