Many_to_many association INSERT seems to ignore a query prefix

Hi,

Using:

  • phoenix 1.3.0-rc.2 (Hex package) (mix)
  • ecto 2.1.4 (Hex package) (mix)
  • phoenix_ecto 3.2.3 (Hex package) (mix)

I have the following schemas:

  schema "users" do
    field :email, :string, null: false
    field :firstname, :string, null: false
    field :lastname, :string, null: false

    many_to_many :positions, Accounts.Position, join_through: "users_positions"
  end

   schema "positions" do
    field :name, :string, null: false

    many_to_many :users, User, join_through: "users_positions"
  end

The join table is like this:

  def change do
    create table(:users_positions, primary_key: false) do
      add :user_id, references(:users, on_delete: :nothing), primary_key: true
      add :position_id, references(:positions, on_delete: :nothing), primary_key: true
    end

    create unique_index(:users_positions, [:position_id, :user_id])
  end

As this is a multi-tenant application, I use Postgres schema prefixes. However when trying to insert a user with a query prefix:

          %Accounts.User{
            firstname: firstname, 
            lastname: lastname, 
            email: email,
            positions: [%Position{name: "admin"}]
          }
          |> Ecto.put_meta(prefix: "tenant_test")
          |> Repo.insert

The first two INSERT into prefix.users and prefix.positions work fine, but the INSERT into users_positions does not have the schema prefix and therefore fails:

22:31:41.066 [debug] QUERY OK db=0.0ms queue=33.5ms
begin []

22:31:41.075 [debug] QUERY OK db=0.9ms
INSERT INTO "tenant_test"."users" ("email","firstname","lastname","inserted_at","updated_at") VALUES ($1,$2,$3,$4,$5) RETURNING "id" ["johndoe@mail.com", "John", "Doe", {{2017, 7, 2}, {20, 31, 41, 67145}}, {{2017, 7, 2}, {20, 31, 41, 70151}}]

22:31:41.076 [debug] QUERY OK db=0.2ms
INSERT INTO "tenant_test"."positions" ("name","inserted_at","updated_at") VALUES ($1,$2,$3) RETURNING "id" ["admin", {{2017, 7, 2}, {20, 31, 41, 75623}}, {{2017, 7, 2}, {20, 31, 41, 75626}}]

22:31:41.080 [debug] QUERY ERROR db=1.6ms
INSERT INTO "users_positions" ("position_id","user_id") VALUES ($1,$2) [5, 8]

22:31:41.081 [debug] QUERY OK db=0.0ms
rollback []
** (Postgrex.Error) ERROR 42P01 (undefined_table): relation "users_positions" does not exist
    (ecto) lib/ecto/adapters/sql.ex:195: Ecto.Adapters.SQL.query!/5
    (ecto) lib/ecto/adapters/postgres.ex:86: Ecto.Adapters.Postgres.insert_all/7
    (ecto) lib/ecto/repo/schema.ex:55: Ecto.Repo.Schema.do_insert_all/7
    (ecto) lib/ecto/association.ex:963: Ecto.Association.ManyToMany.on_repo_change/4
    (ecto) lib/ecto/association.ex:330: anonymous fn/7 in Ecto.Association.on_repo_change/6
    (elixir) lib/enum.ex:1755: Enum."-reduce/3-lists^foldl/2-0-"/3
    (ecto) lib/ecto/association.ex:327: Ecto.Association.on_repo_change/6
    (elixir) lib/enum.ex:1755: Enum."-reduce/3-lists^foldl/2-0-"/3
    (ecto) lib/ecto/association.ex:293: Ecto.Association.on_repo_change/3
    (ecto) lib/ecto/repo/schema.ex:617: Ecto.Repo.Schema.process_children/4
    (ecto) lib/ecto/repo/schema.ex:684: anonymous fn/3 in Ecto.Repo.Schema.wrap_in_transaction/6
    (ecto) lib/ecto/adapters/sql.ex:620: anonymous fn/3 in Ecto.Adapters.SQL.do_transaction/3
    (db_connection) lib/db_connection.ex:1275: DBConnection.transaction_run/4
    (db_connection) lib/db_connection.ex:1199: DBConnection.run_begin/3
    (db_connection) lib/db_connection.ex:790: DBConnection.transaction/3
    (mix) lib/mix/task.ex:300: Mix.Task.run_task/3
    (mix) lib/mix/cli.ex:58: Mix.CLI.run_task/2

I also tried to add a Map.put(:prefix, tenant), but this does not change anything (unsurprisingly).

Did I miss something ?

Thank you

Note: there was a slightly related bug about @schema_prefix and assoc: https://github.com/elixir-ecto/ecto/issues/1746

That’s happening because you must call Ecto.put_meta/2 on your %Position{} too.

There is a lib called apartmentex that manage working with prefixes for tenants. I don’t know if they have a function that adds the prefixes recursively for you.

I’ve also implemented a similar lib (didn’t find apartmentex before I implemented), called triplex The main difference between apartmentex and triplex is that instead of replacing your own Repo calls with Apartmentex calls, in triplex you use the lib just like your code, but instead of Ecto.put_meta you call Triplex.put_tenant.

Best regards, Kelvin Stinghen

1 Like

Hi,

I use Apartmentex but had issues with my queries, that’s why I tried to build them manually to better understand my prefix issues. However I don’t like very much to have to replace all the calls with Apartmentex ones, so I will definitely have a look on triplex, I like the approach, thanks for that.

Moreover, AFAIK, Apartmentex does not apply the prefix recursively:

  def insert(repo, model_or_changeset, tenant, opts \\ []) do
    model_or_changeset
    |> add_prefix(tenant)
    |> repo.insert(opts)
  end

  defp add_prefix(%Changeset{} = changeset, tenant) do
    %{changeset | data: add_prefix(changeset.data, tenant)}
  end

  defp add_prefix(%{__struct__: _} = model, tenant) do
    Ecto.put_meta(model,  prefix: build_prefix(tenant))
  end

  defp add_prefix_to_query(queryable, tenant) do
    queryable
    |> Ecto.Queryable.to_query()
    |> Map.put(:prefix, build_prefix(tenant))
  end

Anyway, I suspected something like that, but then I tried:

          position = %Position{name: "admin"} |> Ecto.put_meta(prefix: "tenant_test")


          changeset = %Accounts.User{
            firstname: firstname, 
            lastname: lastname, 
            email: email,
            positions: [position]
          }
          |> Ecto.put_meta(prefix: "tenant_test")

          IO.inspect(changeset)

          Repo.insert(changeset)

and this still fails even if we can see from the IO.inspect() that the position has its prefix set:

14:54:38.080 [info]  Add John Doe as new test admin
%Accounts.User{__meta__: #Ecto.Schema.Metadata<:built, "tenant_test", "users">,
 email: "johndoe@mail.com", firstname: "John", id: nil, inserted_at: nil,
 lastname: "Doe",
 positions: [%Accounts.Position{__meta__: #Ecto.Schema.Metadata<:built, "tenant_test", "positions">,
   children: #Ecto.Association.NotLoaded<association :children is not loaded>,
   id: nil, inserted_at: nil, name: "admin",
   parent: #Ecto.Association.NotLoaded<association :parent is not loaded>,
   parent_id: nil, updated_at: nil}],
 roles: #Ecto.Association.NotLoaded<association :roles is not loaded>,
 updated_at: nil}

14:54:38.123 [debug] QUERY OK db=0.0ms queue=26.6ms
begin []

14:54:38.134 [debug] QUERY OK db=1.0ms
INSERT INTO "tenant_test"."users" ("email","firstname","lastname","inserted_at","updated_at") VALUES ($1,$2,$3,$4,$5) RETURNING "id" ["johndoe@mail.com", "John", "Doe", {{2017, 7, 3}, {12, 54, 38, 125239}}, {{2017, 7, 3}, {12, 54, 38, 128535}}]

14:54:38.135 [debug] QUERY OK db=0.3ms
INSERT INTO "tenant_test"."positions" ("name","inserted_at","updated_at") VALUES ($1,$2,$3) RETURNING "id" ["admin", {{2017, 7, 3}, {12, 54, 38, 135168}}, {{2017, 7, 3}, {12, 54, 38, 135173}}]

14:54:38.141 [debug] QUERY ERROR db=1.7ms
INSERT INTO "users_positions" ("position_id","user_id") VALUES ($1,$2) [18, 17]

14:54:38.141 [debug] QUERY OK db=0.0ms
rollback []
** (Postgrex.Error) ERROR 42P01 (undefined_table): relation "users_positions" does not exist
    (ecto) lib/ecto/adapters/sql.ex:195: Ecto.Adapters.SQL.query!/5
    (ecto) lib/ecto/adapters/postgres.ex:86: Ecto.Adapters.Postgres.insert_all/7
    (ecto) lib/ecto/repo/schema.ex:55: Ecto.Repo.Schema.do_insert_all/7
    (ecto) lib/ecto/association.ex:963: Ecto.Association.ManyToMany.on_repo_change/4
    (ecto) lib/ecto/association.ex:330: anonymous fn/7 in Ecto.Association.on_repo_change/6
    (elixir) lib/enum.ex:1755: Enum."-reduce/3-lists^foldl/2-0-"/3
    (ecto) lib/ecto/association.ex:327: Ecto.Association.on_repo_change/6
    (elixir) lib/enum.ex:1755: Enum."-reduce/3-lists^foldl/2-0-"/3
    (ecto) lib/ecto/association.ex:293: Ecto.Association.on_repo_change/3
    (ecto) lib/ecto/repo/schema.ex:617: Ecto.Repo.Schema.process_children/4
    (ecto) lib/ecto/repo/schema.ex:684: anonymous fn/3 in Ecto.Repo.Schema.wrap_in_transaction/6
    (ecto) lib/ecto/adapters/sql.ex:620: anonymous fn/3 in Ecto.Adapters.SQL.do_transaction/3
    (db_connection) lib/db_connection.ex:1275: DBConnection.transaction_run/4
    (db_connection) lib/db_connection.ex:1199: DBConnection.run_begin/3
    (db_connection) lib/db_connection.ex:790: DBConnection.transaction/3
    (mix) lib/mix/task.ex:300: Mix.Task.run_task/3
    (mix) lib/mix/cli.ex:58: Mix.CLI.run_task/2

But you put me in the right direction, I will review triplex code to understand what I’m missing. Thank you so much.

That is one of the reasons I didn’t migrate to apartmentex when I found it. JFYI triplex does! :wink:

This is probably why you’re still getting the error: you must put the prefix on your changeset too, and adding positions here creates changesets for them too AFAIK. Triplex handles this by recursively adding the prefix to all structs and changesets (and queries also) on Triplex.put_tenant/2.

Nevermind, now I see you’re not using changesets. If I were you, I would try triplex. Just let me know when you have more insights about it.

Yes you’re right. I use changesets in my code but in this test I was trying some basic things to better understand Ecto. So the changeset variable was actually a misnomer there.

I like very much what what you are considering with triplex, as Apartmentex is very rudimentary, which is nice in a way, but then every developer has to write some similar code around. So I will definitely give triplex a shot.

1 Like

Well, using a changeset with cast_assoc and triplex for recursive Map.put() still fails:

          user = %{
            "firstname" => "John",
            "lastname" => "Doe",
            "email" => "johndoe@mail.com",
            "positions" => [%{"name" => "admin"}]
          }

          changeset = %Accounts.User{}
          |> cast(user, [:firstname, :lastname, :email])
          |> cast_assoc(:positions)
          |> Triplex.put_tenant("tenant_test")
          
          IO.inspect(changeset)
          Repo.insert(changeset)

results in

#Ecto.Changeset<action: nil,
 changes: %{email: "johndoe@mail.com", firstname: "John", lastname: "Doe",
   positions: [#Ecto.Changeset<action: :insert, changes: %{name: "admin"},
     errors: [], data: #Accounts.Position<>, valid?: true>]},
 errors: [], data: #Accounts.User<>, valid?: true>

17:19:59.960 [debug] QUERY OK db=0.0ms queue=29.5ms
begin []

17:19:59.970 [debug] QUERY OK db=1.5ms
INSERT INTO "tenant_test"."users" ("email","firstname","lastname","inserted_at","updated_at") VALUES ($1,$2,$3,$4,$5) RETURNING "id" ["johndoe@mail.com", "John", "Doe", {{2017, 7, 3}, {15, 19, 59, 960849}}, {{2017, 7, 3}, {15, 19, 59, 963912}}]

17:19:59.970 [debug] QUERY OK db=0.4ms
INSERT INTO "tenant_test"."positions" ("name","inserted_at","updated_at") VALUES ($1,$2,$3) RETURNING "id" ["admin", {{2017, 7, 3}, {15, 19, 59, 970082}}, {{2017, 7, 3}, {15, 19, 59, 970087}}]

17:19:59.976 [debug] QUERY ERROR db=1.8ms
INSERT INTO "users_positions" ("position_id","user_id") VALUES ($1,$2) [6, 8]

17:19:59.976 [debug] QUERY OK db=0.0ms
rollback []
** (Postgrex.Error) ERROR 42P01 (undefined_table): relation "users_positions" does not exist
    (ecto) lib/ecto/adapters/sql.ex:195: Ecto.Adapters.SQL.query!/5
    (ecto) lib/ecto/adapters/postgres.ex:86: Ecto.Adapters.Postgres.insert_all/7
    (ecto) lib/ecto/repo/schema.ex:55: Ecto.Repo.Schema.do_insert_all/7
    (ecto) lib/ecto/association.ex:963: Ecto.Association.ManyToMany.on_repo_change/4
    (ecto) lib/ecto/association.ex:330: anonymous fn/7 in Ecto.Association.on_repo_change/6
    (elixir) lib/enum.ex:1755: Enum."-reduce/3-lists^foldl/2-0-"/3
    (ecto) lib/ecto/association.ex:327: Ecto.Association.on_repo_change/6
    (elixir) lib/enum.ex:1755: Enum."-reduce/3-lists^foldl/2-0-"/3
    (ecto) lib/ecto/association.ex:293: Ecto.Association.on_repo_change/3
    (ecto) lib/ecto/repo/schema.ex:617: Ecto.Repo.Schema.process_children/4
    (ecto) lib/ecto/repo/schema.ex:684: anonymous fn/3 in Ecto.Repo.Schema.wrap_in_transaction/6
    (ecto) lib/ecto/adapters/sql.ex:620: anonymous fn/3 in Ecto.Adapters.SQL.do_transaction/3
    (db_connection) lib/db_connection.ex:1275: DBConnection.transaction_run/4
    (db_connection) lib/db_connection.ex:1199: DBConnection.run_begin/3
    (db_connection) lib/db_connection.ex:790: DBConnection.transaction/3
    (mix) lib/mix/task.ex:300: Mix.Task.run_task/3
    (mix) lib/mix/cli.ex:58: Mix.CLI.run_task/2

Did I overlook something?

Give a check at Triplex.put_tenant/2 code

OH! Got an eureka here now.

I guess Triplex.put_tenant code does not consider changeset lists inside another changeset, which is your case!!!

I’m opening a PR to fix it on Triplex!

Here it is: https://github.com/ateliware/triplex/pull/8

@rlefevre, can you test on this branch?

Just replace {:triplex, "~> 0.3.3"} with {:triplex, "~> 0.3.3", github: "ateliware/triplex", branch: "changeset-lists-inside-changesets"}on your deps file, do a mix deps.get and run it again!

Just as a side note there is also Tenantex https://github.com/promptworks/tenantex which is a fork of Apartmentex

3 Likes

I liked how tenantex tackled the problem of putting the tenant on queries and changesets, delegating it to Ecto.Repo since it already has a way to do it.

But at the same time I liked it, I prefer calling

User
|> Triplex.put_tenant("tenant")
|> Repo.all()

Instead of

Repo.all(User, prefix: "tenant")

You know what: looking at it right know, I don’t care that much. Calling tenantex's way is probably better, since is not their way, and yes the default way to do it with crude ecto, and it probably should work recursively out of the box.

1 Like

Ah ! You actually gave the solution with this comment.

I really suspected that it should have worked out of the box (without any recursive setting) based on the comments in this bug.

But I was confused about the differences between using Ecto.put_meta(prefix: "tenant") and adding prefix: "tenant" in the Repo callback. But it actually only works with join_through: associations when using the later one.

I would have expected to find more information in Query Prefix, but this is most likely explained elsewhere.

So even with your last patch, the join_through does not take into account the prefix using triplex. Here is an IO.inspect(structs: :false) of the changeset in case you find is useful, but I’m not convinced this is possible to fix it in the changeset:

%{__struct__: Ecto.Changeset, action: nil,
  changes: %{email: "johndoe@mail.com", firstname: "John", lastname: "Doe",
    positions: [%{__struct__: Ecto.Changeset, action: :insert,
       changes: %{name: "admin"}, constraints: [],
       data: %{__meta__: %{__struct__: Ecto.Schema.Metadata, context: nil,
           source: {"tenant_test", "positions"}, state: :built},
         __struct__: gI.Accounts.Position, id: nil, inserted_at: nil, 
         name: nil, updated_at: nil}, empty_values: [""], errors: [],
       filters: %{}, params: %{"name" => "admin"}, prepare: [], repo: nil,
       required: [],
       types: %{id: :id, inserted_at: :naive_datetime, name: :string,
         updated_at: :naive_datetime}, valid?: true, validations: []}]},
  constraints: [],
  data: %{__meta__: %{__struct__: Ecto.Schema.Metadata, context: nil,
      source: {"tenant_test", "users"}, state: :built},
    __struct__: gI.Accounts.User, email: nil, firstname: nil, id: nil, 
    inserted_at: nil, lastname: nil,
    positions: %{__cardinality__: :many, __field__: :positions,
      __owner__: gI.Accounts.User, __struct__: Ecto.Association.NotLoaded},
    updated_at: nil}, empty_values: [""], errors: [], filters: %{},
  params: %{"email" => "johndoe@mail.com", "firstname" => "John",
    "lastname" => "Doe", "positions" => [%{"name" => "admin"}]}, prepare: [],
  repo: nil, required: [],
  types: %{email: :string, firstname: :string, id: :id,
    inserted_at: :naive_datetime, lastname: :string,
    positions: {:assoc,
     %{__struct__: Ecto.Association.ManyToMany, cardinality: :many,
       defaults: [], field: :positions,
       join_keys: [user_id: :id, position_id: :id],
       join_through: "users_positions",
       on_cast: &gI.Accounts.Position.changeset/2, on_delete: :nothing,
       on_replace: :raise, owner: gI.Accounts.User, owner_key: :id, 
       queryable: gI.Accounts.Position, related: Accounts.Position,
       relationship: :child, unique: false}}, updated_at: :naive_datetime},
  valid?: true, validations: []}

So it seems that the tenantex way (therefore the default ecto way) is actually the only one to work correctly in all cases. It’s still possible to use prefix: "..." with Apartmentex and triplex, but then is there any use for the Ecto.put_meta they use?

Anyway thank you, this was very helpful.

1 Like

Cool, I’m probably deprecating put_tenant too, as there is a way to do it with plain old ecto.

Actually, thank you for clearing the way for a lot of questions I would have sooner or later!! :wink:

I’m working on the Plug you found useful to release a 1.0.0. I’ll let you all know here when I have a fully functional package. Thanks!

1 Like

I think it should be considered indeed.

The subtile differences between @schema_prefix, Repo.*(prefix: ) and Ecto.put_meta(prefix: are hard to grasp for an Ecto newcomer like me. Also if I’m not wrong, the prefix is really completely supported only since Ecto 2, so it’s easy to find some outdated information or code.

1 Like

As a summary for future readers, here is the explicit solution to the first post:

          %Accounts.User{
            firstname: firstname, 
            lastname: lastname, 
            email: email,
            positions: [%Position{name: "admin"}]
          }
          |> Repo.insert(prefix: "tenant_test")

The schema prefix is not used in join_through associations if you use Ecto.put_meta(prefix:, "...").

1 Like

Just an update, that is reality now. :wink:

We’ve reached 1.1.2 now

1 Like

Hello, I have a related problem.

Imagine an Order which has_many OrderItems, and uses cast_assoc(:order_items). Here, Ecto magically calls OrderItem.changeset/2. Then inside OrderItem.changeset/2 we want a function to do something like: put_assoc(:sku, Repo.get!(SKU, sku_id, prefix: prefix)).

The problem is that OrderItem.changeset/2 has no way to know the prefix at this point. When I use Ecto.put_meta(order, prefix: prefix), I expect Ecto to pass the prefix down into the metadata when it does cast_assoc, so then I should be able to get the prefix using Ecto.get_meta(order_item, :prefix). But it comes back nil. Is this a reasonable expectation? Is there a better way to handle this case? Thanks!

EDIT:

I’m getting around this for now by manually calling changeset with the :prefix applied, but this is feeling wrong:

# order.ex
  def put_order_items(changeset, options) do
    case changeset.params do
      %{"order_items" => items} ->
        put_order_items(changeset, items, options)
      %{order_items: items} ->
        put_order_items(changeset, items, options)
      _ -> changeset
    end
  end
  def put_order_items(changeset, items_params, tenant: tenant) do
    order_items = for item_params <- items_params,
      new_item = %OrderItem{} |> Ecto.put_meta(prefix: Triplex.to_prefix(tenant)) do
        OrderItem.changeset(new_item, item_params)
    end
    put_assoc(changeset, :order_items, order_items)
  end
# order_item.ex
  defp prefix(%Ecto.Changeset{} = changeset) do
    Ecto.get_meta(changeset.data, :prefix)
  end

  defp put_sku(changeset) do
    case get_change(changeset, :sku_id) do
      nil -> changeset
      id ->
        sku = Repo.get!(Cart.SKU, id, prefix: prefix(changeset))
        put_assoc(changeset, :sku, sku)
    end
  end

EDIT 2: I’ve cleaned up order.ex a bit using :with:

    cast_assoc(:order_items, with: fn(struct, params) ->
      OrderItem.changeset(
        struct |> Ecto.put_meta(prefix: Triplex.to_prefix(tenant)),
        params)
    end)

Yep, it feels a little bit wrong. When I started triplex, this was the common way of applying the prefix, and then I realised it is a lot intrusive.

Very nice option, didn’t know it, thanks for sharing!

Now about the original question: I think I would make it more explicit, and instead of relying on the cast_assoc functionality (which is great, but does not work well with triplex as you can see) for creating the has_many assoc, I would create an Ecto.Multi to save the Order first and then adding the OrderItems to it. Did you get it?