Skip to content

Combine multiple LIMITs #35383

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

Closed
Tracked by #34846
ranma42 opened this issue Dec 23, 2024 · 2 comments · Fixed by #35384
Closed
Tracked by #34846

Combine multiple LIMITs #35383

ranma42 opened this issue Dec 23, 2024 · 2 comments · Fixed by #35384

Comments

@ranma42
Copy link
Contributor

ranma42 commented Dec 23, 2024

It is currently possible to end up with multiple consecutive LIMIT in a query.
This is especially likely when combined with other query optimizations (such as those that drop unneeded ORDER BY), as mentioned in #34831

Removing them avoids nesting queries and in some cases makes it possible to use a simpler join instead of an apply.

ranma42 added a commit to ranma42/efcore that referenced this issue Dec 23, 2024
Fixes dotnet#35383 for trivial cases (constants/literals and repeated expressions).
ranma42 added a commit to ranma42/efcore that referenced this issue Dec 23, 2024
Fixes dotnet#35383 for trivial cases (constants/literals and repeated expressions).
@roji roji added this to the 10.0.0 milestone Dec 23, 2024
@roji roji self-assigned this Dec 23, 2024
ranma42 added a commit to ranma42/efcore that referenced this issue Dec 24, 2024
Fixes dotnet#35383 for trivial cases (constants/literals and repeated expressions).
roji pushed a commit that referenced this issue Dec 24, 2024
@ahmednfwela
Copy link

I realize this has already been merged, but wouldn't it make sense to use the latest assigned limit, instead of calculating the least limit?

so q.Take(10).Take(90) should end up with Limit 90

@ranma42
Copy link
Contributor Author

ranma42 commented Mar 19, 2025

@ahmednfwela the purpose of this PR is not to change the behavior of Take (in fact, if it did, it would have been considered buggy/wrong 😅 ), but just to make it translate to (slightly) simpler SQL queries.

Regarding the semantics of Take, it is defined in https://learn.microsoft.com/en-us/dotnet/api/system.linq.enumerable.take?view=net-9.0
Note that LINQ operators basically transform the incoming enumerable into a new one in a compositional way (only inspecting the sequence of elements).
The x.Take(10) expression restricts the sequence x to the first 10 elements.
The y.Take(90) restricts the sequence to the first 90 elements and has no way (or reason) to emit other elements besides those from y.

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

Successfully merging a pull request may close this issue.

4 participants