diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 7aa803c..668a98f 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -16,6 +16,7 @@ jobs: ruby-version: - '3.3' - '3.4' + - '4.0' lockfile: - 'Gemfile.rails-7.2.lock' - 'Gemfile.rails-8.0.lock' diff --git a/Gemfile.lock b/Gemfile.lock index f8c11c2..3f9477a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - activerecord-pg-extensions (0.7.1) + activerecord-pg-extensions (0.8.0) activerecord (>= 7.2, < 8.2) railties (>= 7.2, < 8.2) diff --git a/Gemfile.rails-7.2.lock b/Gemfile.rails-7.2.lock index d32098c..a97c92b 100644 --- a/Gemfile.rails-7.2.lock +++ b/Gemfile.rails-7.2.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - activerecord-pg-extensions (0.7.1) + activerecord-pg-extensions (0.8.0) activerecord (>= 7.2, < 8.2) railties (>= 7.2, < 8.2) diff --git a/Gemfile.rails-8.0.lock b/Gemfile.rails-8.0.lock index df87c06..d4c855c 100644 --- a/Gemfile.rails-8.0.lock +++ b/Gemfile.rails-8.0.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - activerecord-pg-extensions (0.7.1) + activerecord-pg-extensions (0.8.0) activerecord (>= 7.2, < 8.2) railties (>= 7.2, < 8.2) diff --git a/lib/active_record/pg_extensions/command_recorder.rb b/lib/active_record/pg_extensions/command_recorder.rb index b3882f8..9797977 100644 --- a/lib/active_record/pg_extensions/command_recorder.rb +++ b/lib/active_record/pg_extensions/command_recorder.rb @@ -8,6 +8,36 @@ def rename_constraint(table_name, old_name, new_name, **options) record(:rename_constraint, [table_name, old_name, new_name, options]) end + def change_constraint(table, constraint, **options) + record(:change_constraint, [table, constraint, options]) + end + + def change_check_constraint(table, **options) + record(:change_check_constraint, [table, options]) + end + + def change_index(table, **options) + record(:change_index, [table, options]) + end + + private + + def invert_change_index(args) + table, options = args + options ||= {} + # reverse the change by swapping :from and :to, leaving everything else intact + inverted = options.merge(from: options[:to], to: options[:from]) + [:change_index, [table, Hash.ruby2_keywords_hash(inverted)]] + end + + def invert_change_check_constraint(args) + table, options = args + options ||= {} + # reverse the change by swapping :from and :to, leaving everything else intact + inverted = options.merge(from: options[:to], to: options[:from]) + [:change_check_constraint, [table, Hash.ruby2_keywords_hash(inverted)]] + end + def invert_rename_constraint(args) table_name, old_name, new_name, options = args options ||= {} @@ -15,6 +45,95 @@ def invert_rename_constraint(args) # `rename_constraint(..., if_exists:)` rather than a positional hash [:rename_constraint, [table_name, new_name, old_name, Hash.ruby2_keywords_hash(options)]] end + + def invert_change_constraint(args) + table, constraint, options = args + options ||= {} + inverted = options.to_h do |key, value| + [key, invert_change_constraint_option(key, value)] + end + [:change_constraint, [table, constraint, Hash.ruby2_keywords_hash(inverted)]] + end + + # stock Rails carries :if_not_exists through to the inverse (remove_*) command + # unchanged, but the remove side only understands :if_exists (and vice versa); + # rewrite the existence option so the reverse migration stays idempotent + def invert_add_index(args) + change_option(args, from: :if_not_exists, to: :if_exists) + super + end + + def invert_remove_index(args) + change_option(args, from: :if_exists, to: :if_not_exists) + super + end + + def invert_add_column(args) + change_option(args, from: :if_not_exists, to: :if_exists) + super + end + + def invert_remove_column(args) + change_option(args, from: :if_exists, to: :if_not_exists) + super + end + + def invert_add_foreign_key(args) + change_option(args, from: :if_not_exists, to: :if_exists) + super + end + + def invert_remove_foreign_key(args) + change_option(args, from: :if_exists, to: :if_not_exists) + super + end + + def invert_add_reference(args) + change_reference_option(args, from: :if_not_exists, to: :if_exists) + super + end + alias_method :invert_add_belongs_to, :invert_add_reference + + def invert_remove_reference(args) + change_reference_option(args, from: :if_exists, to: :if_not_exists) + super + end + alias_method :invert_remove_belongs_to, :invert_remove_reference + + # renames an option on the trailing options hash in place, e.g. so an inverted + # command receives :if_exists where the original had :if_not_exists + def change_option(args, from:, to:) + options = args.last + return unless options.is_a?(Hash) && options.key?(from) + + options[to] = options.delete(from) + end + + # like change_option, but also rewrites the option nested inside a reference's + # `index:` hash (e.g. `add_reference(..., index: { if_not_exists: true })`) + def change_reference_option(args, from:, to:) + change_option(args, from:, to:) + options = args.last + change_option([options[:index]], from:, to:) if options.is_a?(Hash) && options[:index].is_a?(Hash) + end + + def invert_change_constraint_option(key, value) + return value if value.nil? + + case key + when :deferrable, :enforced, :inherit + return !value if [true, false].include?(value) + + raise ArgumentError, "#{key} must be true or false" + when :initially + return :immediate if value == :deferred + return :deferred if value == :immediate + + raise ArgumentError, "initially must be :deferred or :immediate" + else + raise ArgumentError, "unknown change_constraint option: #{key.inspect}" + end + end end end end diff --git a/lib/active_record/pg_extensions/pessimistic_migrations.rb b/lib/active_record/pg_extensions/pessimistic_migrations.rb index fdb1e47..25b2e30 100644 --- a/lib/active_record/pg_extensions/pessimistic_migrations.rb +++ b/lib/active_record/pg_extensions/pessimistic_migrations.rb @@ -89,20 +89,127 @@ def add_index(table_name, column_name, **options) super end - def add_check_constraint(table_name, expression, if_not_exists: false, **options) - options = check_constraint_options(table_name, expression, options) - return if if_not_exists && check_constraint_for(table_name, **options) + # Replace one check constraint with another + # + # @param table [Symbol] The table name + # @param from [String, Hash] The check constraint to be replaced. + # If a Hash is provided, it must contain an :expression key with the check constraint expression. + # Other options are merged with `options` and passed through. + # @param to [String, Hash] The new check constraint to be added. + # If a Hash is provided, it must contain an :expression key with the check constraint expression. + # Other options are merged with `options` and passed through. + # @param if_exists [true, false] If true, the entire operation should be idempotent (only + # dropping/renaming the old constraint if it exists, only adding the new constraint if it doesn't exist.) + # @param delay_validation [true, false] If true, the new check constraint will be added as NOT VALID and + # validated before removing the old check constraint. + def change_check_constraint(table, from:, to:, if_exists: false, delay_validation: false, **options) + delay_validation = false if open_transactions.positive? + options[:validate] = false if delay_validation - super + from_expression, from_options = split_arg(from, :expression, options) + to_expression, to_options = split_arg(to, :expression, options) + + new_constraint_name = check_constraint_name(table, expression: to_expression, **to_options) + # when delaying validation, we don't drop the old constraint until after the new one is validated, + # so we need to rename it if the names are the same + if delay_validation + old_constraint_name = check_constraint_name(table, expression: from_expression, **from_options) + if old_constraint_name == new_constraint_name + new_old_constraint_name = temporary_name(old_constraint_name) + unless check_constraint_exists?(table, name: new_old_constraint_name) + rename_constraint(table, old_constraint_name, new_old_constraint_name, if_exists:) + end + old_constraint_name = new_old_constraint_name + end + else + remove_check_constraint(table, from_expression, if_exists:, **from_options) + end + + add_check_constraint(table, + to_expression, + **to_options, + if_not_exists: if_exists || delay_validation, + validate: !delay_validation) + + return unless delay_validation + + validate_constraint(table, new_constraint_name) + remove_check_constraint(table, name: old_constraint_name, if_exists: true) end - if ActiveRecord.version < Gem::Version.new("7.1") - def remove_check_constraint(table_name, expression = nil, if_exists: false, **options) - options = check_constraint_options(table_name, expression, options) - return if if_exists && !check_constraint_for(table_name, **options) + # Replace one index with another + # + # @param table [Symbol] The table name + # @param from [String, Symbol, Array, Hash] The index to be replaced. + # If a Hash is provided, it must contain a :column key with the indexed column(s). + # Other options are merged with `options` and passed through. + # @param to [String, Symbol, Array, Hash] The new index to be added. + # If a Hash is provided, it must contain a :column key with the indexed column(s). + # Other options are merged with `options` and passed through. + # @param if_exists [true, false] If true, the entire operation should be idempotent (only + # dropping/renaming the old index if it exists, only adding the new index if it doesn't exist.) + # @param algorithm [Symbol, nil] If :concurrently, the new index is created concurrently and the + # old index is only removed after the new one exists. + def change_index(table, from:, to:, if_exists: false, algorithm: nil, **options) + algorithm = nil if open_transactions.positive? + concurrently = algorithm == :concurrently - super + from_column, from_options = split_arg(from, :column, options) + to_column, to_options = split_arg(to, :column, options) + + new_index_name = to_options[:name]&.to_s || index_name(table, column: to_column) + # when creating concurrently, we don't drop the old index until after the new one is created, + # so we need to rename it if the names are the same + if concurrently + old_index_name = from_options[:name]&.to_s || index_name(table, column: from_column) + if old_index_name == new_index_name + new_old_index_name = temporary_name(old_index_name) + # rename_index has no if_exists option, so guard it: skip if the old index is missing + # (only relevant when if_exists makes the whole operation idempotent) or the temp name is taken + old_missing = if_exists && !index_name_exists?(table, old_index_name) + unless old_missing || index_name_exists?(table, new_old_index_name) + rename_index(table, old_index_name, new_old_index_name) + end + old_index_name = new_old_index_name + end + else + remove_index(table, from_column, if_exists:, **from_options) end + + add_index(table, + to_column, + **to_options, + if_not_exists: if_exists || concurrently, + algorithm:) + + return unless concurrently + + remove_index(table, name: old_index_name, if_exists: true, algorithm: :concurrently) + end + + private + + # splits a from/to argument into its subject and options. If a Hash is given, the given key + # (e.g. :column or :expression) is extracted and the rest is merged with options; otherwise + # the argument is treated as the subject itself. + def split_arg(arg, key, options) + return [arg, options] unless arg.is_a?(Hash) + + arg_options = arg.merge(options) + [arg_options.delete(key), arg_options] + end + + # derives a temporary name from +name+ to rename an existing index/constraint out of the way. + # Prefers a readable "_to_be_replaced", but falls back to a hashed (still deterministic, + # still unique to +name+) form when that would exceed the database's identifier length limit, + # following the same scheme Rails uses for long index names. + def temporary_name(name) + suffixed = "#{name}_to_be_replaced" + return suffixed if suffixed.bytesize <= max_identifier_length + + hashed_identifier = "_#{OpenSSL::Digest::SHA256.hexdigest(name.to_s).first(10)}" + short_name = name.to_s.truncate_bytes(max_identifier_length - hashed_identifier.bytesize, omission: nil) + "#{short_name}#{hashed_identifier}" end end end diff --git a/lib/active_record/pg_extensions/postgresql_adapter.rb b/lib/active_record/pg_extensions/postgresql_adapter.rb index 4eab209..7441852 100644 --- a/lib/active_record/pg_extensions/postgresql_adapter.rb +++ b/lib/active_record/pg_extensions/postgresql_adapter.rb @@ -38,6 +38,30 @@ def rename_constraint(table_name, old_name, new_name, if_exists: false) SQL end + # alters the attributes of an existing constraint on a table + # see https://www.postgresql.org/docs/current/sql-altertable.html#SQL-ALTERTABLE-DESC-ALTER-CONSTRAINT + def change_constraint(table, constraint, deferrable: nil, enforced: nil, initially: nil, inherit: nil) + if deferrable.nil? && initially.nil? && enforced.nil? && inherit.nil? + raise ArgumentError, "at least one of :deferrable, :initially, :enforced, or :inherit must be specified" + end + + options = [] + options << (deferrable ? "DEFERRABLE" : "NOT DEFERRABLE") unless deferrable.nil? + unless initially.nil? + case initially + when :deferred then options << "INITIALLY DEFERRED" + when :immediate then options << "INITIALLY IMMEDIATE" + else raise ArgumentError, "initially must be :deferred or :immediate" + end + end + options << (enforced ? "ENFORCED" : "NOT ENFORCED") unless enforced.nil? + + alter_table = "ALTER TABLE #{quote_table_name(table)} ALTER CONSTRAINT #{quote_column_name(constraint)}" + execute("#{alter_table} #{options.join(" ")}") unless options.empty? + # INHERIT/NO INHERIT cannot be combined with other options + execute("#{alter_table} #{inherit ? "INHERIT" : "NO INHERIT"}") unless inherit.nil? + end + # see https://www.postgresql.org/docs/current/sql-altertable.html#SQL-CREATETABLE-REPLICA-IDENTITY def set_replica_identity(table, identity = :default) identity_clause = case identity diff --git a/lib/active_record/pg_extensions/railtie.rb b/lib/active_record/pg_extensions/railtie.rb index 9a5fe09..857aa35 100644 --- a/lib/active_record/pg_extensions/railtie.rb +++ b/lib/active_record/pg_extensions/railtie.rb @@ -14,7 +14,7 @@ class Railtie < Rails::Railtie require "active_record/pg_extensions/transaction" ::ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(PostgreSQLAdapter) - ::ActiveRecord::Migration::CommandRecorder.include(CommandRecorder) + ::ActiveRecord::Migration::CommandRecorder.prepend(CommandRecorder) ::ActiveRecord::ConnectionAdapters::NullTransaction.prepend(NullTransaction) ::ActiveRecord::ConnectionAdapters::Transaction.prepend(Transaction) # if they've already require 'all', then inject now diff --git a/lib/active_record/pg_extensions/version.rb b/lib/active_record/pg_extensions/version.rb index 5fe7e6e..497ec82 100644 --- a/lib/active_record/pg_extensions/version.rb +++ b/lib/active_record/pg_extensions/version.rb @@ -2,6 +2,6 @@ module ActiveRecord module PGExtensions - VERSION = "0.7.1" + VERSION = "0.8.0" end end diff --git a/spec/pessimistic_migrations_spec.rb b/spec/pessimistic_migrations_spec.rb index 3eac5da..406fdb8 100644 --- a/spec/pessimistic_migrations_spec.rb +++ b/spec/pessimistic_migrations_spec.rb @@ -133,22 +133,210 @@ end end - describe "#add_check_constraint" do - it "supports if_not_exists" do - expect(connection).to receive(:check_constraint_for).and_return(double(name: "chk_rails_users_name_not_null")) - connection.add_check_constraint :users, - "name IS NOT NULL", - name: "chk_rails_users_name_not_null", - if_not_exists: true - expect(connection.executed_statements).to eq [] + describe "#change_check_constraint" do + before { allow(connection).to receive(:max_identifier_length).and_return(63) } + + it "drops the old constraint and adds the new one" do + allow(connection).to receive(:check_constraint_for!).and_return(double(name: "chk_old")) + connection.change_check_constraint(:users, from: "old_expr", to: "new_expr") + expect(connection.executed_statements).to eq [ + 'ALTER TABLE "users" DROP CONSTRAINT "chk_old"', + 'ALTER TABLE "users" ADD CONSTRAINT chk_rails_5fb2f20b36 CHECK (new_expr)' + ] + end + + it "accepts hashes with :expression and extra options for from and to" do + allow(connection).to receive(:check_constraint_for!).and_return(double(name: "my_old")) + connection.change_check_constraint(:users, + from: { expression: "old_expr", name: "my_old" }, + to: { expression: "new_expr", name: "my_new" }) + expect(connection.executed_statements).to eq [ + 'ALTER TABLE "users" DROP CONSTRAINT "my_old"', + 'ALTER TABLE "users" ADD CONSTRAINT my_new CHECK (new_expr)' + ] + end + + it "delays validation, dropping the old constraint only after the new one is validated" do + allow(connection).to receive(:check_constraint_exists?) { |_table, **opts| opts[:name] == "chk_rails_6fa6c8b575" } + allow(connection).to receive(:check_constraint_for!).and_return(double(name: "chk_rails_6fa6c8b575")) + connection.change_check_constraint(:users, from: "old_expr", to: "new_expr", delay_validation: true) + expect(connection.executed_statements).to eq [ + 'ALTER TABLE "users" ADD CONSTRAINT chk_rails_5fb2f20b36 CHECK (new_expr) NOT VALID', + 'ALTER TABLE "users" VALIDATE CONSTRAINT "chk_rails_5fb2f20b36"', + 'ALTER TABLE "users" DROP CONSTRAINT "chk_rails_6fa6c8b575"' + ] + end + + it "renames the old constraint first when delaying validation and the names collide" do + # the temporary name doesn't exist until we rename into it + renamed = false + allow(connection).to receive(:check_constraint_exists?) do |_table, name: nil, **| + name == "same_to_be_replaced" && renamed + end + allow(connection).to receive(:check_constraint_for!).and_return(double(name: "same_to_be_replaced")) + allow(connection).to receive(:rename_constraint).and_wrap_original do |original, *args, **kwargs| + renamed = true + original.call(*args, **kwargs) + end + connection.change_check_constraint(:users, + from: { expression: "x", name: "same" }, + to: { expression: "y", name: "same" }, + delay_validation: true) + expect(connection.executed_statements).to eq [ + 'ALTER TABLE "users" RENAME CONSTRAINT "same" TO "same_to_be_replaced"', + 'ALTER TABLE "users" ADD CONSTRAINT same CHECK (y) NOT VALID', + 'ALTER TABLE "users" VALIDATE CONSTRAINT "same"', + 'ALTER TABLE "users" DROP CONSTRAINT "same_to_be_replaced"' + ] + end + + it "skips the rename when the temporary constraint name is already taken" do + # a leftover "same_to_be_replaced" from a previously-interrupted run already exists, so the + # idempotency guard skips the rename and proceeds straight to add/validate/drop + allow(connection).to receive(:check_constraint_exists?) { |_table, name: nil, **| name == "same_to_be_replaced" } + allow(connection).to receive(:check_constraint_for!).and_return(double(name: "same_to_be_replaced")) + connection.change_check_constraint(:users, + from: { expression: "x", name: "same" }, + to: { expression: "y", name: "same" }, + delay_validation: true) + expect(connection.executed_statements).to eq [ + 'ALTER TABLE "users" ADD CONSTRAINT same CHECK (y) NOT VALID', + 'ALTER TABLE "users" VALIDATE CONSTRAINT "same"', + 'ALTER TABLE "users" DROP CONSTRAINT "same_to_be_replaced"' + ] + end + + it "ignores delay_validation inside a transaction" do + allow(connection).to receive(:check_constraint_for!).and_return(double(name: "chk_old")) + connection.transaction do + connection.change_check_constraint(:users, from: "old_expr", to: "new_expr", delay_validation: true) + end + expect(connection.executed_statements).to eq [ + "BEGIN", + 'ALTER TABLE "users" DROP CONSTRAINT "chk_old"', + 'ALTER TABLE "users" ADD CONSTRAINT chk_rails_5fb2f20b36 CHECK (new_expr)', + "COMMIT" + ] + end + + it "is reversible by swapping from and to, keeping other arguments the same" do + recorder = ActiveRecord::Migration::CommandRecorder.new(connection) + recorder.revert do + recorder.change_check_constraint(:users, + from: "old_expr", + to: "new_expr", + if_exists: true, + delay_validation: true) + end + expect(recorder.commands).to eq( + [[:change_check_constraint, + [:users, { from: "new_expr", to: "old_expr", if_exists: true, delay_validation: true }]]] + ) end end - describe "#remove_check_constraint" do - it "supports if_exists" do - expect(connection).to receive(:check_constraint_for).and_return(nil) - connection.remove_check_constraint :users, name: "chk_rails_users_name_not_null", if_exists: true - expect(connection.executed_statements).to eq [] + describe "#change_index" do + before do + allow(connection).to receive_messages(max_identifier_length: 63, select_value: nil, index_exists?: true) + allow(connection).to receive(:index_name_for_remove) do |_table, column, options| + options[:name] || "index_users_on_#{Array(column).join("_and_")}" + end + end + + it "drops the old index and adds the new one" do + connection.change_index(:users, from: :a, to: :b) + expect(connection.executed_statements).to eq [ + 'DROP INDEX "index_users_on_a"', + 'CREATE INDEX "index_users_on_b" ON "users" ("b")' + ] + end + + it "accepts hashes with :column and extra options for from and to" do + connection.change_index(:users, from: { column: :a, name: "my_old" }, to: { column: :b, name: "my_new" }) + expect(connection.executed_statements).to eq [ + 'DROP INDEX "my_old"', + 'CREATE INDEX "my_new" ON "users" ("b")' + ] + end + + it "creates the new index concurrently, dropping the old one only afterward" do + allow(connection).to receive(:index_name_exists?).and_return(false) + connection.change_index(:users, from: :a, to: :b, algorithm: :concurrently) + expect(connection.executed_statements).to eq [ + 'CREATE INDEX CONCURRENTLY IF NOT EXISTS "index_users_on_b" ON "users" ("b")', + 'DROP INDEX CONCURRENTLY "index_users_on_a"' + ] + end + + it "renames the old index first when creating concurrently and the names collide" do + allow(connection).to receive(:index_name_exists?).and_return(false) + connection.change_index(:users, + from: { column: :a, name: "same" }, + to: { column: :b, name: "same" }, + algorithm: :concurrently) + expect(connection.executed_statements).to eq [ + 'ALTER INDEX "same" RENAME TO "same_to_be_replaced"', + 'CREATE INDEX CONCURRENTLY IF NOT EXISTS "same" ON "users" ("b")', + 'DROP INDEX CONCURRENTLY "same_to_be_replaced"' + ] + end + + it "skips the rename when the temporary index name is already taken" do + allow(connection).to receive(:index_name_exists?).and_return(true) + connection.change_index(:users, + from: { column: :a, name: "same" }, + to: { column: :b, name: "same" }, + algorithm: :concurrently) + expect(connection.executed_statements).to eq [ + 'CREATE INDEX CONCURRENTLY IF NOT EXISTS "same" ON "users" ("b")', + 'DROP INDEX CONCURRENTLY "same_to_be_replaced"' + ] + end + + it "keeps the temporary index name within the identifier length limit" do + allow(connection).to receive(:index_name_exists?).and_return(false) + long_name = "a" * 60 # "#{long_name}_old" would be 64 chars, over the 63-char limit + connection.change_index(:users, + from: { column: :a, name: long_name }, + to: { column: :b, name: long_name }, + algorithm: :concurrently) + temp_name = connection.executed_statements.first[/RENAME TO "([^"]+)"/, 1] + expect(temp_name.bytesize).to be <= 63 + expect(temp_name).to start_with("a" * 52).and match(/_[0-9a-f]{10}\z/) + end + + it "skips the rename and the drop when if_exists is set and the old index is missing" do + allow(connection).to receive_messages(index_name_exists?: false, index_exists?: false) + connection.change_index(:users, + from: { column: :a, name: "same" }, + to: { column: :b, name: "same" }, + if_exists: true, + algorithm: :concurrently) + expect(connection.executed_statements).to eq [ + 'CREATE INDEX CONCURRENTLY IF NOT EXISTS "same" ON "users" ("b")' + ] + end + + it "ignores algorithm: :concurrently inside a transaction" do + connection.transaction do + connection.change_index(:users, from: :a, to: :b, algorithm: :concurrently) + end + expect(connection.executed_statements).to eq [ + "BEGIN", + 'DROP INDEX "index_users_on_a"', + 'CREATE INDEX "index_users_on_b" ON "users" ("b")', + "COMMIT" + ] + end + + it "is reversible by swapping from and to, keeping other arguments the same" do + recorder = ActiveRecord::Migration::CommandRecorder.new(connection) + recorder.revert do + recorder.change_index(:users, from: :a, to: :b, if_exists: true, algorithm: :concurrently) + end + expect(recorder.commands).to eq( + [[:change_index, [:users, { from: :b, to: :a, if_exists: true, algorithm: :concurrently }]]] + ) end end end diff --git a/spec/postgresql_adapter_spec.rb b/spec/postgresql_adapter_spec.rb index 84b580e..b3dac06 100644 --- a/spec/postgresql_adapter_spec.rb +++ b/spec/postgresql_adapter_spec.rb @@ -475,4 +475,158 @@ ) end end + + describe "#change_constraint" do + around do |example| + connection.dont_execute(&example) + end + + it "raises if no options are given" do + expect { connection.change_constraint(:users, :my_fk) }.to raise_error(ArgumentError) + end + + it "raises on an invalid initially value" do + expect { connection.change_constraint(:users, :my_fk, initially: :garbage) }.to raise_error(ArgumentError) + end + + it "sets deferrable" do + connection.change_constraint(:users, :my_fk, deferrable: true) + expect(connection.executed_statements).to eq( + ['ALTER TABLE "users" ALTER CONSTRAINT "my_fk" DEFERRABLE'] + ) + end + + it "sets not deferrable" do + connection.change_constraint(:users, :my_fk, deferrable: false) + expect(connection.executed_statements).to eq( + ['ALTER TABLE "users" ALTER CONSTRAINT "my_fk" NOT DEFERRABLE'] + ) + end + + it "combines multiple options in a single statement" do + connection.change_constraint(:users, :my_fk, deferrable: true, initially: :deferred, enforced: false) + expect(connection.executed_statements).to eq( + ['ALTER TABLE "users" ALTER CONSTRAINT "my_fk" DEFERRABLE INITIALLY DEFERRED NOT ENFORCED'] + ) + end + + it "sets initially immediate and enforced" do + connection.change_constraint(:users, :my_fk, initially: :immediate, enforced: true) + expect(connection.executed_statements).to eq( + ['ALTER TABLE "users" ALTER CONSTRAINT "my_fk" INITIALLY IMMEDIATE ENFORCED'] + ) + end + + it "sets inherit in its own statement" do + connection.change_constraint(:users, :my_fk, inherit: true) + expect(connection.executed_statements).to eq( + ['ALTER TABLE "users" ALTER CONSTRAINT "my_fk" INHERIT'] + ) + end + + it "sets no inherit in a statement separate from other options" do + connection.change_constraint(:users, :my_fk, deferrable: true, inherit: false) + expect(connection.executed_statements).to eq( + ['ALTER TABLE "users" ALTER CONSTRAINT "my_fk" DEFERRABLE', + 'ALTER TABLE "users" ALTER CONSTRAINT "my_fk" NO INHERIT'] + ) + end + + it "is reversible, inverting each non-nil option" do + recorder = ActiveRecord::Migration::CommandRecorder.new(connection) + recorder.revert do + recorder.change_constraint(:users, + :my_fk, + deferrable: true, + initially: :deferred, + enforced: false, + inherit: true) + end + expect(recorder.commands).to eq( + [[:change_constraint, + [:users, :my_fk, { deferrable: false, initially: :immediate, enforced: true, inherit: false }]]] + ) + end + + it "raises when reversing an invalid :initially value" do + recorder = ActiveRecord::Migration::CommandRecorder.new(connection) + expect do + recorder.revert { recorder.change_constraint(:users, :my_fk, initially: :garbage) } + end.to raise_error(ArgumentError, /initially must be :deferred or :immediate/) + end + + it "raises when reversing a non-boolean flag option" do + recorder = ActiveRecord::Migration::CommandRecorder.new(connection) + expect do + recorder.revert { recorder.change_constraint(:users, :my_fk, deferrable: "yes") } + end.to raise_error(ArgumentError, /deferrable must be true or false/) + end + + it "raises when reversing an unknown option" do + recorder = ActiveRecord::Migration::CommandRecorder.new(connection) + expect do + recorder.revert { recorder.change_constraint(:users, :my_fk, bogus: true) } + end.to raise_error(ArgumentError, /unknown change_constraint option/) + end + end + + describe "reversing existence options" do + def inverted + recorder = ActiveRecord::Migration::CommandRecorder.new(connection) + recorder.revert { yield recorder } + recorder.commands.map { |command, args, _block| [command, args] } + end + + it "reverses add_index(if_not_exists:) into remove_index(if_exists:)" do + commands = inverted { |r| r.add_index(:users, :foo, if_not_exists: true, name: "idx") } + expect(commands).to eq([[:remove_index, [:users, :foo, { name: "idx", if_exists: true }]]]) + end + + it "reverses remove_index(if_exists:) into add_index(if_not_exists:)" do + commands = inverted { |r| r.remove_index(:users, :foo, if_exists: true, name: "idx") } + expect(commands).to eq([[:add_index, [:users, :foo, { name: "idx", if_not_exists: true }]]]) + end + + it "reverses add_column(if_not_exists:) into remove_column(if_exists:)" do + commands = inverted { |r| r.add_column(:users, :foo, :string, if_not_exists: true) } + expect(commands).to eq([[:remove_column, [:users, :foo, :string, { if_exists: true }]]]) + end + + it "reverses remove_column(if_exists:) into add_column(if_not_exists:)" do + commands = inverted { |r| r.remove_column(:users, :foo, :string, if_exists: true) } + expect(commands).to eq([[:add_column, [:users, :foo, :string, { if_not_exists: true }]]]) + end + + it "reverses add_foreign_key(if_not_exists:) into remove_foreign_key(if_exists:)" do + commands = inverted { |r| r.add_foreign_key(:users, :accounts, if_not_exists: true) } + expect(commands).to eq([[:remove_foreign_key, [:users, :accounts, { if_exists: true }]]]) + end + + it "reverses remove_foreign_key(if_exists:) into add_foreign_key(if_not_exists:)" do + commands = inverted { |r| r.remove_foreign_key(:users, :accounts, if_exists: true) } + expect(commands).to eq([[:add_foreign_key, [:users, :accounts, { if_not_exists: true }]]]) + end + + it "reverses add_reference(if_not_exists:) into remove_reference(if_exists:)" do + commands = inverted { |r| r.add_reference(:users, :account, if_not_exists: true) } + expect(commands).to eq([[:remove_reference, [:users, :account, { if_exists: true }]]]) + end + + it "reverses remove_reference(if_exists:) into add_reference(if_not_exists:)" do + commands = inverted { |r| r.remove_reference(:users, :account, if_exists: true) } + expect(commands).to eq([[:add_reference, [:users, :account, { if_not_exists: true }]]]) + end + + it "reverses the existence option nested in a reference's index: hash" do + commands = inverted { |r| r.add_reference(:users, :account, index: { if_not_exists: true, unique: true }) } + expect(commands).to eq( + [[:remove_reference, [:users, :account, { index: { if_exists: true, unique: true } }]]] + ) + end + + it "reverses add_belongs_to as add_reference" do + commands = inverted { |r| r.add_belongs_to(:users, :account, if_not_exists: true) } + expect(commands).to eq([[:remove_reference, [:users, :account, { if_exists: true }]]]) + end + end end