Made migrations transactional for PostgreSQL [#834 state:resolved]

Patch originally from http://dev.rubyonrails.org/ticket/5470

6 files changed, 59 insertions, 4 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 90be9b700a..74a1239aeb 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *Edge*
 
+* Transactional migrations for databases which support them.  #834 [divoxx, Adam Wiggins, Tarmo Tänav]
+
 * Set config.active_record.timestamped_migrations = false to have migrations with numeric prefix instead of UTC timestamp. #446. [Andrew Stone, Nik Wakelin]
 
 * change_column_default preserves the not-null constraint.  #617 [Tarmo Tänav]
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 47dbf5a5f3..6924bb7e6f 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -51,6 +51,13 @@ module ActiveRecord
         true
       end
 
+      # Does this adapter support DDL rollbacks in transactions?  That is, would
+      # CREATE TABLE or ALTER TABLE get rolled back by a transaction?  PostgreSQL,
+      # SQL Server, and others support this.  MySQL and others do not.
+      def supports_ddl_transactions?
+        false
+      end
+
       # Should primary key values be selected from their corresponding
       # sequence before the insert statement?  If true, next_sequence_value
       # is called before each insert to set the record's primary key.
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 723e1a42dd..bc6fd4e722 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -335,6 +335,10 @@ module ActiveRecord
         postgresql_version >= 80200
       end
 
+      def supports_ddl_transactions?
+        true
+      end
+
       # Returns the configured supported identifier length supported by PostgreSQL,
       # or report the default of 63 on PostgreSQL 7.x.
       def table_alias_length
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index fd77f27b77..c7bc76264d 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -461,14 +461,22 @@ module ActiveRecord
         Base.logger.info "Migrating to #{migration.name} (#{migration.version})"
 
         # On our way up, we skip migrating the ones we've already migrated
-        # On our way down, we skip reverting the ones we've never migrated
         next if up? && migrated.include?(migration.version.to_i)
 
+        # On our way down, we skip reverting the ones we've never migrated
         if down? && !migrated.include?(migration.version.to_i)
           migration.announce 'never migrated, skipping'; migration.write
-        else
-          migration.migrate(@direction)
-          record_version_state_after_migrating(migration.version)
+          next
+        end
+
+        begin
+          ddl_transaction do
+            migration.migrate(@direction)
+            record_version_state_after_migrating(migration.version)
+          end
+        rescue => e
+          canceled_msg = Base.connection.supports_ddl_transactions? ? "this and " : ""
+          raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace
         end
       end
     end
@@ -531,5 +539,14 @@ module ActiveRecord
       def down?
         @direction == :down
       end
+
+      # Wrap the migration in a transaction only if supported by the adapter.
+      def ddl_transaction(&block)
+        if Base.connection.supports_ddl_transactions?
+          Base.transaction { block.call }
+        else
+          block.call
+        end
+      end
   end
 end
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index b3e6b29165..9639588ef2 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -937,6 +937,21 @@ if ActiveRecord::Base.connection.supports_migrations?
       assert_equal(0, ActiveRecord::Migrator.current_version)
     end
 
+    if current_adapter?(:PostgreSQLAdapter)
+      def test_migrator_one_up_with_exception_and_rollback
+        assert !Person.column_methods_hash.include?(:last_name)
+
+        e = assert_raises(StandardError) do
+          ActiveRecord::Migrator.up(MIGRATIONS_ROOT + "/broken", 100)
+        end
+
+        assert_equal "An error has occurred, this and all later migrations canceled:\n\nSomething broke", e.message
+
+        Person.reset_column_information
+        assert !Person.column_methods_hash.include?(:last_name)
+      end
+    end
+
     def test_finds_migrations
       migrations = ActiveRecord::Migrator.new(:up, MIGRATIONS_ROOT + "/valid").migrations
       [['1', 'people_have_last_names'],
diff --git a/activerecord/test/migrations/broken/100_migration_that_raises_exception.rb b/activerecord/test/migrations/broken/100_migration_that_raises_exception.rb
new file mode 100644
index 0000000000..ffb224dad9
--- /dev/null
+++ b/activerecord/test/migrations/broken/100_migration_that_raises_exception.rb
@@ -0,0 +1,10 @@
+class MigrationThatRaisesException < ActiveRecord::Migration
+  def self.up
+    add_column "people", "last_name", :string
+    raise 'Something broke'
+  end
+
+  def self.down
+    remove_column "people", "last_name"
+  end
+end