From eb589fed6f4950d441bc6aed8dfaaeffec061322 Mon Sep 17 00:00:00 2001 From: Nishant Modak Date: Mon, 6 Jan 2014 14:34:22 +0530 Subject: Make change_table use object of current database adapter - Earlier, change_table was creating database-agnostic object. - After this change, it will create correct object based on current database adapter. - This will ensure that create_table and change_table will get same objects. - This makes update_table_definition method public and nodoc. - Fixes #13577 and #13503 --- activerecord/CHANGELOG.md | 8 ++++++++ .../connection_adapters/abstract/schema_statements.rb | 10 +++++----- .../connection_adapters/postgresql_adapter.rb | 10 +++++----- .../lib/active_record/migration/command_recorder.rb | 10 ++++++---- .../test/cases/adapters/postgresql/hstore_test.rb | 17 +++++++++++++++++ .../test/cases/migration/command_recorder_test.rb | 3 ++- 6 files changed, 43 insertions(+), 15 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index efe29fbc88..add5334f39 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* `change_table` now uses the current adapter's `update_table_definition` + method to retrieve a specific table definition. + This ensures that `change_table` and `create_table` will use + similar objects. + Fixes #13577 and #13503. + + *Nishant Modak*, *Prathamesh Sonpatki*, *Rafael Mendonça França* + * Fixed ActiveRecord::Store nil conversion TypeError when using YAML coder. In case the YAML passed as paramter is nil, uses an empty string. 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 00383bad3b..88bf15bc18 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -714,7 +714,7 @@ module ActiveRecord # require the order columns appear in the SELECT. # # columns_for_distinct("posts.id", ["posts.created_at desc"]) - def columns_for_distinct(columns, orders) # :nodoc: + def columns_for_distinct(columns, orders) #:nodoc: columns end @@ -736,6 +736,10 @@ module ActiveRecord remove_column table_name, :created_at end + def update_table_definition(table_name, base) #:nodoc: + Table.new(table_name, base) + end + protected def add_index_sort_order(option_strings, column_names, options = {}) if options.is_a?(Hash) && order = options[:order] @@ -848,10 +852,6 @@ module ActiveRecord def create_alter_table(name) AlterTable.new create_table_definition(name, false, {}) end - - def update_table_definition(table_name, base) - Table.new(table_name, base) - 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 11a5eba464..7e188907e1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -721,6 +721,10 @@ module ActiveRecord !native_database_types[type].nil? end + def update_table_definition(table_name, base) #:nodoc: + Table.new(table_name, base) + end + protected # Returns the version of the connected PostgreSQL server. @@ -800,7 +804,7 @@ module ActiveRecord end end - FEATURE_NOT_SUPPORTED = "0A000" # :nodoc: + FEATURE_NOT_SUPPORTED = "0A000" #:nodoc: def exec_no_cache(sql, name, binds) log(sql, name, binds) { @connection.async_exec(sql) } @@ -990,10 +994,6 @@ module ActiveRecord def create_table_definition(name, temporary, options, as = nil) TableDefinition.new native_database_types, name, temporary, options, as end - - def update_table_definition(table_name, base) - Table.new(table_name, base) - end end end end diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 01c73be849..17300f0598 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -86,7 +86,7 @@ module ActiveRecord alias :remove_belongs_to :remove_reference def change_table(table_name, options = {}) - yield ConnectionAdapters::Table.new(table_name, self) + yield delegate.update_table_definition(table_name, self) end private @@ -159,9 +159,11 @@ module ActiveRecord # Forwards any missing method call to the \target. def method_missing(method, *args, &block) - @delegate.send(method, *args, &block) - rescue NoMethodError => e - raise e, e.message.sub(/ for #<.*$/, " via proxy for #{@delegate}") + if @delegate.respond_to?(method) + @delegate.send(method, *args, &block) + else + super + end end end end diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index 6df5d8f533..d8782f5eaa 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -70,6 +70,23 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase Hstore.reset_column_information end + def test_hstore_migration + hstore_migration = Class.new(ActiveRecord::Migration) do + def change + change_table("hstores") do |t| + t.hstore :keys + end + end + end + + hstore_migration.new.suppress_messages do + hstore_migration.migrate(:up) + assert_includes @connection.columns(:hstores).map(&:name), "keys" + hstore_migration.migrate(:down) + assert_not_includes @connection.columns(:hstores).map(&:name), "keys" + end + end + def test_cast_value_on_write x = Hstore.new tags: {"bool" => true, "number" => 5} assert_equal({"bool" => "true", "number" => "5"}, x.tags) diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 1b205d372f..35b656ee43 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -4,7 +4,8 @@ module ActiveRecord class Migration class CommandRecorderTest < ActiveRecord::TestCase def setup - @recorder = CommandRecorder.new + connection = ActiveRecord::Base.connection + @recorder = CommandRecorder.new(connection) end def test_respond_to_delegates -- cgit v1.2.3