Artur-Sulej / excellent_migrations

An Elixir tool for checking safety of database migrations.
MIT License
232 stars 25 forks source link

Check is incorrect for index #27

Open epinault opened 1 year ago

epinault commented 1 year ago

based on recent change in Ecto SQL 3.9.x , it seems that it s not working correctly anymore this check. They added an advisory pg lock. per https://hexdocs.pm/ecto_sql/Ecto.Migration.html#index/3-adding-dropping-indexes-concurrently

I tried creating a migration that has a config of advisory lock but the check complained incorrectly until I added the following

@disable_migration_lock true

to my migration. This is wrong because the point of the advisory lock is to avoid removing all completely the lock?

  use Ecto.Migration

  @disable_ddl_transaction true

  def change do
    create table(:mytable) do
      add(:some_field, :uuid, null: false)

      timestamps()
    end

    create(unique_index(:mytable, [:some_field], concurrently: true))
end
I get an a credo error with "Index concurrently without disable ddl transaction" 

so then I have to fix it by added @disable_migration_lock true but based on the way it s supposed to work, I should not have to turn it off at all

I would be happy to help but not sure how this works

Stratus3D commented 2 weeks ago

Not sure if I'm encountering the same exact issue as @epinault , but shouldn't this alone pass the excellent migrations checks?

defmodule MyMigration do
  use Ecto.Migration

  def change do
    create table(:mytable) do
      add(:some_field, :uuid, null: false)

      timestamps()
    end

    create(unique_index(:mytable, [:some_field]))
end

Why do I need to concurrently create an index on a newly created table? The table is new, and therefore doesn't contain any data, so creating the index should be fast.