Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong migration order for unique_index when renaming field #159

Open
michaelst opened this issue Jul 29, 2023 · 2 comments
Open

Wrong migration order for unique_index when renaming field #159

michaelst opened this issue Jul 29, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@michaelst
Copy link
Contributor

Recreating the unique_index should happen after the rename action or do both index actions after the rename

  def down do
    drop_if_exists unique_index(:access__teams, [:license_key_id],
                     name: "access__teams_license_key_id_index"
                   )

    create unique_index(:access__teams, [:idp_entity_id],
             name: "access__teams_idp_entity_id_index"
           )

    rename table(:access__teams), :license_key_id, to: :idp_entity_id
  end
@michaelst michaelst added bug Something isn't working needs review labels Jul 29, 2023
@dmitriid
Copy link

dmitriid commented Sep 5, 2023

Just ran into this as well. This happens for non-unque indexes, too

defmodule Blog.Resources.Page do
  use Ash.Resource,
    data_layer: AshPostgres.DataLayer

  postgres do
    table("pages")
    repo(Blog.Repo)

    custom_indexes do
-      index [:published], unique: false
-      index [:published, :published_at], unique: false
+      index [:is_published], unique: false
+      index [:is_published, :published_at], unique: false
    end
  end

  attributes do
    uuid_primary_key :id

-    attribute :published, :boolean do
+    attribute :is_published, :boolean do
      default false
    end

    attribute :published_at, :naive_datetime do
      allow_nil? true
    end
  end
end

This change generates the following:

defmodule Blog.Repo.Migrations.TablePageRenamePublishedToIsPublished do
  use Ecto.Migration

  def up do
    drop_if_exists index(:pages, ["published", "published_at"],
                     name: "pages_published_published_at_index"
                   )

    drop_if_exists index(:pages, ["published"], name: "pages_published_index")

    create index(:pages, ["is_published", "published_at"])

    create index(:pages, ["is_published"])

    rename table(:pages), :published, to: :is_published
  end

  def down do
    rename table(:pages), :is_published, to: :published

    drop_if_exists index(:pages, ["is_published"], name: "pages_is_published_index")

    drop_if_exists index(:pages, ["is_published", "published_at"],
                     name: "pages_is_published_published_at_index"
                   )

    create index(:pages, ["published"])

    create index(:pages, ["published", "published_at"])
  end
end

Note that down is correct. I guess it's a matter of ordering in the up generator. Should be drop index, rename column, create index.

@MaxfieldLewin
Copy link

+1 this also occurs when removing a field and a related index

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants