Skip to content
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

find_with_related implicitly adds ordering on left.id #2220

Open
swwu opened this issue May 13, 2024 · 3 comments
Open

find_with_related implicitly adds ordering on left.id #2220

swwu opened this issue May 13, 2024 · 3 comments

Comments

@swwu
Copy link

swwu commented May 13, 2024

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 (*)
@swwu swwu changed the title SelectTwoMany forces ordering by id find_with_related implicitly adds ordering by left.id May 13, 2024
@swwu swwu changed the title find_with_related implicitly adds ordering by left.id find_with_related implicitly adds ordering on left.id May 13, 2024
@tyt2y3
Copy link
Member

tyt2y3 commented May 13, 2024

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
Copy link
Author

swwu commented May 13, 2024

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.

@tyt2y3
Copy link
Member

tyt2y3 commented May 14, 2024

I totally agree with you, would appreciate if you can make a PR on any of the points above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants