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

Custom primary keys, it works? #18

Open
JanStevens opened this issue Dec 18, 2016 · 12 comments
Open

Custom primary keys, it works? #18

JanStevens opened this issue Dec 18, 2016 · 12 comments

Comments

@JanStevens
Copy link

JanStevens commented Dec 18, 2016

Hello,

I see the readme stating that custom primary keys is still a work in progress but I managed to get it to work, my fear is that this is wrong and its not yet fully supported. Whats the current state of it?

Schema

  @primary_key {:uuid, :string, []}
  @derive {Phoenix.Param, key: :uuid}

  schema "device_configs" do
    field :environment, :string, default: "staging"
    field :configs, :map, default: %{}
    timestamps()
  end

Migration

    create_if_not_exists table(:device_configs, primary_key: false) do
      add :uuid, :string, primary_key: true
      add :environment, :string, default: "staging"
      add :configs, :map, default: %{}

      timestamps()
    end
DevicesEx.Repo.insert(%DevicesEx.DeviceConfig{uuid: "ACR001"})
{:ok,
 %DevicesEx.DeviceConfig{__meta__: #Ecto.Schema.Metadata<:loaded, "device_configs">,
  configs: %{}, environment: "staging",
  inserted_at: ~N[2016-12-18 15:50:21.840696],
  updated_at: ~N[2016-12-18 15:50:21.847592], uuid: "ACR001"}}
iex(5)> DevicesEx.Repo.all(query)
[debug] Selecting by match specification `[{{:device_configs, :"$1", :"$2", :"$3", :"$4", :"$5"}, [], [[:"$1", :"$2", :"$3", :"$4", :"$5"]]}]` with limit `nil`
[%DevicesEx.DeviceConfig{__meta__: #Ecto.Schema.Metadata<:built, "device_configs">,
  configs: %{}, environment: "staging",
  inserted_at: {{2016, 12, 18}, {15, 50, 21, 840696}},
  updated_at: {{2016, 12, 18}, {15, 50, 21, 847592}}, uuid: "ACR001"}]
iex(6)> DevicesEx.Repo.insert(%DevicesEx.DeviceConfig{uuid: "ACR001"})
** (CaseClauseError) no case clause matching: {:error, :already_exists}
    (ecto) lib/ecto/repo/schema.ex:459: Ecto.Repo.Schema.apply/4
    (ecto) lib/ecto/repo/schema.ex:198: anonymous fn/11 in Ecto.Repo.Schema.do_insert/4
@AndrewDryga
Copy link
Member

Your example should work, but only because your primary key will be first in Mnesia record (and Mnesia triggers this field as PK).

@JanStevens
Copy link
Author

Great thanks, and what would be the current limitations / possible issues?

@AndrewDryga
Copy link
Member

Actually, I can't even guess :). Looks like it should work, but I have never tried this way. And in nearest future I will work on Ecto integrations tests to merge this PR elixir-ecto/ecto#1865, so can't tell when I will be able to get back to PK's.

@JanStevens
Copy link
Author

Great ill keep you posted. And that looks awesome :D Great job!

@AndrewDryga
Copy link
Member

Thanks for support and testing it :).

@JanStevens
Copy link
Author

Allright, first issue that pops up during testing, I'm trying to test that the unique constraint is validated correctly. Kinda knew that it wouldn't work but here is the output I receive:

  1) test changeset validations with non unique uuid (DevicesEx.DeviceConfigTest)
     test/models/device_config_test.exs:20
     ** (CaseClauseError) no case clause matching: {:error, :already_exists}
     stacktrace:
       (ecto) lib/ecto/repo/schema.ex:459: Ecto.Repo.Schema.apply/4
       (ecto) lib/ecto/repo/schema.ex:198: anonymous fn/11 in Ecto.Repo.Schema.do_insert/4
       test/models/device_config_test.exs:23: (test)

It seems the error message from mnesia is not correctly translated to the upper layers

@AndrewDryga
Copy link
Member

@JanStevens Can you give more code so I will be able to reproduce this? In meanwhile I have added new case that test insert with duplicate primary key.

@AndrewDryga
Copy link
Member

Can you re-run tests against latest adapter version?

@JanStevens
Copy link
Author

Hello,

Still same issue, I've setup a repo here with a working project (just run the tests): https://github.com/JanStevens/devices_ex

@kexoth
Copy link

kexoth commented Dec 29, 2016

Hello @AndrewDryga & @JanStevens

I'm migrating an app from Amnesia to this library, but have a similar issue.
I wanted to use a custom primary key & did as @JanStevens.
It worked at start, i.e. I can insert records & get them, also when I query directly :mnesia the results are as expected.

However I cannot update nor delete them. I get this error:

** (KeyError) key :struct not found in: []
     (elixir) lib/keyword.ex:333: Keyword.fetch!/2
       (ecto) lib/ecto/exceptions.ex:170: Ecto.NoPrimaryKeyValueError.exception/1
(ecto_mnesia) lib/ecto_mnesia/adapter.ex:265: Ecto.Mnesia.Adapter.get_pk!/2
(ecto_mnesia) lib/ecto_mnesia/adapter.ex:238: Ecto.Mnesia.Adapter.delete/4
       (ecto) lib/ecto/repo/schema.ex:459: Ecto.Repo.Schema.apply/4
       (ecto) lib/ecto/repo/schema.ex:352: anonymous fn/9 in Ecto.Repo.Schema.do_delete/4

I'm using :ecto_mnesia, "0.6.10". Doing the same operations directly with :mnesia work.

@scrogson
Copy link

scrogson commented Feb 10, 2017

However I cannot update nor delete them. I get this error:

I ran into that same error tonight. The error is being raised here. The KeyError is happening because the exception is supposed to be raised with the struct: module passed in.

The get_pk!/2 function will need to become get_pk!/3 and pass in the schema module so that it can be raised properly.

However, even after correcting the error in the source code locally, that didn't fix the error. I ended up needing to switch my options to the following in order to get it to work:

@primary_key {:uuid, :binary_id, autogenerate: true}

This seems like a bug to me. The code seems to be looking for a field that is autogenerated. Shouldn't the primary key be found with module.__schema__(:primary_key)?

@AndrewDryga
Copy link
Member

@scrogson you are right, I'll fix it in next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants