ash-project / ash

A declarative, extensible framework for building Elixir applications.
https://www.ash-hq.org
MIT License
1.63k stars 216 forks source link

Migration generator assumes all foreign keys are UUIDs #600

Closed jimsynz closed 8 months ago

jimsynz commented 1 year ago

Describe the bug I have created two resources like so:

defmodule MyApp.Location do
  use Ash.Resource, data_layer: AshPostgres.DataLayer

  attributes do
    integer_primary_key :id
    attribute :name, :string, allow_nil?: false
  end

  relationships do
    has_many :users, MyApp.User
  end

  postgres do
    repo MyApp.Repo
    table "locations"
  end
end

defmodule MyApp.User do
  use Ash.Resource, data_layer: AshPostgres.DataLayer

  attributes do
    integer_primary_key :id
    attribute :name, :string, allow_nil?: false
  end

  relationships do
    belongs_to :location, MyApp.Location do
      attribute_writable? true
      allow_nil? false
    end
  end

  postgres do
    repo MyApp.Repo
    table "users"
  end
end

When adding the relationship between the two resources and running the migration generator, it generates the following (incorrect) migration:

defmodule MyApp.Repo.Migrations.AddUserLocationRelationship do
  @moduledoc """
  Updates resources based on their most recent snapshots.

  This file was autogenerated with `mix ash_postgres.generate_migrations`
  """

  use Ecto.Migration

  def up do
    alter table(:users) do
      add :location_id,
          references(:locations,
            column: :id,
            name: "users_location_id_fkey",
            type: :uuid,
            prefix: "public"
          ),
          null: false
    end
  end

  def down do
    drop constraint(:users, "users_location_id_fkey")

    alter table(:users) do
      remove :location_id
    end
  end
end

Note that the references macro has it's type argument set to :uuid instead of :bigserial.

** Runtime

$ elixir --version
Erlang/OTP 26 [erts-14.0] [source] [64-bit] [smp:5:5] [ds:5:5:10] [async-threads:1] [jit]

Elixir 1.14.5 (compiled with Erlang/OTP 24)
$ uname -a
Linux be01e1dd814e 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
$ mix hex.outdated
mix hex.outdated
Dependency    Current  Latest  Status      
ash           2.9.20   2.9.20  Up-to-date  
ash_postgres  1.3.29   1.3.29  Up-to-date  

Run `mix hex.outdated APP` to see requirements for a specific dependency.

Additional context Running in a devcontainer on my M1-based mac.

jimsynz commented 1 year ago

Ahh, I've figured out that the actual source of the problem is that attribute_type in the belongs_to DSL defaults to UUID. I think we should add a validator that ensures that it is of the same type as the referee's public key type.

zachdaniel commented 1 year ago

Discussed out of band that there is a validation that should prevent this from happening. That validation is in Ash. I'll move this issue there, but we should get a reproduction in test or something more concrete to debug.

zachdaniel commented 8 months ago

I believe this validation is functioning as expected now, in ash. Closing this as fixed.