Treat lookups by key as bounded, and skip Includes EF ignores

claude.md

@@ -40,8 +40,10 @@ The service provider alone is not enough. Entity Framework keys a compiled query
 | `QueryComplexityOptionsExtension.cs` | Holds the levels, and keys the internal service provider |
 | `QueryInterceptor.cs` | `IQueryExpressionInterceptor`: measures shape and strips markers, once per compiled shape |
 | `ShapeAnalyzer.cs` | One pass measuring nodes, depth, operators, navigations and includes |
-| `CollectionCounter.cs` | The collections one SQL query loads, through collection includes and projections. A split query counts none |
-| `UnboundedDetector.cs` | The types of the rows a query can return without a limit, found once per compiled shape |
+| `CollectionCounter.cs` | The collections one SQL query loads, through collection includes and projections. A split query counts none, and nor does an Include that Entity Framework ignores, because the query returns no entity for it to load into |
+| `UnboundedDetector.cs` | The types of the rows a query can return without a limit, found once per compiled shape, with lists limited and without |
+| `KeyLookup.cs` | Whether a `Where` looks its rows up by primary or alternate key, which bounds it like a `Take`. A key in a list is only bounded when `MaxInValues` is set |
+| `RowOperators.cs` | The operators that return some of their source's rows, unchanged: the ones allowed between a `DbSet` and a lookup by key, and between an ignored Include and what ignores it |
 | `UnboundedEntities.cs` | Which of those types `RejectUnbounded` checks: `All`, `None`, `AllExcept`, `Only` |
 | `Sequences.cs` | Whether a type is a sequence, and what it holds |
 | `Markers.cs`, `MarkerReader.cs` | The per query marker calls, and reading (`Read`) and removing (`Strip`) them |

readme.md

@@ -126,7 +126,7 @@ A query is checked against the throw levels before the log levels, so a throw le
 | `MaxSingleQueryCollections` | Collections one SQL query loads | While compiled | 1 |
 | `MaxTake` | The value passed to `Take` | Every execution | 1000 |
 | `MaxInValues` | Values in the largest list the query sends | Every execution | 1000 |
-| `RejectUnbounded` | A query returning rows with no `Take` | While compiled | `All` |
+| `RejectUnbounded` | A query returning rows with no `Take` or lookup by key | While compiled | `All` |
 
 A check fires when the measured value is greater than the level. A level of `null` turns that check off.
 
@@ -145,23 +145,35 @@ It does not count:
  * Reference navigations.
  * A collection only read by an aggregate, like `_.Employees.Count()` or `_.Employees.Any()`, which is a subquery rather than a join.
  * Any collection in a split query, from `AsSplitQuery()` or `UseQuerySplittingBehavior(QuerySplittingBehavior.SplitQuery)`, since each collection is then loaded by its own query. `AsSingleQuery()` overrides the default.
+ * An `Include` that Entity Framework ignores, because the query returns no entity for it to load into. A `Select` returning no entity, like `Select(_ => _.Name)` or `Select(_ => new { _.Name })`, or an aggregate like `Count()`, makes Entity Framework ignore the Includes before it. A projection that could return an entity keeps them counted, including one that passes the entity to a method.
 
 The log default of 1 matches the point where Entity Framework logs `MultipleCollectionIncludeWarning`. That warning only covers `Include`, and only when no splitting behavior is configured.
 
 
 ### Unbounded queries
 
-A query is bounded when it cannot return more rows than a `Take` allows:
+A query is bounded when it cannot return more rows than a `Take`, or a [lookup by key](#lookups-by-key), allows:
 
 - A query that returns one row, an aggregate or a count is bounded, so `First`, `Single`, `Count`, `Any`, `Sum` and friends never fire.
-- `Take` bounds everything below it.
-- `SelectMany`, `Join`, `GroupJoin`, `LeftJoin`, `RightJoin` and `Zip` return more rows than their source, so a `Take` below one of them bounds the source rather than the query.
+- `Take` bounds everything below it, and so does a lookup by key.
+- `SelectMany`, `Join`, `GroupJoin`, `LeftJoin`, `RightJoin` and `Zip` return more rows than their source, so a `Take` or a lookup below one of them bounds the source rather than the query.
 - `Concat` and `Union` are bounded only when both sides are.
 - Every other operator returns no more rows than its source.
 
 The message names the types of the rows returned without a `Take`, and so does `QueryComplexityViolation.RowTypes`. A row type is the entity a query reads, not what it projects to, so `Employees.Select(_ => _.Name)` returns `Employee` rows. A query that joins in another sequence returns rows of both types: `Departments.SelectMany(_ => _.Employees)` returns `Department` and `Employee` rows.
 
 
+### Lookups by key
+
+A `Where` that compares the key of its rows with a value, like `Employees.Where(_ => _.Id == id)`, returns at most one row, so it needs no `Take`:
+
+- Other conditions can be added with `&&`. With `||`, each side has to be a lookup.
+- A composite key needs every part compared.
+- The key is the primary key or an alternate key. A unique index does not count: a filter can make it unique among only some of the rows, and a column that allows null can hold null in many rows.
+- `ids.Contains(_.Id)` returns a row for each value in `ids`, so it is only bounded when `MaxInValues` is set, which limits that list. The log and throw levels each decide this with their own `MaxInValues`. For a composite key, one part can be looked up in a list and the rest compared.
+- Only the rows of a `DbSet` can be looked up by their key. Between the `DbSet` and the `Where` there can be filters, ordering, `Skip`, `Take`, `Distinct`, `OfType`, and options such as `Include` and `AsNoTracking`. After a `Select`, a `SelectMany`, a `Join` or a `Concat`, the same key can be in many rows, as it can in the rows `FromSql` returns.
+
+
 ### Choosing the types to check
 
 Some apps have no large table at all. An admin or workflow app where every table holds hundreds or thousands of rows can return all of them, and this check only reports queries that are fine. Turn it off, and keep the rest:

src/EfQueryComplexity/CollectionCounter.cs

@@ -6,14 +6,16 @@
 /// A single query joins every collection it loads, so each multiplies the rows returned for the
 /// others, a cartesian explosion. A split query loads each collection in its own query, so counts
 /// none. A collection only read by an aggregate, like <c>_.Employees.Count()</c>, is a subquery rather
-/// than a join, so is not counted.
+/// than a join, so is not counted. Nor is an Include that Entity Framework ignores, since the query
+/// returns no entity for it to load into.
 /// </remarks>
 sealed class CollectionCounter(IModel model) :
     ExpressionVisitor
 {
     // Keyed on the full path from the root, so an Include chain that restates a collection, to
     // ThenInclude something else below it, counts that collection once
     HashSet<string> includePaths = [];
+    HashSet<MethodCallExpression> ignoredIncludes = [];
     int projected;
 
     public static int Count(Expression query, IModel model, bool splitByDefault)
@@ -56,24 +58,101 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
         var method = node.Method;
         var declaringType = method.DeclaringType;
 
-        if (declaringType == typeof(EntityFrameworkQueryableExtensions) &&
-            method.Name is "Include" or "ThenInclude")
+        if (IsInclude(method))
+        {
+            if (!ignoredIncludes.Contains(node))
+            {
+                AddIncludePaths(node);
+            }
+
+            return base.VisitMethodCall(node);
+        }
+
+        if (declaringType != typeof(Queryable) &&
+            declaringType != typeof(Enumerable))
         {
-            AddIncludePaths(node);
             return base.VisitMethodCall(node);
         }
 
-        if ((declaringType == typeof(Queryable) || declaringType == typeof(Enumerable)) &&
-            method.Name == "Select")
+        if (method.Name == "Select")
         {
+            var selector = node.Arguments[1];
+            if (!ReturnsEntities(selector))
+            {
+                IgnoreIncludes(node.Arguments[0]);
+            }
+
             Visit(node.Arguments[0]);
-            new ProjectionCounter(this).Visit(node.Arguments[1]);
+            new ProjectionCounter(this).Visit(selector);
             return node;
         }
 
+        // An aggregate, like Count or Any, returns a value rather than entities
+        if (node.Arguments is [var source, ..] &&
+            !Sequences.IsSequence(node.Type) &&
+            !HoldsEntities(node.Type))
+        {
+            IgnoreIncludes(source);
+        }
+
         return base.VisitMethodCall(node);
     }
 
+    static bool IsInclude(MethodInfo method) =>
+        method.DeclaringType == typeof(EntityFrameworkQueryableExtensions) &&
+        method.Name is "Include" or "ThenInclude";
+
+    // An Include only loads into the entities a query returns, so Entity Framework ignores the ones
+    // before an operator that returns none. Walking stops at an operator that changes the rows, since
+    // an Include below it can load into what that operator returns.
+    void IgnoreIncludes(Expression source)
+    {
+        var current = source;
+        while (current is MethodCallExpression call &&
+               RowOperators.KeepsRows(call.Method))
+        {
+            if (IsInclude(call.Method))
+            {
+                ignoredIncludes.Add(call);
+            }
+
+            current = call.Arguments[0];
+        }
+    }
+
+    bool ReturnsEntities(Expression selector)
+    {
+        // Queryable takes the selector as an expression, and Enumerable, inside a lambda, as a delegate
+        if (selector is UnaryExpression {NodeType: ExpressionType.Quote} quote)
+        {
+            selector = quote.Operand;
+        }
+
+        // A method group cannot be looked into, so it could return anything
+        if (selector is not LambdaExpression lambda)
+        {
+            return true;
+        }
+
+        var finder = new EntityFinder(this);
+        finder.Visit(lambda.Body);
+        return finder.Found;
+    }
+
+    // Whether a value is, or holds, entities. A shared type counts, since any entity could use it.
+    bool HoldsEntities(Type type)
+    {
+        var element = Sequences.ElementType(type);
+        if (element.IsValueType ||
+            element == typeof(string))
+        {
+            return false;
+        }
+
+        return model.IsShared(element) ||
+               model.FindEntityType(element) != null;
+    }
+
     void AddIncludePaths(MethodCallExpression node)
     {
         var path = new List<string>();
@@ -295,4 +374,95 @@ void VisitSelectors(Expression node)
             }
         }
     }
+
+    /// <summary>
+    /// Finds an entity a projection can return, which the Includes before it would load into.
+    /// </summary>
+    /// <remarks>
+    /// An entity is not returned when a member is read from it, when it is compared, or when a LINQ
+    /// operator reads it. What those return is checked where it is used. An entity anywhere else,
+    /// such as in a constructor or passed to a method, could be returned.
+    /// </remarks>
+    sealed class EntityFinder(CollectionCounter counter) :
+        ExpressionVisitor
+    {
+        public bool Found { get; private set; }
+
+        public override Expression? Visit(Expression? node)
+        {
+            if (node != null &&
+                counter.HoldsEntities(node.Type))
+            {
+                Found = true;
+                return node;
+            }
+
+            return base.Visit(node);
+        }
+
+        // The parameters are rows coming in, not what the lambda returns
+        protected override Expression VisitLambda<T>(Expression<T> node)
+        {
+            Visit(node.Body);
+            return node;
+        }
+
+        protected override Expression VisitMember(MemberExpression node)
+        {
+            if (node.Expression != null)
+            {
+                Read(node.Expression);
+            }
+
+            return node;
+        }
+
+        protected override Expression VisitBinary(BinaryExpression node)
+        {
+            if (node.NodeType is ExpressionType.Equal or ExpressionType.NotEqual)
+            {
+                Read(node.Left);
+                Read(node.Right);
+                return node;
+            }
+
+            return base.VisitBinary(node);
+        }
+
+        // A LINQ operator, or EF.Property, reads its source rather than returning it
+        protected override Expression VisitMethodCall(MethodCallExpression node)
+        {
+            var declaringType = node.Method.DeclaringType;
+            if (node.Arguments is [var source, ..] &&
+                (declaringType == typeof(Queryable) ||
+                 declaringType == typeof(Enumerable) ||
+                 ShapeAnalyzer.IsProperty(node)))
+            {
+                Read(source);
+                foreach (var argument in node.Arguments.Skip(1))
+                {
+                    Visit(argument);
+                }
+
+                return node;
+            }
+
+            return base.VisitMethodCall(node);
+        }
+
+        // Looks into a value that is read rather than returned. A cast, such as the one reaching a
+        // member of a derived type, is read along with it.
+        void Read(Expression value)
+        {
+            while (value is UnaryExpression
+                   {
+                       NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked or ExpressionType.TypeAs
+                   } cast)
+            {
+                value = cast.Operand;
+            }
+
+            base.Visit(value);
+        }
+    }
 }