-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
False positive "Skip/Take without OrderBy" when using SplitQuery #23803
Comments
Triage: improve exception message and mention Split query |
Seems like this is a duplicate of #22579. |
Note to self: Cover this in #24706 |
@NatanBagrov May I ask, why did you say "SingleOrDefault" is not a row limiting query, but "FirstOrDefault" is? I thought opposite. Thank you. |
|
@smitpatel thank you. From what you have described above, Usually, |
Order is not important for Single because the ideal expectation is that there is only 1 row so there is no issue of choosing correct result. Row limiting operation determines restricting number of rows to a smaller set. SingleOrDefault doesn't require selection criteria to determine how to limit the rows so it doesn't throw the error. |
In the first sentence, you said "there's no issue of choosing the correct result" for Also, what I have learned long time ago is that |
The crucial point is that for SingleOrDefault, you get an exception for any set that contains more than one element (that's just how the operator works). So an ORDER BY in SQL isn't required: it's OK for the database to return rows in any order, since if we get more than one we throw anyway. In contrast, FirstOrDefault without ORDER BY is completely non-deterministic: if you don't specify ordering, you have no idea which row you get back out of the matching set.
It's true that if you want only one row, FirstOrDefault allows you to fetch only rather than all of them, which is good for performance. However, you must still specify which row you want back, i.e. according to which ordering. |
@roji Thank you. |
All of EF Core's warnings can be suppressed, see the docs. The relevant EventId here is CoreEventId.RowLimitingOperationWithoutOrderByWarning (see the original post above, where the warning is configured to throw). |
@roji Thank you very much! |
Hi @roji, is it possible to configure these warning events per query? As @changhuixu asked, "in some code". I only see a global context configuration. Is there any option similar to |
No, there's no such way - we've not received requests for per-query warning suppression. If you assume responsibility for your queries, it's generally good enough to suppress the warning globally and then make sure on a query-by-query basis that things make sense. |
Sure thing, I agree with you. Just asking... :) |
When using
.UseQuerySplittingBehavior(QuerySplittingBehavior.SplitQuery)
we get a the following warningeven without really applying and row limiting operation.
Repro:
Configuration:
Query:
As far as I recall,
SingleOrDefault{Async}
is not a row limiting (FirstOrDefault{Async}
is), and as you can see, there is noSkip/Take
here.EF Core version 5.0.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 5.0.1
Operating system: Win 10 x64
The text was updated successfully, but these errors were encountered: