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

Ecto preload function fails for many_to_many and has_many through #2534

Closed
geofflane opened this Issue May 4, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@geofflane
Copy link

geofflane commented May 4, 2018

Ecto 2.2.10

This is for the preload function in the form like:

comment_fn = fn post_ids -> load_comments(post_ids) end
from p in Post, preload: [comment: ^comment_fn]

From a discussion at: https://elixirforum.com/t/ecto-preload-function-discarding-data/14031/5

As far as I can tell:

has_many through passes a list of child ids to the preload function and will exclude anything returned that doesn’t have an id that matches one of those ids. (Only allows a use case where you want the exact same items that would be returned from the main query). This is unexpected because it’s a different set of ids compared to the other cases. I would expect that instead it would be passing the parent ids and letting the query figure out how to load the data.

many_to_many passes the parent ids to the preload function and will exclude anything returned that doesn’t have an id that matches those ids. (Only would work accidentally if the parent and child had the same ids). I would expect the matching of children should be the referenced parent_id in the child and not the child's own id.

has_many passes the parent ids to the preload function and will work as expected matching the parent id to the “parent_id” property on the child items. (Works as expected)

I implemented a simple test app: https://github.com/geofflane/ecto_preload_test
It's uses Post, Comment, and Tag to show has_many, many_to_many, and has_many through examples. It's a bit contrived, but should show the issue.

@josevalim

This comment has been minimized.

Copy link
Member

josevalim commented Oct 3, 2018

The only way I can think of solving this is by forcing you to return the relevant key when preloading the data as part of a tuple.

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