Commit 7fe79ad6 authored by Patrick Bair's avatar Patrick Bair Committed by Yannis Roussos

Update database guides for new index name rules

Update the database guides to highight new rules around creating indexes
with names, or referring to indexes (either during exists? check or
removal) by their name.
parent ddd56f64
......@@ -121,3 +121,71 @@ may be affected by factors such as (but not limited to):
In other words, this data is only reliable for a frequently used database with
plenty of data and with as many GitLab features enabled (and being used) as
possible.
## Requirements for naming indexes
Indexes with complex definitions need to be explicitly named rather than
relying on the implicit naming behavior of migration methods. In short,
that means you **must** provide an explicit name argument for an index
created with one or more of the following options:
- `where`
- `using`
- `order`
- `length`
- `type`
- `opclass`
### Considerations for index names
Index names don't have any significance in the database, so they should
attempt to communicate intent to others. The most important rule to
remember is that generic names are more likely to conflict or be duplicated,
and should not be used. Some other points to consider:
- For general indexes, use a template, like: `index_{table}_{column}_{options}`.
- For indexes added to solve a very specific problem, it may make sense
for the name to reflect their use.
- Identifiers in PostgreSQL have a maximum length of 63 bytes.
- Check `db/structure.sql` for conflicts and ideas.
### Why explicit names are required
As Rails is database agnostic, it generates an index name only
from the required options of all indexes: table name and column name(s).
For example, imagine the following two indexes are created in a migration:
```ruby
def up
add_index :my_table, :my_column
add_index :my_table, :my_column, where: 'my_column IS NOT NULL'
end
```
Creation of the second index would fail, because Rails would generate
the same name for both indexes.
This is further complicated by the behavior of the `index_exists?` method.
It considers only the table name, column name(s) and uniqueness specification
of the index when making a comparison. Consider:
```ruby
def up
unless index_exists?(:my_table, :my_column, where: 'my_column IS NOT NULL')
add_index :my_table, :my_column, where: 'my_column IS NOT NULL'
end
end
```
The call to `index_exists?` will return true if **any** index exists on
`:my_table` and `:my_column`, and index creation will be bypassed.
The `add_concurrent_index` helper is a requirement for creating indexes
on populated tables. Since it cannot be used inside a transactional
migration, it has a built-in check that detects if the index already
exists. In the event a match is found, index creation is skipped.
Without an explicit name argument, Rails can return a false positive
for `index_exists?`, causing a required index to not be created
properly. By always requiring a name for certain types of indexes, the
chance of error is greatly reduced.
......@@ -465,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,
......@@ -512,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
......@@ -526,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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment