Say we have a Post which has the following association: has_many(:liking_users, through: [:likes, :user]). Two users with usernames "A" and "B" have liked it.
Now we want to preload those users, ordered by their usernames in descending order.
post = Repo.get(Post, 1)
post |> Repo.preload(liking_users: from(u in User, join: l in assoc(u, :likes), order_by: [desc: u.username]))
When accessing post.liking_users, they appear in this order: A, B.
The preloaded users appear in this order: B, A.
(Not sure if this is strictly speaking a bug, but surely there is a way to achieve this with Ecto?)
This is a bug related to #2534. Basically Ecto orders the associations so the preloading step is faster. We should probably remove the ordering and use map to build the preloads.
@zoldar iirc you had a PR almost ready for this issue, is this correct? We are planning to tackle this as part of 3.0 effort, so please let us know if you need any help. :)
@josevalim When I've started writing tests, I've realized that my approach doesn't work for recursively traversed preloads. I've tried to push it further in the recent weeks but without much success (can't focus on anything involved on a computer outside work recently - dunno, maybe first symptoms of burnout :| ). Anyway, if anyone would like to tackle that, here's what I initially came up with (the new addition to the integration tests is failing): https://github.com/elixir-ecto/ecto/compare/master...zoldar:zoldar-preserve-order-in-preloads
@zoldar no worries at all! Sometimes the best thing to do to solve a problem is to step away. :) You may even find a solution a month later, when completely away from it. I will take a look at your tests, they will be very helpful!
Also worth noting that in master we have already relaxed those constraints a bit by sorting only for the relationship key first. This makes it good enough in most cases: #2524.
@zoldar so the reason your test is not passing it is because it is a :through association. In this case, I don't believe there is anything we can do, because we first need to find all comments and then we get all users, and there is no direct ordering relationship between post and authors, as we are passing through comments.
With all that said, I think #2524 is enough to address this and it will be slightly more efficient than maps, so we should be good to go! Thank you for the test case and the alternative solution!
@josevalim why i can't have the my order_by before the order by the relationship key?
using the preload, the queries are (with variables override to run in DB):
-- [debug] QUERY OK source="people_companies" db=1.3ms
SELECT p0."*" FROM "people_companies" AS p0 WHERE (p0."company_id" = '<<company_id>>') ORDER BY p0."company_id"
-- [debug] QUERY OK source="invites" db=1.0ms queue=0.7ms
SELECT i0."inserted_at" AS i0 WHERE (i0."person_company_id" = ANY('<<person_company_ids>>')) ORDER BY i0."person_company_id", i0."inserted_at" DESC LIMIT 5
results:
2018-03-15 17:55:38
2018-03-16 12:23:17
2018-03-22 19:00:31
2018-03-15 16:54:37
2018-03-20 16:40:22
but, if i try to switch the order of the ORDER BY's work fine and with the behavior that i want:
-- [debug] QUERY OK source="people_companies" db=1.3ms
SELECT p0."*" FROM "people_companies" AS p0 WHERE (p0."company_id" = '<<company_id>>') ORDER BY p0."company_id"
-- [debug] QUERY OK source="invites" db=1.0ms queue=0.7ms
SELECT i0."inserted_at" AS i0 WHERE (i0."person_company_id" = ANY('<<person_company_ids>>')) ORDER BY i0."inserted_at" DESC, i0."person_company_id" LIMIT 5
results:
2018-03-26 11:36:20
2018-03-22 19:00:31
2018-03-20 16:40:22
2018-03-16 12:23:17
2018-03-15 17:55:38
Most helpful comment
This is a bug related to #2534. Basically Ecto orders the associations so the preloading step is faster. We should probably remove the ordering and use map to build the preloads.