diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aa051f..76061d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## master (unreleased) +- Enhance columns removal check to check indexes `where` and `include` options - Do not wait before running retried background migrations ## 0.19.6 (2024-09-26) diff --git a/lib/online_migrations/command_checker.rb b/lib/online_migrations/command_checker.rb index dd2ac33..57c062a 100644 --- a/lib/online_migrations/command_checker.rb +++ b/lib/online_migrations/command_checker.rb @@ -453,13 +453,7 @@ def check_columns_removal(command, *args, **options) if !new_table?(table_name) indexes = connection.indexes(table_name).select do |index| - case index.columns - when String - # Expression index - columns.any? { |column| index.columns.include?(column) } - else - (index.columns & columns).any? - end + columns.any? { |column| index_include_column?(index, column) } end raise_error :remove_column, @@ -892,6 +886,14 @@ def index_corruption? postgresql_version < Gem::Version.new("14.4") end + def index_include_column?(index, column) + # Expression index + (index.columns.is_a?(String) && index.columns.include?(column)) || + index.columns.include?(column) || + (Utils.ar_version >= 7.1 && index.include && index.include.include?(column)) || + (index.where && index.where.include?(column)) + end + def run_custom_checks(method, args) OnlineMigrations.config.checks.each do |options, check| if !options[:start_after] || version > options[:start_after] diff --git a/test/command_checker/removing_columns_test.rb b/test/command_checker/removing_columns_test.rb index 63771b4..ae9242e 100644 --- a/test/command_checker/removing_columns_test.rb +++ b/test/command_checker/removing_columns_test.rb @@ -120,6 +120,38 @@ def test_remove_column_with_expression_index "remove_index :users, name: :index_users_on_lower_email, algorithm: :concurrently" end + class RemoveColumnWithPartialIndex < TestMigration + def change + safety_assured do + add_index :users, :email, where: "created_at > '2000-01-01'" + end + + remove_column :users, :created_at + end + end + + def test_remove_column_with_partial_index + assert_unsafe RemoveColumnWithPartialIndex, + "remove_index :users, name: :index_users_on_email, algorithm: :concurrently" + end + + class RemoveColumnWithIncludeIndex < TestMigration + def change + safety_assured do + add_index :users, :email, include: :name + end + + remove_column :users, :name + end + end + + def test_remove_column_with_include_index + skip if ar_version < 7.1 + + assert_unsafe RemoveColumnWithIncludeIndex, + "remove_index :users, name: :index_users_on_email, algorithm: :concurrently" + end + def test_remove_column_with_index_small_table OnlineMigrations.config.small_tables = [:users]