Skip to content

Commit

Permalink
Enhance columns removal check to check indexes where and include
Browse files Browse the repository at this point in the history
…options
  • Loading branch information
fatkodima committed Oct 20, 2024
1 parent 1d241ef commit 7b901f5
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
16 changes: 9 additions & 7 deletions lib/online_migrations/command_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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]
Expand Down
32 changes: 32 additions & 0 deletions test/command_checker/removing_columns_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down

0 comments on commit 7b901f5

Please sign in to comment.