Skip to content

Add cop so indexes are referred to by name

Patrick Bair requested to merge 229852-refer-to-index-by-name-only into master

What does this MR do?

Relates to: #229852 (closed)

As discussed in the linked issue, there have been occasional production incidents caused by undetected index name conflicts. In !39503 (merged), a cop was introduced to require add_concurrent_index to require an explicit name when an options hash is provided, which should reduce likelihood of some of these issues.

To further build on that, this MR introduces another cop which requires an explicit name be given to migration methods that operate on an existing index. The goal is to reduce likelihood of migrations like:

# pattern inside non-transactional migration
unless index_exists? :table, :column
  add_index :table, :column, name: 'partial_index', where: 'column IS NOT NULL'
end

this can skip index creation even though the existing index doesn't match the definition of the index being added, because the name is not passed to index_exists?. Or when removing an index like:

remove_concurrent_index :table, :column

which erroneously meant to refer to an index that no longer exists, but due to another index existing on the same table/column, it runs without error, dropping the wrong index.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Patrick Bair

Merge request reports