ash-project / ash_postgres

The PostgreSQL data layer for Ash Framework
https://hexdocs.pm/ash_postgres
MIT License
127 stars 71 forks source link

Custom index unexpectedly dropped when upgrading to 2.0 #284

Closed rgraff closed 3 months ago

rgraff commented 3 months ago

Describe the bug When using attribute-base multitenancy and upgrading to Ash Postgres 2.0, the migration generator will drop any define custom_indexs

To Reproduce With an existing resource and snapshot that has a custom index such as:

    custom_indexes do
      index [:environment_key, :key]
    end
  end

And with the latest release, the migration generator will produce:

  def up do
    drop_if_exists index(:accounts, [:organization_id, :environment_key, :key])
  end

Expected behavior I would expect no change to the index.

Additional context Discussed here: https://discord.com/channels/711271361523351632/711271361523351636/1240164223775146035

zachdaniel commented 3 months ago

Very strange. What changed about the snapshot? This would be pretty hard to debug after the fact

zachdaniel commented 3 months ago

Could you tell me what your snapshot had for that custom index under all_tenants? before you did the upgrade?

rgraff commented 3 months ago

I diff'ed the snapshots, and saw a few expected changes but not under custom_indexes. @sevenseacat said she had a repo that could reproduce.

zachdaniel commented 3 months ago

Oh, awesome. In that case @sevenseacat can you point me to it?

rgraff commented 3 months ago

@zachdaniel I'll have a diff shortly too. Working on it now

sevenseacat commented 3 months ago

its not in a repo but I managed to reproduce it with one of my existing apps.

I have a resource with no multitenancy, and a custom index -

 postgres do
    custom_indexes do
      index "name gin_trgm_ops", name: "artists_name_gin_index", using: "GIN"
    end
  end

The current snapshot for this resource looks like:

  "custom_indexes": [
    {
      "message": null,
      "name": "artists_name_gin_index",
      "table": null,
      "include": null,
      "prefix": null,
      "fields": [
        {
          "type": "string",
          "value": "name gin_trgm_ops"
        }
      ],
      "where": null,
      "unique": false,
      "using": "GIN",
      "nulls_distinct": true,
      "error_fields": [],
      "all_tenants?": false,
      "concurrently": false
    }
  ],

As soon as I added a dummy multitenancy by attribute:

    multitenancy do
      strategy :attribute
      attribute :id
    end

And generated migrations, the index got dropped and was not recreated. Note that the keys are different - the multitenancy attribute is in it, but the name is the same as before because I provided it when defining the index.

drop_if_exists index(:artists, [:id, "name gin_trgm_ops"], name: "artists_name_gin_index")

Some other foreign keys on the resource were dropped and recreated in what looks like the exact same way.

The new snapshot still has the index in it and it looks exactly the same -

  "custom_indexes": [
    {
      "message": null,
      "name": "artists_name_gin_index",
      "table": null,
      "include": null,
      "prefix": null,
      "where": null,
      "fields": [
        {
          "type": "string",
          "value": "name gin_trgm_ops"
        }
      ],
      "unique": false,
      "using": "GIN",
      "all_tenants?": false,
      "error_fields": [],
      "concurrently": false,
      "nulls_distinct": true
    }
  ],

Even though that index no longer exists in the actual database.

Looking at it now I'm not sure its the exact same issue, but I was like why you drop my things mate

zachdaniel commented 3 months ago

Okay, so, I've fixed this insofar it should now properly add the indexes after removing. When changing certain aspects of multitenancy, we drop and recreate indexes.

zachdaniel commented 3 months ago

Fix is in main