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

Multi-tenancy and preloads not working as expected #4407

Open
egeersoz opened this issue Apr 25, 2024 · 8 comments
Open

Multi-tenancy and preloads not working as expected #4407

egeersoz opened this issue Apr 25, 2024 · 8 comments

Comments

@egeersoz
Copy link

egeersoz commented Apr 25, 2024

Elixir version

1.16.0

Database and Version

14.2

Ecto Versions

3.10.3

Database Adapter and Versions (postgrex, myxql, etc)

Postgrex 0.17.0

Current behavior

This is related to #3387, but pertains to preloading associations where the associated record is on another schema. My use case is a multi-tenant app where each tenant has its own schema. Some tenant records have foreign keys that point to records in the public schema. The relevant tenant migration line looks like this:

add :agent_reminder_job_id, references("oban_jobs", on_delete: :nilify_all, prefix: "public")

The migration itself works fine, i.e. Ecto creates the foreign key association correctly. The problem rears its head when preloading records. For example:

Ecto schema:

schema "invoices" do
  ...
  belongs_to :agent_reminder_job, Oban.Job # this schema is under `public` 
end
query =
  from i in Invoice,
  preload: [:agent_reminder_job]

Repo.one(query, prefix: "my_tenant_schema")

This results in:

(Postgrex.Error) ERROR 42P01 (undefined_table) relation "my_tenant_schema.oban_jobs" does not exist

Expected behavior

Ecto should be"smart" enough to figure out that it should look at the public.oban_jobs table to preload the associated Oban.Job record.

The most user-friendly method would be adding a prefix option to schema association functions (e.g. belongs_to) that mirrors the prefix option in Ecto.migration.references/2.

@josevalim
Copy link
Member

This guide explains the order: https://hexdocs.pm/ecto/multi-tenancy-with-query-prefixes.html

I believe that, if Oban.Job explicitly lists its prefix as public (or allow it to be configured via a compile flag), then it will always win. At the moment I don't think a change in Ecto is required.

@egeersoz
Copy link
Author

I apologize, I'm a bit confused. Reading the section on from/join prefixes, it says you can define a prefix per join, so wouldn't it make sense to also be able to define it per preload?

Essentially, how would you fulfill the requirement whereby an invoice in a tenant schema needs to have its oban job fetched from the public schema? The end result I'm trying to achieve is a Repo operation that returns an Invoice struct with its oban jobs.

@josevalim
Copy link
Member

Reading the section on from/join prefixes, it says you can define a prefix per join, so wouldn't it make sense to also be able to define it per preload?

Sure, that could be a feature. However, I still think the best solution in your case is to explicitly declare that Oban Jobs belong to the public prefix via @schema_prefix. This means you wouldn't have to explicitly pass the prefix on every join.

@egeersoz
Copy link
Author

Well, it's a third-party library that we have no control over at that level. In any case, I hope this issue can be kept open and the feature I proposed considered for a future release. Thank you.

@josevalim
Copy link
Member

Ecto is also a third party library that also does not expose the desired control. If you can ask a feature for Ecto, you can also ask one for Oban and that would arguably be better. :)

So I would like to see that attempted first before we add a feature to Ecto that I believe to be a suboptimal solution to the problem presented.

@egeersoz
Copy link
Author

egeersoz commented Apr 25, 2024

I understand. I've actually already discussed my use case with Oban's creator. Oban supports multi-tenancy, i.e. one can create an oban_jobs table under each tenant schema. But that requires also creating Oban instances to monitor and manage those tables, dynamically creating new tables and launching new instances for new tenants... i.e. significant architectural complexity. I didn't want to go down that route just yet, because the only reason I'd be doing that (instead of the default public.oban_jobs table with a single Oban instance) would be because of Ecto not having a prefix option for preloads (and instead, I think, expecting a hardcoded @schema_prefix on the association schema). I decided to open this issue instead and start a discussion. Ecto is pretty close to the "core" of the Elixir ecosystem, and I think it's important for it to be able to support common use cases, such as multi-tenancy, well.

There are also cases where the association table's prefix may also be dynamic. For example, it can be another tenant table. Suppose it's a chat application where users from one organization can join the channels of another organization, like Slack. In such a scenario, the schema name for the association table would not be known in advance and could not be hardcoded into a @schema_prefix. If I'm understanding you and the docs correctly, Ecto would fall short in those cases as well. Doesn't apply here of course, but wanted to note that as a way of saying this isn't necessarily just about my particular use case.

@Schultzer
Copy link
Contributor

I understand. I've actually already discussed my use case with Oban's creator. Oban supports multi-tenancy, i.e. one can create an oban_jobs table under each tenant schema. But that requires also creating Oban instances to monitor and manage those tables, dynamically creating new tables and launching new instances for new tenants... i.e. significant architectural complexity. I didn't want to go down that route just yet, because the only reason I'd be doing that (instead of the default public.oban_jobs table with a single Oban instance) would be because of Ecto not having a prefix option for preloads (and instead, I think, expecting a hardcoded @schema_prefix on the association schema). I decided to open this issue instead and start a discussion. Ecto is pretty close to the "core" of the Elixir ecosystem, and I think it's important for it to be able to support common use cases, such as multi-tenancy, well.

There are also cases where the association table's prefix may also be dynamic. For example, it can be another tenant table. Suppose it's a chat application where users from one organization can join the channels of another organization, like Slack. In such a scenario, the schema name for the association table would not known in advance and could not be hardcoded into a @schema_prefix. If I'm understanding you and the docs correctly, Ecto would fall short in those cases as well. Doesn't apply here of course, but wanted to note that as a way of saying this isn't necessarily just about my particular use case.

I think there is a good argument for this, but I have a hard time seeing this even be done ergonomically with a table to associate the prefixes and then join them without resorting to raw SQL. But maybe there is something I’m missing since I’m not very well-versed in these scenarios

@greg-rychlewski
Copy link
Member

I understand. I've actually already discussed my use case with Oban's creator. Oban supports multi-tenancy, i.e. one can create an oban_jobs table under each tenant schema. But that requires also creating Oban instances to monitor and manage those tables, dynamically creating new tables and launching new instances for new tenants... i.e. significant architectural complexity. I didn't want to go down that route just yet, because the only reason I'd be doing that (instead of the default public.oban_jobs table with a single Oban instance) would be because of Ecto not having a prefix option for preloads (and instead, I think, expecting a hardcoded @schema_prefix on the association schema). I decided to open this issue instead and start a discussion. Ecto is pretty close to the "core" of the Elixir ecosystem, and I think it's important for it to be able to support common use cases, such as multi-tenancy, well.

There are also cases where the association table's prefix may also be dynamic. For example, it can be another tenant table. Suppose it's a chat application where users from one organization can join the channels of another organization, like Slack. In such a scenario, the schema name for the association table would not be known in advance and could not be hardcoded into a @schema_prefix. If I'm understanding you and the docs correctly, Ecto would fall short in those cases as well. Doesn't apply here of course, but wanted to note that as a way of saying this isn't necessarily just about my particular use case.

I think we should take things one step at a time before the discussion goes in too many directions at once.

Let's go back to Jose's question. He asked if you could submit a request to Oban to let you configure @schema_prefix for the Oban.Job schema. Based on your reply it is not clear if this happened. I see you discussed their current multi-tenant solutions but that is different than what Jose is talking about. That is indeed the simplest/cleanest solution for the problem you reported.

The second thing is, you would never have to do that complicated infrastructure setup, even with the way Ecto is today. You can always define a custom preload query instead of using Ecto's default and set the prefix that way.

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

No branches or pull requests

4 participants