Modify change_table to remove the need for the block argument.

2 files changed, 64 insertions, 7 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 be9a02682b..11da84e245 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -241,14 +241,19 @@ module ActiveRecord
       #
       # See also Table for details on
       # all of the various column transformation
-      def change_table(table_name, options = {})
-        if supports_bulk_alter? && options[:bulk]
-          recorder = ActiveRecord::Migration::CommandRecorder.new(self)
-          yield Table.new(table_name, recorder)
-          bulk_change_table(table_name, recorder.commands)
-        else
-          yield Table.new(table_name, self)
+      def change_table(table_name, options = {}, &blk)
+        bulk_change = supports_bulk_alter? && options[:bulk]
+        recorder = bulk_change ? ActiveRecord::Migration::CommandRecorder.new(self) : self
+        table = Table.new(table_name, recorder)
+
+        if block_given?
+          if blk.arity == 1
+            yield table
+          else
+            table.instance_eval(&blk)
+          end
         end
+        bulk_change_table(table_name, recorder.commands) if bulk_change
       end
 
       # Renames a table.
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 50242a497c..e276993508 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -1757,6 +1757,58 @@ if ActiveRecord::Base.connection.supports_migrations?
         end
       end
     end
+
+    def test_change_table_without_block_parameter_no_bulk
+      Person.connection.create_table :testings, :force => true do
+        string :foo
+      end
+      assert Person.connection.column_exists?(:testings, :foo, :string)
+
+      Person.connection.change_table :testings do
+        remove :foo
+        integer :bar
+      end
+
+      assert_equal %w(bar id), Person.connection.columns(:testings).map { |c| c.name }.sort
+    ensure
+      Person.connection.drop_table :testings rescue nil
+    end
+
+    def test_change_table_without_block_parameter_with_bulk
+      Person.connection.create_table :testings, :force => true do
+        string :foo
+      end
+      assert Person.connection.column_exists?(:testings, :foo, :string)
+
+      assert_queries(1) do
+        Person.connection.change_table(:testings, :bulk => true) do
+          integer :bar
+          string :foo_bar
+        end
+      end
+
+      assert_equal %w(bar foo foo_bar id), Person.connection.columns(:testings).map { |c| c.name }.sort
+    ensure
+      Person.connection.drop_table :testings rescue nil
+    end
+
+    def test_change_table_should_not_have_mixed_syntax
+      Person.connection.create_table :testings, :force => true do
+        string :foo
+      end
+      assert_raise(NoMethodError) do
+        Person.connection.change_table :testings do |t|
+          t.remove :foo
+          integer :bar
+        end
+      end
+      assert_raise(NameError) do
+        Person.connection.change_table :testings do
+          t.remove :foo
+          integer :bar
+        end
+      end
+    end
   end # SexierMigrationsTest
 
   class MigrationLoggerTest < ActiveRecord::TestCase