From 18a8c05e887a73e75efcfb36e6987edc83a1175a Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 16 Jul 2026 12:18:11 -0600 Subject: [PATCH 1/8] add change_constraint method for setting constraint options --- .../pg_extensions/command_recorder.rb | 33 +++++++ .../pg_extensions/postgresql_adapter.rb | 24 +++++ spec/postgresql_adapter_spec.rb | 94 +++++++++++++++++++ 3 files changed, 151 insertions(+) diff --git a/lib/active_record/pg_extensions/command_recorder.rb b/lib/active_record/pg_extensions/command_recorder.rb index b3882f8..93b50e2 100644 --- a/lib/active_record/pg_extensions/command_recorder.rb +++ b/lib/active_record/pg_extensions/command_recorder.rb @@ -15,6 +15,39 @@ 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 change_constraint(table, constraint, **options) + record(:change_constraint, [table, constraint, 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 + + private + + 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/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/spec/postgresql_adapter_spec.rb b/spec/postgresql_adapter_spec.rb index 84b580e..c9426a1 100644 --- a/spec/postgresql_adapter_spec.rb +++ b/spec/postgresql_adapter_spec.rb @@ -475,4 +475,98 @@ ) 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 end From 3a8e1d6b38505db74558c4dabfd705be9b6c1895 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 16 Jul 2026 12:35:43 -0600 Subject: [PATCH 2/8] make add/remove index/column/foreign_key if_exists/if_not_exists invertible --- .../pg_extensions/command_recorder.rb | 72 +++++++++++++++++-- lib/active_record/pg_extensions/railtie.rb | 2 +- spec/postgresql_adapter_spec.rb | 60 ++++++++++++++++ 3 files changed, 128 insertions(+), 6 deletions(-) diff --git a/lib/active_record/pg_extensions/command_recorder.rb b/lib/active_record/pg_extensions/command_recorder.rb index 93b50e2..b3a24de 100644 --- a/lib/active_record/pg_extensions/command_recorder.rb +++ b/lib/active_record/pg_extensions/command_recorder.rb @@ -8,6 +8,12 @@ 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 + + private + def invert_rename_constraint(args) table_name, old_name, new_name, options = args options ||= {} @@ -16,10 +22,6 @@ def invert_rename_constraint(args) [:rename_constraint, [table_name, new_name, old_name, Hash.ruby2_keywords_hash(options)]] end - def change_constraint(table, constraint, **options) - record(:change_constraint, [table, constraint, options]) - end - def invert_change_constraint(args) table, constraint, options = args options ||= {} @@ -29,7 +31,67 @@ def invert_change_constraint(args) [:change_constraint, [table, constraint, Hash.ruby2_keywords_hash(inverted)]] end - private + # 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? 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/spec/postgresql_adapter_spec.rb b/spec/postgresql_adapter_spec.rb index c9426a1..b3dac06 100644 --- a/spec/postgresql_adapter_spec.rb +++ b/spec/postgresql_adapter_spec.rb @@ -569,4 +569,64 @@ 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 From b903b5b04cd001df663d0ca9d964a62c77743f87 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 16 Jul 2026 12:45:26 -0600 Subject: [PATCH 3/8] remove methods now in stock Rails (7.2+) --- .../pg_extensions/pessimistic_migrations.rb | 16 ---------------- spec/pessimistic_migrations_spec.rb | 19 ------------------- 2 files changed, 35 deletions(-) diff --git a/lib/active_record/pg_extensions/pessimistic_migrations.rb b/lib/active_record/pg_extensions/pessimistic_migrations.rb index fdb1e47..42c374a 100644 --- a/lib/active_record/pg_extensions/pessimistic_migrations.rb +++ b/lib/active_record/pg_extensions/pessimistic_migrations.rb @@ -88,22 +88,6 @@ def add_index(table_name, column_name, **options) end 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) - - super - 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) - - super - end - end end end end diff --git a/spec/pessimistic_migrations_spec.rb b/spec/pessimistic_migrations_spec.rb index 3eac5da..ae4c1f5 100644 --- a/spec/pessimistic_migrations_spec.rb +++ b/spec/pessimistic_migrations_spec.rb @@ -132,23 +132,4 @@ ] 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 [] - 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 [] - end - end end From 53dea85c34b5b735f9cb1f061ef74fbeda506df1 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 16 Jul 2026 14:03:38 -0600 Subject: [PATCH 4/8] add change_check_constraint helper --- .../pg_extensions/command_recorder.rb | 12 +++ .../pg_extensions/pessimistic_migrations.rb | 60 +++++++++++ spec/pessimistic_migrations_spec.rb | 100 ++++++++++++++++++ 3 files changed, 172 insertions(+) diff --git a/lib/active_record/pg_extensions/command_recorder.rb b/lib/active_record/pg_extensions/command_recorder.rb index b3a24de..051afbc 100644 --- a/lib/active_record/pg_extensions/command_recorder.rb +++ b/lib/active_record/pg_extensions/command_recorder.rb @@ -12,8 +12,20 @@ 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 + private + 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 ||= {} diff --git a/lib/active_record/pg_extensions/pessimistic_migrations.rb b/lib/active_record/pg_extensions/pessimistic_migrations.rb index 42c374a..b8f871b 100644 --- a/lib/active_record/pg_extensions/pessimistic_migrations.rb +++ b/lib/active_record/pg_extensions/pessimistic_migrations.rb @@ -88,6 +88,66 @@ def add_index(table_name, column_name, **options) end super end + + # 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 + + if from.is_a?(Hash) + from_options = from.merge(options) + from_expression = from_options.delete(:expression) + else + from_expression = from + from_options = options + end + if to.is_a?(Hash) + to_options = to.merge(options) + to_expression = to_options.delete(:expression) + else + to_expression = to + to_options = options + end + + 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 = "#{old_constraint_name}_old" + 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 end end end diff --git a/spec/pessimistic_migrations_spec.rb b/spec/pessimistic_migrations_spec.rb index ae4c1f5..a2dad87 100644 --- a/spec/pessimistic_migrations_spec.rb +++ b/spec/pessimistic_migrations_spec.rb @@ -132,4 +132,104 @@ ] end end + + describe "#change_check_constraint" do + 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_old" && renamed + end + allow(connection).to receive(:check_constraint_for!).and_return(double(name: "same_old")) + 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_old"', + 'ALTER TABLE "users" ADD CONSTRAINT same CHECK (y) NOT VALID', + 'ALTER TABLE "users" VALIDATE CONSTRAINT "same"', + 'ALTER TABLE "users" DROP CONSTRAINT "same_old"' + ] + end + + it "skips the rename when the temporary constraint name is already taken" do + # a leftover "same_old" 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_old" } + allow(connection).to receive(:check_constraint_for!).and_return(double(name: "same_old")) + 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_old"' + ] + 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 end From a3983c9ee441a979d857596e1349b951ab84babc Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 16 Jul 2026 14:20:19 -0600 Subject: [PATCH 5/8] add change_index helper --- .../pg_extensions/command_recorder.rb | 12 +++ .../pg_extensions/pessimistic_migrations.rb | 78 +++++++++++++--- spec/pessimistic_migrations_spec.rb | 93 +++++++++++++++++++ 3 files changed, 169 insertions(+), 14 deletions(-) diff --git a/lib/active_record/pg_extensions/command_recorder.rb b/lib/active_record/pg_extensions/command_recorder.rb index 051afbc..9797977 100644 --- a/lib/active_record/pg_extensions/command_recorder.rb +++ b/lib/active_record/pg_extensions/command_recorder.rb @@ -16,8 +16,20 @@ 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 ||= {} diff --git a/lib/active_record/pg_extensions/pessimistic_migrations.rb b/lib/active_record/pg_extensions/pessimistic_migrations.rb index b8f871b..2722f51 100644 --- a/lib/active_record/pg_extensions/pessimistic_migrations.rb +++ b/lib/active_record/pg_extensions/pessimistic_migrations.rb @@ -106,20 +106,8 @@ def change_check_constraint(table, from:, to:, if_exists: false, delay_validatio delay_validation = false if open_transactions.positive? options[:validate] = false if delay_validation - if from.is_a?(Hash) - from_options = from.merge(options) - from_expression = from_options.delete(:expression) - else - from_expression = from - from_options = options - end - if to.is_a?(Hash) - to_options = to.merge(options) - to_expression = to_options.delete(:expression) - else - to_expression = to - to_options = options - end + 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, @@ -148,6 +136,68 @@ def change_check_constraint(table, from:, to:, if_exists: false, delay_validatio validate_constraint(table, new_constraint_name) remove_check_constraint(table, name: old_constraint_name, if_exists: true) end + + # 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 + + 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 = "#{old_index_name}_old" + # 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 end end end diff --git a/spec/pessimistic_migrations_spec.rb b/spec/pessimistic_migrations_spec.rb index a2dad87..b02c04d 100644 --- a/spec/pessimistic_migrations_spec.rb +++ b/spec/pessimistic_migrations_spec.rb @@ -232,4 +232,97 @@ ) end end + + 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_old"', + 'CREATE INDEX CONCURRENTLY IF NOT EXISTS "same" ON "users" ("b")', + 'DROP INDEX CONCURRENTLY "same_old"' + ] + 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_old"' + ] + 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 From 314807c82540dc0446006d43646aa23b38f4d327 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 16 Jul 2026 14:26:40 -0600 Subject: [PATCH 6/8] fix potential issues with the temporary index name during replacement --- .../pg_extensions/pessimistic_migrations.rb | 17 +++++++-- spec/pessimistic_migrations_spec.rb | 36 +++++++++++++------ 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/lib/active_record/pg_extensions/pessimistic_migrations.rb b/lib/active_record/pg_extensions/pessimistic_migrations.rb index 2722f51..25b2e30 100644 --- a/lib/active_record/pg_extensions/pessimistic_migrations.rb +++ b/lib/active_record/pg_extensions/pessimistic_migrations.rb @@ -115,7 +115,7 @@ def change_check_constraint(table, from:, to:, if_exists: false, delay_validatio 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 = "#{old_constraint_name}_old" + 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 @@ -163,7 +163,7 @@ def change_index(table, from:, to:, if_exists: false, algorithm: nil, **options) 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 = "#{old_index_name}_old" + 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) @@ -198,6 +198,19 @@ def split_arg(arg, key, options) 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 end diff --git a/spec/pessimistic_migrations_spec.rb b/spec/pessimistic_migrations_spec.rb index b02c04d..406fdb8 100644 --- a/spec/pessimistic_migrations_spec.rb +++ b/spec/pessimistic_migrations_spec.rb @@ -134,6 +134,8 @@ end 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") @@ -169,9 +171,9 @@ # 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_old" && renamed + name == "same_to_be_replaced" && renamed end - allow(connection).to receive(:check_constraint_for!).and_return(double(name: "same_old")) + 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) @@ -181,18 +183,18 @@ to: { expression: "y", name: "same" }, delay_validation: true) expect(connection.executed_statements).to eq [ - 'ALTER TABLE "users" RENAME CONSTRAINT "same" TO "same_old"', + '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_old"' + '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_old" from a previously-interrupted run already exists, so the + # 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_old" } - allow(connection).to receive(:check_constraint_for!).and_return(double(name: "same_old")) + 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" }, @@ -200,7 +202,7 @@ 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_old"' + 'ALTER TABLE "users" DROP CONSTRAINT "same_to_be_replaced"' ] end @@ -273,9 +275,9 @@ to: { column: :b, name: "same" }, algorithm: :concurrently) expect(connection.executed_statements).to eq [ - 'ALTER INDEX "same" RENAME TO "same_old"', + 'ALTER INDEX "same" RENAME TO "same_to_be_replaced"', 'CREATE INDEX CONCURRENTLY IF NOT EXISTS "same" ON "users" ("b")', - 'DROP INDEX CONCURRENTLY "same_old"' + 'DROP INDEX CONCURRENTLY "same_to_be_replaced"' ] end @@ -287,10 +289,22 @@ algorithm: :concurrently) expect(connection.executed_statements).to eq [ 'CREATE INDEX CONCURRENTLY IF NOT EXISTS "same" ON "users" ("b")', - 'DROP INDEX CONCURRENTLY "same_old"' + '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 97e5a40bd392c0944443bf95f58192bc4479afc8 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 16 Jul 2026 14:30:35 -0600 Subject: [PATCH 7/8] add Ruby 4.0 to matrix --- .github/workflows/push.yml | 1 + 1 file changed, 1 insertion(+) 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' From a5a23f98f81696b1b328cfa42b151f3f77c88224 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 16 Jul 2026 12:51:51 -0600 Subject: [PATCH 8/8] v0.8.0 --- Gemfile.lock | 2 +- Gemfile.rails-7.2.lock | 2 +- Gemfile.rails-8.0.lock | 2 +- lib/active_record/pg_extensions/version.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) 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/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