sea-orm
sea-orm copied to clipboard
`find_with_related` implicitly adds ordering on `left.id`
Description
find_with_related
implicitly adds an order by
on the left table's id. This is extremely unintuitive; for instance:
LeftEntity::find()
.find_with_related(RightEntity)
.filter(...)
.order_by(LeftCol1, Order::Desc)
.order_by(LeftCol2, Order::Desc)
generates a query that is ordered by (LeftId, LeftCol1, LeftCol2)
rather than (LeftCol1, LeftCol2)
, as might be expected.
Steps to Reproduce
- Write a query using
find_with_related
that includes anorder_by
after thefind_with_related
- Either try to execute it, or
.build(..).to_string()
it - Observe that the order clause has
order by left.id asc
prepended to it
Expected Behavior
In the example case from the description, I would expect the ordering to be (LeftCol1, LeftCol2)
.
Actual Behavior
In the example case from the description, the actual ordering is (LeftId, LeftCol1, LeftCol2)
.
Reproduces How Often
Deterministically.
Workarounds
Placing the order_by
before the find_with_related
, like so:
LeftEntity::find()
.order_by(LeftCol1, Order::Desc)
.order_by(LeftCol2, Order::Desc)
.find_with_related(RightEntity)
.filter(...)
produces an ordering on (LeftCol1, LeftCol2, LeftId)
, which is acceptable. However, the fact that the query needs to be constructed in this manner is very non-obvious.
Reproducible Example
See description
Versions
├── sea-orm v0.12.15 (*)
├── sea-query v0.30.7 (*)
│ ├── sea-orm v0.12.15 (*)
in the past, the consolidator needs the query results to be ordered. this constrait was dropped in https://github.com/SeaQL/sea-orm/pull/1743 but the behaviour has not changed. removing the default order by is a break I want to avoid, but I agree users should be given the preference if they want to override the ordering
That makes sense. I think just calling attention in documentation to the fact that find_with_related
introduces an ordering would be valuable for now, since the current behavior is very surprising.
The workaround I noted (placing the order_by
calls before find_by_related
) isn't terrible or anything—it's just not very intuitive—so maybe even a small note explaining it as an option would probably cover most ordering use cases for the time being.
I totally agree with you, would appreciate if you can make a PR on any of the points above