From b337390889cb4a9f80ed08daf072a043f0e7ddf3 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Fri, 1 Mar 2013 11:39:39 +0100 Subject: [PATCH] transactions can be turned off per Migration. Closes #9483. There are SQL Queries that can't run inside a transaction. Since the Migrator used to wrap all Migrations inside a transaction there was no way to run these queries within a migration. This patch adds `self.disable_ddl_transaction!` to the migration to turn transactions off when necessary. --- activerecord/CHANGELOG.md | 15 +++++++ activerecord/lib/active_record/migration.rb | 44 ++++++++++++++++--- .../test/cases/migration/logger_test.rb | 2 +- activerecord/test/cases/migration_test.rb | 32 ++++++++++++++ guides/source/migrations.md | 6 ++- 5 files changed, 90 insertions(+), 9 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 97616ffc58524..a0ca36788df66 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,20 @@ ## Rails 4.0.0 (unreleased) ## +* Make it possible to execute migrations without a transaction even + if the database adapter supports DDL transactions. + Fixes #9483. + + Example: + + class ChangeEnum < ActiveRecord::Migration + self.disable_ddl_transaction! + def up + execute "ALTER TYPE model_size ADD VALUE 'new_value'" + end + end + + *Yves Senn* + * Assigning "0.0" to a nullable numeric column does not make it dirty. Fix #9034. diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 823595a1281b3..62e8881c4c76c 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -330,6 +330,23 @@ def initialize # # For a list of commands that are reversible, please see # ActiveRecord::Migration::CommandRecorder. + # + # == Transactional Migrations + # + # If the database adapter supports DDL transactions, all migrations will + # automatically be wrapped in a transaction. There are queries that you + # can't execute inside a transaction though, and for these situations + # you can turn the automatic transactions off. + # + # class ChangeEnum < ActiveRecord::Migration + # self.disable_ddl_transaction! + # def up + # execute "ALTER TYPE model_size ADD VALUE 'new_value'" + # end + # end + # + # Remember that you can still open your own transactions, even if you + # are in a Migration with self.disable_ddl_transaction!. class Migration autoload :CommandRecorder, 'active_record/migration/command_recorder' @@ -351,6 +368,7 @@ def call(env) class << self attr_accessor :delegate # :nodoc: + attr_accessor :disable_ddl_transaction # :nodoc: end def self.check_pending! @@ -365,8 +383,16 @@ def self.migrate(direction) new.migrate direction end - cattr_accessor :verbose + # Disable DDL transactions for this migration. + def self.disable_ddl_transaction! + @disable_ddl_transaction = true + end + + def disable_ddl_transaction # :nodoc: + self.class.disable_ddl_transaction + end + cattr_accessor :verbose attr_accessor :name, :version def initialize(name = self.class.name, version = nil) @@ -375,8 +401,8 @@ def initialize(name = self.class.name, version = nil) @connection = nil end + self.verbose = true # instantiate the delegate object after initialize is defined - self.verbose = true self.delegate = new # Reverses the migration commands for the given block and @@ -663,7 +689,7 @@ def basename File.basename(filename) end - delegate :migrate, :announce, :write, :to => :migration + delegate :migrate, :announce, :write, :disable_ddl_transaction, to: :migration private @@ -856,12 +882,12 @@ def migrate Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger begin - ddl_transaction do + ddl_transaction(migration) do migration.migrate(@direction) record_version_state_after_migrating(migration.version) end rescue => e - canceled_msg = Base.connection.supports_ddl_transactions? ? "this and " : "" + canceled_msg = use_transaction?(migration) ? "this and " : "" raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace end end @@ -935,12 +961,16 @@ def down? end # Wrap the migration in a transaction only if supported by the adapter. - def ddl_transaction - if Base.connection.supports_ddl_transactions? + def ddl_transaction(migration) + if use_transaction?(migration) Base.transaction { yield } else yield end end + + def use_transaction?(migration) + !migration.disable_ddl_transaction && Base.connection.supports_ddl_transactions? + end end end diff --git a/activerecord/test/cases/migration/logger_test.rb b/activerecord/test/cases/migration/logger_test.rb index ee0c20747e0de..97efb94b667de 100644 --- a/activerecord/test/cases/migration/logger_test.rb +++ b/activerecord/test/cases/migration/logger_test.rb @@ -7,6 +7,7 @@ class LoggerTest < ActiveRecord::TestCase self.use_transactional_fixtures = false Migration = Struct.new(:name, :version) do + def disable_ddl_transaction; false end def migrate direction # do nothing end @@ -34,4 +35,3 @@ def test_migration_should_be_run_without_logger end end end - diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 831b2ee2b4bea..960d28fcf5b31 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -254,7 +254,39 @@ def migrate(x) assert_equal "An error has occurred, this and all later migrations canceled:\n\nSomething broke", e.message Person.reset_column_information + assert_not Person.column_methods_hash.include?(:last_name), + "On error, the Migrator should revert schema changes but it did not." + end + + def test_migration_without_transaction + unless ActiveRecord::Base.connection.supports_ddl_transactions? + skip "not supported on #{ActiveRecord::Base.connection.class}" + end + assert_not Person.column_methods_hash.include?(:last_name) + + migration = Class.new(ActiveRecord::Migration) { + self.disable_ddl_transaction! + + def version; 101 end + def migrate(x) + add_column "people", "last_name", :string + raise 'Something broke' + end + }.new + + migrator = ActiveRecord::Migrator.new(:up, [migration], 101) + e = assert_raise(StandardError) { migrator.migrate } + assert_equal "An error has occurred, all later migrations canceled:\n\nSomething broke", e.message + + Person.reset_column_information + assert Person.column_methods_hash.include?(:last_name), + "without ddl transactions, the Migrator should not rollback on error but it did." + ensure + Person.reset_column_information + if Person.column_methods_hash.include?(:last_name) + Person.connection.remove_column('people', 'last_name') + end end def test_schema_migrations_table_name diff --git a/guides/source/migrations.md b/guides/source/migrations.md index 89ae564c24750..bd63970beab74 100644 --- a/guides/source/migrations.md +++ b/guides/source/migrations.md @@ -61,6 +61,10 @@ migrations are wrapped in a transaction. If the database does not support this then when a migration fails the parts of it that succeeded will not be rolled back. You will have to rollback the changes that were made by hand. +NOTE: There are certain queries that can't run inside a transaction. If your +adapter supports DDL transactions you can use `disable_ddl_transaction!` to +disable them for a single migration. + If you wish for a migration to do something that Active Record doesn't know how to reverse, you can use `reversible`: @@ -180,7 +184,7 @@ end ``` If the migration name is of the form "CreateXXX" and is -followed by a list of column names and types then a migration creating the table +followed by a list of column names and types then a migration creating the table XXX with the columns listed will be generated. For example: ```bash