Skip to content

Require name for all complex indexes

Patrick Bair requested to merge 229852-require-name-for-all-complex-index-creation into master

What does this MR do?

Relates to #229852 (closed)

As a follow up to !39503 (merged), expand the cop to flag offenses on non-concurrent index creation. Initially this was thought to not be a concern, as add_concurrent_index is more likely to cause problems due to:

  1. an internal existence check, which does nothing if an existing index is found
  2. being used on existing tables, more likely to have schema inconsistencies

But there are still scenarios on a new table where not using a name could cause problems. For example, perhaps we create a new table with a text column, and a gin index for searching:

def up
  create_table :table do
    t.text :column
  end

  add_index :table, :column, using: :gin, opclass: :gin_trgm_ops
end

At this time, the add_index doesn't require an explicit name, so it uses the default rails name. Then later, the column needs sorting by index so a basic btree index is added:

def up
  add_concurrent_index :table, :column
end

But this index also does not require an explicit name, because there are no options passed to it. As a result, the name could clash with the previously created index. To help prevent this, also register a rubocop offense on the initial add_index to ensure it is explicitly named.

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 Mayra Cabrera

Merge request reports