From 24cbb66a638a4e6080ea3fd45eebd349eba62c5d Mon Sep 17 00:00:00 2001 From: Chris Conley Date: Tue, 23 Nov 2010 15:12:43 -0500 Subject: [PATCH 1/2] Fixed MysqlAdapter#column_exists? to return correct result when sending in the :limit option. --- .../abstract/schema_statements.rb | 6 ++++- .../connection_adapters/mysql_adapter.rb | 11 ++++++++ activerecord/test/cases/migration_test.rb | 26 ++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 4e770c3..bd69ccf 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -66,11 +66,15 @@ module ActiveRecord def column_exists?(table_name, column_name, type = nil, options = {}) columns(table_name).any?{ |c| c.name == column_name.to_s && (!type || c.type == type) && - (!options[:limit] || c.limit == options[:limit]) && + (!options[:limit] || limit_exists?(c.limit, options[:limit])) && (!options[:precision] || c.precision == options[:precision]) && (!options[:scale] || c.scale == options[:scale]) } end + def limit_exists?(existing_limit, new_limit) + existing_limit == new_limit + end + # Creates a new table with the name +table_name+. +table_name+ may either # be a String or a Symbol. # diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index ce23524..7839ef3 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -533,6 +533,17 @@ module ActiveRecord columns end + def limit_exists?(existing_limit, new_limit) + case new_limit + when 5..8 + existing_limit == 8 + when nil, 4, 11 + existing_limit == 4 + else + existing_limit == new_limit + end + end + def create_table(table_name, options = {}) #:nodoc: super(table_name, options.reverse_merge(:options => "ENGINE=InnoDB")) end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 3037d73..b2dcbd4 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -1073,6 +1073,32 @@ if ActiveRecord::Base.connection.supports_migrations? Person.connection.drop_table :testings rescue nil end + def test_column_exists_with_integer_limit + Person.connection.create_table :testings do |t| + t.column :one, :integer, :limit => 1 + t.column :two, :integer, :limit => 2 + t.column :three, :integer, :limit => 3 + t.column :four, :integer, :limit => 4 + t.column :five, :integer, :limit => 5 + t.column :six, :integer, :limit => 6 + t.column :seven, :integer, :limit => 7 + t.column :eight, :integer, :limit => 8 + t.column :eleven, :integer, :limit => 11 + end + + assert Person.connection.column_exists?(:testings, :one, :integer, :limit => 1) + assert Person.connection.column_exists?(:testings, :two, :integer, :limit => 2) + assert Person.connection.column_exists?(:testings, :three, :integer, :limit => 3) + assert Person.connection.column_exists?(:testings, :four, :integer, :limit => 4) + assert Person.connection.column_exists?(:testings, :five, :integer, :limit => 5) + assert Person.connection.column_exists?(:testings, :six, :integer, :limit => 6) + assert Person.connection.column_exists?(:testings, :seven, :integer, :limit => 7) + assert Person.connection.column_exists?(:testings, :eight, :integer, :limit => 8) + assert Person.connection.column_exists?(:testings, :eleven, :integer, :limit => 11) + ensure + Person.connection.drop_table :testings rescue nil + end + def test_add_table assert !Reminder.table_exists? -- 1.7.2.2 From ca5ffff32b7268b376dbec44c64f70cb49faaf52 Mon Sep 17 00:00:00 2001 From: Chris Conley Date: Tue, 23 Nov 2010 16:18:20 -0500 Subject: [PATCH 2/2] Moved #limit_exists? to Column and made sure we are only doing anything special for type "integer". [#6049] --- .../abstract/schema_definitions.rb | 4 +++ .../abstract/schema_statements.rb | 6 +---- .../connection_adapters/mysql_adapter.rb | 23 ++++++++++--------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 60ccf9e..a34fa70 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -50,6 +50,10 @@ module ActiveRecord !default.nil? end + def limit_exists?(new_limit) + self.limit == new_limit + end + # Returns the Ruby class that corresponds to the abstract data type. def klass case type diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index bd69ccf..31c6904 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -66,15 +66,11 @@ module ActiveRecord def column_exists?(table_name, column_name, type = nil, options = {}) columns(table_name).any?{ |c| c.name == column_name.to_s && (!type || c.type == type) && - (!options[:limit] || limit_exists?(c.limit, options[:limit])) && + (!options[:limit] || c.limit_exists?(options[:limit])) && (!options[:precision] || c.precision == options[:precision]) && (!options[:scale] || c.scale == options[:scale]) } end - def limit_exists?(existing_limit, new_limit) - existing_limit == new_limit - end - # Creates a new table with the name +table_name+. +table_name+ may either # be a String or a Symbol. # diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 7839ef3..fe8b526 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -92,6 +92,18 @@ module ActiveRecord super end + def limit_exists?(new_limit) + return super unless type.to_s == 'integer' + case new_limit + when 5..8 + self.limit == 8 + when nil, 4, 11 + self.limit == 4 + else + self.limit == new_limit + end + end + private def simplified_type(field_type) return :boolean if MysqlAdapter.emulate_booleans && field_type.downcase.index("tinyint(1)") @@ -533,17 +545,6 @@ module ActiveRecord columns end - def limit_exists?(existing_limit, new_limit) - case new_limit - when 5..8 - existing_limit == 8 - when nil, 4, 11 - existing_limit == 4 - else - existing_limit == new_limit - end - end - def create_table(table_name, options = {}) #:nodoc: super(table_name, options.reverse_merge(:options => "ENGINE=InnoDB")) end -- 1.7.2.2