validate keys for references/2

[SteffenDE]

Hi there, I recently had a bug where I accidentally used

create table(:foo, primary_key: false) do
   add(:account_id, references(:account, type: :binary_id, primary_key: true))
end

instead of

create table(:foo, primary_key: false) do
   add(:account_id, references(:account, type: :binary_id), primary_key: true)
end

This PR enforces that only supported keys are used.

[josevalim]

Maybe we could use struct! here instead?

[josevalim]

💚 💙 💜 💛 ❤️

[lleger]

I recently upgraded from v3.11.3 to v3.12.0 and tests started failing. Looks like this PR was the reason -- turns out one of our migrations had the kind of bug this PR was designed to fix. I think it's a good change, but dropping this needle in case others find themselves in the same haystack.

** (KeyError) key :mode not found in: %Ecto.Migration.Table{
  name: "foo",
  prefix: nil,
  comment: nil,
  primary_key: true,
  engine: nil,
  options: nil
}
    (stdlib 5.2) :maps.update(:mode, :cascade, %Ecto.Migration.Table{name: "foo", prefix: nil, comment: nil, primary_key: true, engine: nil, options: nil})
    (elixir 1.16.1) lib/kernel.ex:2442: anonymous fn/2 in Kernel.struct!/2
    (elixir 1.16.1) lib/enum.ex:2528: Enum."-reduce/3-lists^foldl/2-0-"/3
    iex:3: (file)

[adamu]

edit: false alarm, see below comment 😓

We have some migrations that set non-null foreign keys, like this:

add(:other_table_id, references(:other_table, on_delete: :delete_all), null: false)

And they are also failing with this change:

** (KeyError) key :null not found in: %Ecto.Migration.Reference{
  name: nil,
  prefix: nil,
  table: "foo",
  column: :id,
  type: :bigserial,
  on_delete: :nothing,
  on_update: :nothing,
  validate: true,
  with: [],
  match: nil,
  options: []
}
    (stdlib 5.2.3.1) :maps.update(:null, true, %Ecto.Migration.Reference{name: nil, prefix: nil, table: "foo", column: :id, type: :bigserial, on_delete: :nothing, on_update: :nothing, validate: true, with: [], match: nil, options: []})
    (elixir 1.17.2) lib/kernel.ex:2472: anonymous fn/2 in Kernel.struct!/2
    (elixir 1.17.2) lib/enum.ex:2531: Enum."-reduce/3-lists^foldl/2-0-"/3
    (ecto_sql 3.12.0) lib/ecto/migration.ex:1457: Ecto.Migration.references/2

But null is a column option to add, not references. Seems like a bug or have I misunderstood?

[greg-rychlewski]

@adamu Are you sure :null is not in references? Based on the location of the code failure it is parsing the options given to references. And I can also not reproduce unless null: false is inside of references.

If you're sure, could you please provide a small script like this that creates the issue? Thank you

[adamu]

Hi @greg-rychlewski, I have to apologise, it appears I was misreading the migration log 😳

It was along the lines of this:

15:56:43.272 [info] == Running 20240729013658 Foo.Repo.Migrations.AddStuff.change/0 forward
** (KeyError) key :null not found in: %Ecto.Migration.Reference{
...
    (ecto_sql 3.12.0) lib/ecto/migration.ex:1457: Ecto.Migration.references/2
    priv/repo/migrations/20240822004413_actual_problem_is_here.exs:8

Where I was then looking in the AddStuff migration, which was fine, the error (null in references) was in actual_problem_is_here.exs.

Apologies for the noise.