diff options
Diffstat (limited to 'doc/development/migration_style_guide.md')
-rw-r--r-- | doc/development/migration_style_guide.md | 62 |
1 files changed, 55 insertions, 7 deletions
diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index ebaceac6d17..207dd02d258 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -39,7 +39,7 @@ Changes to the schema should be committed to `db/structure.sql`. This file is automatically generated by Rails, so you normally should not edit this file by hand. If your migration is adding a column to a table, that column will be added at the bottom. Please do not reorder -columns manually for existing tables as this will cause confusing to +columns manually for existing tables as this will cause confusion to other people using `db/structure.sql` generated by Rails. When your local database in your GDK is diverging from the schema from @@ -260,7 +260,9 @@ def up end def down - drop_table :issues + with_lock_retries do + drop_table :issues + end end ``` @@ -298,7 +300,7 @@ include Gitlab::Database::MigrationHelpers def up with_lock_retries do - add_foreign_key :imports, :projects, column: :project_id, on_delete: :cascade + add_foreign_key :imports, :projects, column: :project_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey end end @@ -316,7 +318,7 @@ include Gitlab::Database::MigrationHelpers def up with_lock_retries do - add_foreign_key :imports, :users, column: :user_id, on_delete: :cascade + add_foreign_key :imports, :users, column: :user_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey end end @@ -367,6 +369,7 @@ migration involves one of the high-traffic tables: - `users` - `projects` - `namespaces` +- `gitlab_subscriptions` - `issues` - `merge_requests` - `ci_pipelines` @@ -462,13 +465,17 @@ class MyMigration < ActiveRecord::Migration[6.0] disable_ddl_transaction! def up - remove_concurrent_index :table_name, :column_name + remove_concurrent_index :table_name, :column_name, name: :index_name end end ``` Note that it is not necessary to check if the index exists prior to -removing it. +removing it, however it is required to specify the name of the +index that is being removed. This can be done either by passing the name +as an option to the appropriate form of `remove_index` or `remove_concurrent_index`, +or more simply by using the `remove_concurrent_index_by_name` method. Explicitly +specifying the name is important to ensure the correct index is removed. For a small table (such as an empty one or one with less than `1,000` records), it is recommended to use `remove_index` in a single-transaction migration, @@ -509,11 +516,16 @@ class MyMigration < ActiveRecord::Migration[6.0] end def down - remove_concurrent_index :table, :column + remove_concurrent_index :table, :column, name: index_name end end ``` +You must explicitly name indexes that are created with more complex +definitions beyond table name, column name(s) and uniqueness constraint. +Consult the [Adding Database Indexes](adding_database_indexes.md#requirements-for-naming-indexes) +guide for more details. + If you need to add a unique index, please keep in mind there is the possibility of existing duplicates being present in the database. This means that should always _first_ add a migration that removes any duplicates, before adding the @@ -523,6 +535,42 @@ For a small table (such as an empty one or one with less than `1,000` records), it is recommended to use `add_index` in a single-transaction migration, combining it with other operations that don't require `disable_ddl_transaction!`. +## Testing for existence of indexes + +If a migration requires conditional logic based on the absence or +presence of an index, you must test for existence of that index using +its name. This helps avoids problems with how Rails compares index definitions, +which can lead to unexpected results. For more details, review the +[Adding Database Indexes](adding_database_indexes.md#why-explicit-names-are-required) +guide. + +The easiest way to test for existence of an index by name is to use the +`index_name_exists?` method, but the `index_exists?` method can also +be used with a name option. For example: + +```ruby +class MyMigration < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + INDEX_NAME = 'index_name' + + def up + # an index must be conditionally created due to schema inconsistency + unless index_exists?(:table_name, :column_name, name: INDEX_NAME) + add_index :table_name, :column_name, name: INDEX_NAME + end + end + + def down + # no op + end +end +``` + +Keep in mind that concurrent index helpers like `add_concurrent_index`, +`remove_concurrent_index`, and `remove_concurrent_index_by_name` already +perform existence checks internally. + ## Adding foreign-key constraints When adding a foreign-key constraint to either an existing or a new column also |