Use the same database prefix as the existing rows when preloading via a lateral join

[sevenseacat]

This fixes the specific issue I was having in #105 (preloads not using the same database prefix as the records being preloaded from), but I don't think it will be a complete solution - there are a few other calls to repo.all in the file that will probably have the same problem.

I'm not sure what the best approach to fixing them all in an idiomatic way would be.

[mbuhot]

I don't think all the calls to repo.all would need to be changed?

Call in load_rows/6 line 346:

dataloader/lib/dataloader/ecto.ex

Lines 338 to 348 in de44dc0

	defp load_rows(col, inputs, queryable, query, repo, repo_opts) do
	case query do
	%Ecto.Query{limit: limit, offset: offset} when not is_nil(limit) or not is_nil(offset) ->
	load_rows_lateral(col, inputs, queryable, query, repo, repo_opts)
	_ ->
	query
	|> where([q], field(q, ^col) in ^inputs)
	|> repo.all(repo_opts)
	end
	end

Should respect the prefix: in repo_opts and the schemas in the query.

---

Call in load_rows_lateral/6 line 365:

dataloader/lib/dataloader/ecto.ex

Lines 350 to 366 in de44dc0

	defp load_rows_lateral(col, inputs, queryable, query, repo, repo_opts) do
	# Approximate a postgres unnest with a subquery
	inputs_query =
	queryable
	|> where([q], field(q, ^col) in ^inputs)
	|> select(^[col])
	|> distinct(true)
	query =
	query
	|> where([q], field(q, ^col) == field(parent_as(:input), ^col))
	from(input in subquery(inputs_query), as: :input)
	|> join(:inner_lateral, q in subquery(query))
	|> select([_input, q], q)
	|> repo.all(repo_opts)
	end

Should respect the prefix: in repo_opts and the schemas in the query.

---

Call in preload_lateral/5 line 697:

dataloader/lib/dataloader/ecto.ex

Lines 676 to 710 in de44dc0

	def preload_lateral([%schema{} = struct | _] = structs, assoc, query, repo, repo_opts) do
	[pk] = schema.__schema__(:primary_key)
	prefix = struct.__meta__.prefix
	repo_opts = Keyword.put(repo_opts, :prefix, prefix)
	assocs = expand_assocs(schema, [assoc])
	inner_query =
	assocs
	|> Enum.reverse()
	|> build_preload_lateral_query(query, :join_first)
	|> maybe_distinct(assocs)
	results =
	from(x in schema,
	as: :parent,
	inner_lateral_join: y in subquery(inner_query),
	where: field(x, ^pk) in ^Enum.map(structs, &Map.get(&1, pk)),
	select: {field(x, ^pk), y}
	)
	|> repo.all(repo_opts)
	{keyed, default} =
	case schema.__schema__(:association, assoc) do
	%{cardinality: :one} ->
	{results |> Map.new(), nil}
	%{cardinality: :many} ->
	{Enum.group_by(results, fn {k, _} -> k end, fn {_, v} -> v end), []}
	end
	structs
	|> Enum.map(&Map.put(&1, assoc, Map.get(keyed, Map.get(&1, pk), default)))
	end

This one is passing the repo_opts into the repo.all call, but might be dropping the prefix: from the schemas in the query when building the inner_query.

The Ecto.Association.assoc_query function does something similar, maybe it can provide a clue as to how to preserve the schema prefixes properly.

[sevenseacat]

In my (admittedly rudimentary) testing, the only data in repo_opts was the caller, being set manually in run_batch/2. I'll do some more investigation, as I'm not too sure how the batching stuff works.

[mbuhot]

From #105

My app uses Postgres schemas for multi-tenancy, and in 1.0.7 this worked fine. As far as I can tell, I haven't needed to manually specify the schema for Dataloader anywhere - my Absinthe resolvers use the correct prefix (eg. loading data using Repo.one(query, prefix: "my_prefix"), and then Dataloader magically does the rest.

I think I understand the problem now.
The old code used Ecto.Repo.preload which the docs suggests that the prefix used to load the collection is also used when preloading the relations:

https://hexdocs.pm/ecto/Ecto.Repo.html#c:preload/3

:prefix - the prefix to fetch preloads from. By default, queries will use the same prefix as the one in the given collection. This option allows the prefix to be changed.

preload_lateral should be updated to do the same. Pretty much as you have in the PR, but maybe replace Keyword.put with Keyword.put_new?

[benwilson512]

@mbuhot does this look good to you now?

[mbuhot]

Yep 👍 LGTM @benwilson512

[tlvenn]

@benwilson512 would be great to merge this one when you get the chance and we should be in a good shape to release a new version. Thanks a lot in advance.

[benwilson512]

Thanks for the reminder!