Bug: concurrent_index complaining when creating/dropping an index on a partitioned table in PostgreSQL

francktrouillez commented 7 months ago

I got the cop RuboCop(Sequel/ConcurrentIndex) complaining when I'm trying to run a migration to create/drop an index on a partitioned table in PostgreSQL when concurrently: true is not set, while it is currently not possible in PostgreSQL.

Steps to reproduce

Create a migration that adds an index to a partitioned table
Don't add concurrently: true
Check that the cop RuboCop(Sequel/ConcurrentIndex) is complaining
Add concurrently: true
Run the migration

Get an exception

Sequel::DatabaseError: PG::FeatureNotSupported: ERROR:  cannot create index on partitioned table "my_table" concurrently

ROM::SQL.migration do
  no_transaction

  change do
    alter_table :my_table do
      add_index :my_index, concurrently: true
    end
  end
end

Expected behavior

The cop should not complain, as the proposed change is not working for partitioned table in PostgreSQL

Actual behavior

The cop is complaining, forcing me to change to a non-working solution where a Sequel::DatabaseError: PG::FeatureNotSupported is raised.

Proposed change

If this situation is still happening with up-to-date gems and ruby versions, I believe we could go in 2 directions:

Remove the cop: fast and simple, but might decrease experience for people that don't fact this issue.
Make the cop less restrictive or more clear: we could enforce to specify concurrently, even if it is set to false, and change the message from 'Prefer creating or dropping new index concurrently' to 'Specifyconcurrentlyoption when creating or dropping an index.
I'm of course open for other solutions!

I'm more in favor of the second option of keeping the cop but forcing it to say we should specify the concurrently option, instead of setting it to true, but I'm open for suggestions!

I've opened a PR for this if it makes sense.

System configuration

rubocop-sequel version: 0.3.4

sequel version: 5.76.0

pg version: 1.5.4

Ruby version: ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]

cyberdelia commented 7 months ago

You could also disable the cop locally using comments, see: https://docs.rubocop.org/rubocop/1.62/configuration.html#disabling-cops-within-source-code

francktrouillez commented 7 months ago

You could also disable the cop locally using comments, see: https://docs.rubocop.org/rubocop/1.62/configuration.html#disabling-cops-within-source-code

Yes, that was also a possibility! Thanks for pointing that out! I think updating the message is still a good idea, as it is more explicit about what's expected and doesn't enforce "wrong" implementation in some situation (like the one in this PR).

rubocop / rubocop-sequel