EF Core Query Review
N+1: lazy loading and loops over navigations
// non-compiling: illustrative
// WRONG: 1 query for orders + N queries for customers
var orders = await db.Orders.ToListAsync();
foreach (var o in orders) Console.WriteLine(o.Customer.Name); // lazy-load per row
Fix with a projection, not an Include, when you only need a few fields:
var rows = await db.Orders
.Select(o => new { o.Id, CustomerName = o.Customer.Name })
.ToListAsync(); // single query, single roundtrip
If lazy-loading proxies are enabled project-wide, treat every navigation access outside the query as a suspect. Prefer disabling lazy loading entirely; it converts silent N+1 into a visible exception.
Cartesian explosion with multiple collection Includes
// non-compiling: illustrative
// WRONG: rows = orders x items x payments; 100 orders with 50 items and 10 payments = 50,000 rows
var o = await db.Orders.Include(x => x.Items).Include(x => x.Payments).ToListAsync();
Two or more collection Includes on the same level multiply row counts. Use AsSplitQuery() (one query per collection, consistent only inside a transaction or with snapshot isolation) or split into separate targeted queries. One collection Include is fine; two is a review comment; three is a rejection.
Projection over materialization
Materializing full entities to return a DTO is the most common over-fetch. Select into the DTO directly: EF translates it to a column list, skips change tracking, and avoids loading unmapped blobs.
// non-compiling: illustrative
// WRONG
var users = await db.Users.Include(u => u.Profile).ToListAsync();
return users.Select(u => new UserDto(u.Id, u.Profile.AvatarUrl));
// RIGHT
return await db.Users.Select(u => new UserDto(u.Id, u.Profile.AvatarUrl)).ToListAsync();
AsNoTracking
Every read-only query path (GET endpoints, reports, exports) must be AsNoTracking() or a projection (projections are untracked automatically). Tracking cost is per-entity snapshot allocation and identity-map lookups; on a 10k-row report it dominates. Do not set NoTrackingWithIdentityResolution by reflex - only when the same principal repeats across rows and reference identity matters. Conversely: if the code later mutates the entity and calls SaveChanges, AsNoTracking silently does nothing - that is a bug, not a perf win.
Client-side evaluation
EF Core throws on non-translatable expressions everywhere except the final Select - and that exception is your friend. The dangerous cases are the ones that do NOT throw:
// non-compiling: illustrative
// WRONG: ToList() before Where pulls the whole table
var active = db.Users.ToList().Where(u => IsActive(u));
// WRONG: AsEnumerable mid-query does the same, quietly
var page = db.Users.AsEnumerable().Skip(200).Take(20);
Any ToList/AsEnumerable/ToArray followed by more LINQ operators moves filtering into memory. Also flag calling instance methods or local functions inside Where - they force this pattern.
Pagination and counting
Skip/TakewithoutOrderByreturns nondeterministic pages. Always order by a unique key (append.ThenBy(x => x.Id)).- Offset pagination past ~10k rows scans; use keyset (
WHERE (CreatedAt, Id) > (@lastCreated, @lastId)) for infinite scroll. Count()before fetching doubles roundtrips; if you need both, accept it explicitly or use a windowedCOUNT(*) OVER()via raw SQL - do not call.Count()on a materialized list you fetched only for counting.
Checklist for any reviewed query
- Filter (
Where) and page (Take) in SQL, not memory. - Read-only:
AsNoTrackingor projection. - At most one collection Include per level, else
AsSplitQuery. - No
Containsover an unbounded in-memory list (parameter limit blowups; batch or use temp table/OPENJSON). - No queries inside loops - rewrite as one query with
Where(x => ids.Contains(x.Id))or a join.