SeaQL/sea-orm

`find_with_related` implicitly adds ordering on `left.id`

Opened this issue · 3 comments

swwu commented

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

  1. Write a query using find_with_related that includes an order_by after the find_with_related
  2. Either try to execute it, or .build(..).to_string() it
  3. 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 #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

swwu commented

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