summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--doc/development/migration_style_guide.md5
-rw-r--r--lib/gitlab/database/migration_helpers.rb15
-rw-r--r--spec/lib/gitlab/database/migration_helpers_spec.rb62
3 files changed, 71 insertions, 11 deletions
diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md
index 243ac7f0c98..1e060ffd941 100644
--- a/doc/development/migration_style_guide.md
+++ b/doc/development/migration_style_guide.md
@@ -136,11 +136,14 @@ class MyMigration < ActiveRecord::Migration
disable_ddl_transaction!
def up
- remove_concurrent_index :table_name, :column_name if index_exists?(:table_name, :column_name)
+ remove_concurrent_index :table_name, :column_name
end
end
```
+Note that it is not necessary to check if the index exists prior to
+removing it.
+
## Adding indexes
If you need to add a unique index please keep in mind there is the possibility
diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb
index 21287a8efd0..55160ca8708 100644
--- a/lib/gitlab/database/migration_helpers.rb
+++ b/lib/gitlab/database/migration_helpers.rb
@@ -59,6 +59,11 @@ module Gitlab
disable_statement_timeout
end
+ if index_exists?(table_name, column_name, options)
+ Rails.logger.warn "Index not created because it already exists (this may be due to an aborted migration or similar): table_name: #{table_name}, column_name: #{column_name}"
+ return
+ end
+
add_index(table_name, column_name, options)
end
@@ -83,6 +88,11 @@ module Gitlab
disable_statement_timeout
end
+ unless index_exists?(table_name, column_name, options)
+ Rails.logger.warn "Index not removed because it does not exist (this may be due to an aborted migration or similar): table_name: #{table_name}, column_name: #{column_name}"
+ return
+ end
+
remove_index(table_name, options.merge({ column: column_name }))
end
@@ -107,6 +117,11 @@ module Gitlab
disable_statement_timeout
end
+ unless index_exists_by_name?(table_name, index_name)
+ Rails.logger.warn "Index not removed because it does not exist (this may be due to an aborted migration or similar): table_name: #{table_name}, index_name: #{index_name}"
+ return
+ end
+
remove_index(table_name, options.merge({ name: index_name }))
end
diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb
index 1de3a14b809..9074e17ae80 100644
--- a/spec/lib/gitlab/database/migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migration_helpers_spec.rb
@@ -67,17 +67,35 @@ describe Gitlab::Database::MigrationHelpers do
model.add_concurrent_index(:users, :foo, unique: true)
end
+
+ it 'does nothing if the index exists already' do
+ expect(model).to receive(:index_exists?)
+ .with(:users, :foo, { algorithm: :concurrently, unique: true }).and_return(true)
+ expect(model).not_to receive(:add_index)
+
+ model.add_concurrent_index(:users, :foo, unique: true)
+ end
end
context 'using MySQL' do
- it 'creates a regular index' do
- expect(Gitlab::Database).to receive(:postgresql?).and_return(false)
+ before do
+ allow(Gitlab::Database).to receive(:postgresql?).and_return(false)
+ end
+ it 'creates a regular index' do
expect(model).to receive(:add_index)
.with(:users, :foo, {})
model.add_concurrent_index(:users, :foo)
end
+
+ it 'does nothing if the index exists already' do
+ expect(model).to receive(:index_exists?)
+ .with(:users, :foo, { unique: true }).and_return(true)
+ expect(model).not_to receive(:add_index)
+
+ model.add_concurrent_index(:users, :foo, unique: true)
+ end
end
end
@@ -95,6 +113,7 @@ describe Gitlab::Database::MigrationHelpers do
context 'outside a transaction' do
before do
allow(model).to receive(:transaction_open?).and_return(false)
+ allow(model).to receive(:index_exists?).and_return(true)
end
context 'using PostgreSQL' do
@@ -103,18 +122,41 @@ describe Gitlab::Database::MigrationHelpers do
allow(model).to receive(:disable_statement_timeout)
end
- it 'removes the index concurrently by column name' do
- expect(model).to receive(:remove_index)
- .with(:users, { algorithm: :concurrently, column: :foo })
+ describe 'by column name' do
+ it 'removes the index concurrently' do
+ expect(model).to receive(:remove_index)
+ .with(:users, { algorithm: :concurrently, column: :foo })
- model.remove_concurrent_index(:users, :foo)
+ model.remove_concurrent_index(:users, :foo)
+ end
+
+ it 'does nothing if the index does not exist' do
+ expect(model).to receive(:index_exists?)
+ .with(:users, :foo, { algorithm: :concurrently, unique: true }).and_return(false)
+ expect(model).not_to receive(:remove_index)
+
+ model.remove_concurrent_index(:users, :foo, unique: true)
+ end
end
- it 'removes the index concurrently by index name' do
- expect(model).to receive(:remove_index)
- .with(:users, { algorithm: :concurrently, name: "index_x_by_y" })
+ describe 'by index name' do
+ before do
+ allow(model).to receive(:index_exists_by_name?).with(:users, "index_x_by_y").and_return(true)
+ end
+
+ it 'removes the index concurrently by index name' do
+ expect(model).to receive(:remove_index)
+ .with(:users, { algorithm: :concurrently, name: "index_x_by_y" })
+
+ model.remove_concurrent_index_by_name(:users, "index_x_by_y")
+ end
+
+ it 'does nothing if the index does not exist' do
+ expect(model).to receive(:index_exists_by_name?).with(:users, "index_x_by_y").and_return(false)
+ expect(model).not_to receive(:remove_index)
- model.remove_concurrent_index_by_name(:users, "index_x_by_y")
+ model.remove_concurrent_index_by_name(:users, "index_x_by_y")
+ end
end
end