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

First() vs Single() for queries that return one result #4835

Open
bassem-mf opened this issue Oct 14, 2024 · 10 comments
Open

First() vs Single() for queries that return one result #4835

bassem-mf opened this issue Oct 14, 2024 · 10 comments
Assignees
Milestone

Comments

@bassem-mf
Copy link

I think the Entity Framework documentation should recommend one option over the other when the user knows that the query will return one result.

I think the feature of Single() throwing an exception when there is more than one matching element is rarely needed when querying a database because users are normally querying to fetch data, not to validate data already saved in the database.

Single() should have a worse performance in at least some cases since it may require a full table scan to ensure there isn't a second match.

This ASP.NET article recommends First() over Single()
https://learn.microsoft.com/en-us/aspnet/core/data/ef-rp/crud?view=aspnetcore-8.0#ways-to-read-one-entity

But I saw @roji recommending Single() over First() in some issue comments
dotnet/efcore#31623 (comment)
dotnet/efcore#29782 (comment)

Also using First() without OrderBy() produces a warning

The query uses the 'First'/'FirstOrDefault' operator without 'OrderBy' and filter operators. This may lead to unpredictable results.

Which makes me think the Entity Framework team wants users to call Single() for queries that return one item.

It will also help if the documentation says when First() performs significantly better that Single(). For example when filtering by a non-indexed column.

@roji roji added this to the 9.0.0 milestone Oct 14, 2024
@roji
Copy link
Member

roji commented Oct 14, 2024

I'm planning some work on our query documentation, I'll make a note to possibly integrate some guidance there. There's no completely clear-cut answer: you're right that limiting to 2 results instead of 1 could have a perf impact on some queries; on the other hand, First() requires an ordering to work reliably (EF warns when there isn't one), whereas Single() does not... And Single() can more clearly express the intent of a given query.

@roji roji self-assigned this Oct 14, 2024
@KLuuKer
Copy link

KLuuKer commented Dec 16, 2024

@roji what about the case when doing a First\FirstOrDefault for cases when I'm selecting by a unique id column..

db.Users
    .Where(u => u.UserId == myId)
    .Select(u => new { u.UserId }) // normally selecting a more complex model here, just barebone example
    .FirstOrDefaultAsync();

@roji
Copy link
Member

roji commented Dec 16, 2024

@KLuuKer I don't think there's anything to say beyond what I already wrote above; I'd generally recommend just using SingleOrDefaultAsync, as that makes it very clear to the reader that you're not getting the first row out of several, but are rather asserting that there's only one (or zero) possible rows. The only downside is the TOP 2 which that adds to the SQL, though that should generally have no impact if there really is a unique index in the database.

In theory, EF could know the the query can only ever return a single row (because it knows UserId is unique), and flow that information, affecting the translation of FirstOrDefaultAsync/SingleOrDefaultAsync. But that sort of optimization is a bit far off.

@KLuuKer
Copy link

KLuuKer commented Dec 16, 2024

@roji I'm more annoyed by the warning i get in my logs (of not including an order by),
then having EF magically "optimize" the query

@roji
Copy link
Member

roji commented Dec 16, 2024

@KLuuKer that's why I'm suggesting using SingleOrDefaultAsync.

@KLuuKer
Copy link

KLuuKer commented Dec 16, 2024

@roji but i don't want TOP 2 and also don't want the warning for that specific case,
and i don't know if SQL "like's" the fact that i add a orderby on the query (for performance)
.... also i'm kinda lazy in having to fix it in like 1000 places....

@roji
Copy link
Member

roji commented Dec 16, 2024

Why don't you want TOP 2?

@bassem-mf
Copy link
Author

bassem-mf commented Dec 18, 2024

@roji what about the case when doing a First\FirstOrDefault for cases when I'm selecting by a unique id column..

db.Users
    .Where(u => u.UserId == myId)
    .Select(u => new { u.UserId }) // normally selecting a more complex model here, just barebone example
    .FirstOrDefaultAsync();

@KLuuKer I think the example in your comment will not generate a FirstWithoutOrderByAndFilterWarning. This warning is generated when the query does not contain a predicate (filter) or an ordering. But your query does contain a predicate Where().

Here is the relevant piece of code that shows when this warning is generated for relational databases.
https://github.com/dotnet/efcore/blob/08422f346606324b1cdfb48b0aefe3cd230c85f3/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs#L693

If the query contains either a predicate (filter) or an ordering, the warning is Not generated.

@KLuuKer
Copy link

KLuuKer commented Jan 1, 2025

@roji
well.... it might be because inside my select is something like

.Select(u => new {
   u.UserId,
   // bunch of other properties
   OtherStuff = new {
     Foo = u.OtherTable.Foo
     Bar = u.OtherTable.Bar
  }
})

so might it be because of the split query mode? (we have AsSplitQuery enabled by default)

p.s. happy new year 🎉

@roji
Copy link
Member

roji commented Jan 2, 2025

@KLuuKer I don't see what any of that is relevant to wanting or not wanting TOP 2...

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

No branches or pull requests

3 participants