elixir-ecto / ecto

A toolkit for data mapping and language integrated query.
https://hexdocs.pm/ecto
Apache License 2.0
6.15k stars 1.43k forks source link

belongs_to does not infer type based on @foreign_key_type as docs would suggest. #2806

Closed joshchernoff closed 5 years ago

joshchernoff commented 5 years ago

When reading the docs for schema attributes specifically for @foreign_key_type https://hexdocs.pm/ecto/Ecto.Schema.html#module-schema-attributes

It states:

@foreign_key_type - configures the default foreign key type used by belongs_to associations. Defaults to :id;

I found this not be true and that not only do I have to set the @foreign_key_type but also I would have to set the belongs_to type explicitly if its not the default :id type.

The type of error I get was:

** (Ecto.Query.CastError) deps/ecto/lib/ecto/association.ex:516: value `"074a1581-fd36-49e7-a756-03d30f378312"` in `where` cannot be cast to type :id in query:

I had initaliy started this as a question on the elixir forums found here: https://elixirforum.com/t/ecto-query-casterror-value-some-uuid-in-where-cannot-be-cast-to-type-id-in-query/17816

As you can see my solution was to explicitly set the belongs_to type as it was not inferred based on setting the schema's type.

Environment

josevalim commented 5 years ago

Can you please post your migration and schemas and can you make sure that you are calling @foreign_key_type in the schema that calls belongs_to? Thank you.

josevalim commented 5 years ago

Btw, here is a test case that shows this feature is working: https://github.com/elixir-ecto/ecto/blob/master/test/ecto/schema_test.exs#L107-L126 - so more information to reproduce the issue is welcome.

joshchernoff commented 5 years ago
defmodule PolymorphicProductions.Social.Pix do
...

  @primary_key {:id, :binary_id, autogenerate: true}
  @foreign_key_type :binary_id
...
  schema "pics" do
...
    has_many(:comments, PolymorphicProductions.Social.Comment)
...
  end

...
end

defmodule PolymorphicProductions.Social.Comment do
...
  alias PolymorphicProductions.Social.Pix

  schema "comments" do
...
    belongs_to(:pix, Pix, type: :binary_id)
...
  end

...
end

Migrations:
defmodule PolymorphicProductions.Repo.Migrations.CreateComments do
  use Ecto.Migration

  def change do
    create table(:comments) do
...
      add(:pix_id, references(:pics, on_delete: :nothing, type: :uuid)))

      timestamps()
    end

    create(index(:comments, [:pix_id]))
  end
end

defmodule PolymorphicProductions.Repo.Migrations.CreatePics do
  use Ecto.Migration

  def change do
    create table(:pics, primary_key: false) do
      add(:id, :binary_id, primary_key: true)
...
    end
  end
end
joshchernoff commented 5 years ago

I should clarify the above works. but only when I set the type via belongs_to(:pix, Pix, type: :binary_id)

josevalim commented 5 years ago

@joshchernoff it seems you are not setting @foreign_key_type :binary_id in the module that invokes the belongs_to.

joshchernoff commented 5 years ago

the @foreign_key_type for the PolymorphicProductions.Social.Comment should be the default :id the @foreign_key_type for the PolymorphicProductions.Social.Pix should be :binary_id

josevalim commented 5 years ago

@joshchernoff right, I am saying that won't work. When you set @foreign_key_type, it affects the belongs_to calls in that module, it doesn't affect how other modules will belong to you. Generally speaking, the option is only useful when you change the foreign key type across the whole system.

joshchernoff commented 5 years ago

That seem odd to me, why would you want to state the type of a different module's foreign_key_type via declaring the current module's @foreign_key_type for a belongs_to. Would that not also change the current module's @foreign_key_type ? what if it is different vs its association?

josevalim commented 5 years ago

Right, but it simply cannot work on the other way because that could trigger deadlocks during compilation, as modules would have to query each other for the type. So as said above:

Generally speaking, the option is only useful when you change the foreign key type across the whole system.

It is basically a mechanism for flipping the default generally. If only part of your system wants to change it, then it is better to do it as an option per association.

joshchernoff commented 5 years ago

Ok thank you for your help, that makes better sense. Maybe more could be done to help explain that in the docs. I think its easy to infer what I believed to be true. I'm not sure how I would word it.

josevalim commented 5 years ago

I just pushed a commit along those lines, if you feel they could be improved, please do send another PR! --

José Valimwww.plataformatec.com.br http://www.plataformatec.com.br/Founder and Director of R&D