diff --git a/README.md b/README.md index 6ae1acb..ac6f5bb 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,35 @@ class BackportPublishedDefaultOnPosts < ActiveRecord::Migration end ``` +### Adding Foreign Keys + +#### Bad + +Adding a foreign key causes an `ShareRowExclusiveLock` (or `AccessExclusiveLock` on PostgreSQL before 9.5) on both tables. +It is possible to add a foreign key in one step and validate later (which only causes RowShareLocks) + +```ruby +class AddUsersForeignKeyToOrder < ActiveRecord::Migration + add_foreign_key :users, :order +end +``` + +#### Good + +Add the foreign key and validate in another migration: + +```ruby +class AddUsersForeignKeyToOrder < ActiveRecord::Migration + add_foreign_key :users, :order, validate: false +end + +class ValidateUsersForeignKeyToOrder < ActiveRecord::Migration + validate_foreign_key :users, :order +end +``` + +Note, both `add_foreign_key` and `validate_foreign_key` accept `name` and `column` options. + ### TODO * Changing a column type diff --git a/lib/zero_downtime_migrations.rb b/lib/zero_downtime_migrations.rb index bd329fe..e362805 100644 --- a/lib/zero_downtime_migrations.rb +++ b/lib/zero_downtime_migrations.rb @@ -8,6 +8,7 @@ require_relative "zero_downtime_migrations/validation" require_relative "zero_downtime_migrations/validation/add_column" require_relative "zero_downtime_migrations/validation/add_index" +require_relative "zero_downtime_migrations/validation/add_foreign_key" require_relative "zero_downtime_migrations/validation/ddl_migration" require_relative "zero_downtime_migrations/validation/find_each" require_relative "zero_downtime_migrations/validation/mixed_migration" diff --git a/lib/zero_downtime_migrations/validation/add_foreign_key.rb b/lib/zero_downtime_migrations/validation/add_foreign_key.rb new file mode 100644 index 0000000..e22f4b9 --- /dev/null +++ b/lib/zero_downtime_migrations/validation/add_foreign_key.rb @@ -0,0 +1,54 @@ +module ZeroDowntimeMigrations + class Validation + class AddForeignKey < Validation + def validate! + return unless requires_validate_constraints? + error!(message) unless foreign_key_not_validated? + end + + private + + def message + <<-MESSAGE.strip_heredoc + Adding a foreign key causes an `ShareRowExclusiveLock` (or `AccessExclusiveLock` on PostgreSQL before 9.5) on both tables. + It is possible to add a foreign key in one step and validate later (which only causes RowShareLocks) + + class Add#{foreign_table}ForeignKeyTo#{table} < ActiveRecord::Migration + def change + add_foreign_key :#{table}, #{foreign_table}, validate: false + end + end + + class Validate#{foreign_table}ForeignKeyOn#{table} < ActiveRecord::Migration + def change + validate_foreign_key :#{table}, :#{foreign_table} + end + end + + Note, both `add_foreign_key` and `validate_foreign_key` accept `name` and `column` options. + MESSAGE + end + + def foreign_key_not_validated? + options[:validate] == false + end + + def foreign_table + args[1] + end + + def table + args[0] + end + + def requires_validate_constraints? + supports_validate_constraints? + end + + def supports_validate_constraints? + ActiveRecord::Base.connection.respond_to?(:supports_validate_constraints?) && + ActiveRecord::Base.connection.supports_validate_constraints? + end + end + end +end diff --git a/spec/zero_downtime_migrations/validation/add_foreign_key_spec.rb b/spec/zero_downtime_migrations/validation/add_foreign_key_spec.rb new file mode 100644 index 0000000..f20338b --- /dev/null +++ b/spec/zero_downtime_migrations/validation/add_foreign_key_spec.rb @@ -0,0 +1,77 @@ +RSpec.describe ZeroDowntimeMigrations::Validation::AddForeignKey do + let(:error) { ZeroDowntimeMigrations::UnsafeMigrationError } + + context "when the database supports_validate_constraints" do + context "adding a foreign key and not specifying validate" do + let(:migration) do + Class.new(ActiveRecord::Migration[5.0]) do + def change + create_table :order + add_column :users, :order_id, :integer + add_foreign_key :users, :order + end + end + end + + it "raises an unsafe migration error" do + expect { migration.migrate(:up) }.to raise_error(error) + end + end + + context "adding a foreign key and specifying validate true" do + let(:migration) do + Class.new(ActiveRecord::Migration[5.0]) do + def change + create_table :order + add_column :users, :order_id, :integer + add_foreign_key :users, :order, validate: true + end + end + end + + it "raises an unsafe migration error" do + expect { migration.migrate(:up) }.to raise_error(error) + end + end + + context "adding a foreign key and specifying validate false" do + let(:migration) do + Class.new(ActiveRecord::Migration[5.0]) do + def change + create_table :order + add_column :users, :order_id, :integer + add_foreign_key :users, :order, validate: false + end + end + end + + it "raises nothing" do + expect { migration.migrate(:up) }.to_not raise_error + end + end + end + + context "when the database does not supports_validate_constraints" do + before do + allow_any_instance_of(ActiveRecord::ConnectionAdapters::PostgreSQLAdapter).to receive( + :supports_validate_constraints? + ).and_return(false) + end + + context "adding a foreign key and not specifying validate" do + let(:migration) do + Class.new(ActiveRecord::Migration[5.0]) do + def change + create_table :order + add_column :users, :order_id, :integer + add_foreign_key :users, :order + end + end + end + + it "raises nothing" do + expect { migration.migrate(:up) }.to_not raise_error + end + end + end +end