Name the collections in the MaxSingleQueryCollections message

readme.md

@@ -163,6 +163,8 @@ It does not count:
 
 The log default of 1 allows one chain of nested collections, and logs as soon as two load side by side.
 
+The message names the collections loaded side by side, in query order, and so does `QueryComplexityViolation.Collections`. Each is the entity it is from and the navigations to it, so `Employees.Include(_ => _.Tasks).Include(_ => _.Department).ThenInclude(_ => _.Employees)` reports `Employee.Tasks, Employee.Department.Employees`. A chain of nested collections is named by the one at its end.
+
 
 ### Unbounded queries
 

src/EfQueryComplexity/CollectionCounter.cs

@@ -14,61 +14,88 @@
 sealed class CollectionCounter(IModel model) :
     ExpressionVisitor
 {
+    // A counted collection, named for the message as the entity it is from and the navigations to
+    // it, with where in the query it was found so the message lists them in query order
+    readonly record struct Collection(int Order, string Name);
+
     // 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 = [];
 
     // The collections a projection returns in place of the entity they belong to, keyed like
-    // includePaths, with the count of what the projection loads inside each. An Include of one of
-    // them only says what to load under it, so the two are one collection.
-    Dictionary<string, int> projectedPaths = [];
+    // includePaths, with what the projection loads inside each. An Include of one of them only says
+    // what to load under it, so the two are one collection.
+    Dictionary<string, List<Collection>> projectedPaths = [];
 
     // What projections load that has no path, so cannot be nested under an Include
-    int projected;
+    List<Collection> projected = [];
+
+    // The name and position of each path, from where it was first found
+    Dictionary<string, Collection> paths = [];
+    int order;
 
-    public static int Count(Expression query, IModel model, bool splitByDefault)
+    /// <summary>
+    /// The collections the query loads side by side, in query order, named as the entity they are
+    /// from and the navigations to them, for example <c>Department.Employees</c>.
+    /// </summary>
+    public static IReadOnlyList<string> Count(Expression query, IModel model, bool splitByDefault)
     {
         if (IsSplit(query, splitByDefault))
         {
-            return 0;
+            return [];
         }
 
         var counter = new CollectionCounter(model);
         counter.Visit(query);
-        return counter.CountPaths() + counter.projected;
+        var collections = counter.CountPaths();
+        collections.AddRange(counter.projected);
+        return collections
+            .OrderBy(_ => _.Order)
+            .Select(_ => _.Name)
+            .ToList();
     }
 
     // Each collection counts as one, or as the sum of what it holds when that is more. A path is held
     // by the longest other path it starts with, so walking the longest first counts every collection
     // before the one holding it.
-    int CountPaths()
+    List<Collection> CountPaths()
     {
-        var held = new Dictionary<string, int>(projectedPaths);
+        var held = new Dictionary<string, List<Collection>>();
+        foreach (var (path, inner) in projectedPaths)
+        {
+            held[path] = [..inner];
+        }
+
         foreach (var path in includePaths)
         {
-            held.TryAdd(path, 0);
+            held.TryAdd(path, []);
         }
 
-        var total = 0;
+        var total = new List<Collection>();
         foreach (var path in held.Keys.OrderByDescending(_ => _.Length).ToList())
         {
-            var count = Math.Max(1, held[path]);
+            var collections = held[path];
+            if (collections.Count == 0)
+            {
+                collections = [paths[path]];
+            }
+
             var holder = Holder(path, held);
             if (holder == null)
             {
-                total += count;
+                total.AddRange(collections);
             }
             else
             {
-                held[holder] += count;
+                held[holder].AddRange(collections);
             }
         }
 
         return total;
     }
 
-    static string? Holder(string path, Dictionary<string, int> paths)
+    static string? Holder(string path, Dictionary<string, List<Collection>> paths)
     {
         var current = path;
         while (true)
@@ -117,12 +144,14 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
 
         if (IsInclude(method))
         {
+            // The source first, so the Includes are found in query order
+            var result = base.VisitMethodCall(node);
             if (!ignoredIncludes.Contains(node))
             {
                 AddIncludePaths(node);
             }
 
-            return base.VisitMethodCall(node);
+            return result;
         }
 
         if (declaringType != typeof(Queryable) &&
@@ -142,7 +171,7 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
             Visit(node.Arguments[0]);
             var projection = new ProjectionCounter(this, RowNotReturned(selector));
             projection.Visit(selector);
-            projected += projection.Count;
+            projected.AddRange(projection.Collections);
             return node;
         }
 
@@ -243,19 +272,22 @@ void AddIncludePaths(MethodCallExpression node)
         var rootType = IncludeChainRoot(node, path);
         var type = rootType;
         var key = PathRoot(rootType);
+        var name = rootType.Name;
 
-        foreach (var name in path)
+        foreach (var segment in path)
         {
-            var navigation = FindNavigation(type, name);
+            var navigation = FindNavigation(type, segment);
             if (navigation == null)
             {
                 return;
             }
 
-            key += "." + name;
-            if (navigation.IsCollection)
+            key += "." + segment;
+            name += "." + segment;
+            if (navigation.IsCollection &&
+                includePaths.Add(key))
             {
-                includePaths.Add(key);
+                AddPath(key, name);
             }
 
             type = navigation.TargetEntityType.ClrType;
@@ -266,6 +298,12 @@ void AddIncludePaths(MethodCallExpression node)
     static string PathRoot(Type type) =>
         type.FullName ?? type.Name;
 
+    void AddPath(string key, string name) =>
+        paths.TryAdd(key, Name(name));
+
+    Collection Name(string name) =>
+        new(order++, name);
+
     // Fills path with the navigation names from the root entity to the end of this Include chain,
     // and returns the root entity type
     static Type IncludeChainRoot(MethodCallExpression node, List<string> path)
@@ -376,7 +414,7 @@ sealed class ProjectionCounter(CollectionCounter counter, ParameterExpression? r
         ExpressionVisitor
     {
         // The collections with no path, which an Include cannot be nested under
-        public int Count { get; private set; }
+        public List<Collection> Collections { get; } = [];
 
         public override Expression? Visit(Expression? node)
         {
@@ -388,13 +426,22 @@ sealed class ProjectionCounter(CollectionCounter counter, ParameterExpression? r
             if (Sequences.IsSequence(node.Type) &&
                 LoadsRows(node, out var navigation))
             {
+                var name = counter.Name(Describe(node, navigation));
+
                 // A projection inside the collection can return further collections, which are
                 // nested in it
                 var inner = new ProjectionCounter(counter, null);
                 inner.VisitSelectors(node);
-                if (!AddProjectedPath(navigation, inner.Count))
+                if (!AddProjectedPath(navigation, inner.Collections))
                 {
-                    Count += Math.Max(1, inner.Count);
+                    if (inner.Collections.Count == 0)
+                    {
+                        Collections.Add(name);
+                    }
+                    else
+                    {
+                        Collections.AddRange(inner.Collections);
+                    }
                 }
 
                 return node;
@@ -403,6 +450,18 @@ sealed class ProjectionCounter(CollectionCounter counter, ParameterExpression? r
             return base.Visit(node);
         }
 
+        // A collection navigation is named for the entity it is read from, and a query for the set
+        // it reads
+        static string Describe(Expression node, MemberExpression? navigation)
+        {
+            if (navigation == null)
+            {
+                return $"DbSet<{Sequences.ElementType(node.Type).Name}>";
+            }
+
+            return $"{navigation.Expression!.Type.Name}.{navigation.Member.Name}";
+        }
+
         // An aggregate, like Count or Any, reads a collection without returning it
         protected override Expression VisitMethodCall(MethodCallExpression node)
         {
@@ -461,7 +520,7 @@ bool LoadsRows(Expression node, out MemberExpression? navigation)
 
         // Records a collection read from the row the projection is given, by the path an Include
         // would name it with. A collection projected twice is loaded twice, so the second has no path.
-        bool AddProjectedPath(MemberExpression? navigation, int inner)
+        bool AddProjectedPath(MemberExpression? navigation, List<Collection> inner)
         {
             if (navigation == null ||
                 row == null)
@@ -495,7 +554,14 @@ bool AddProjectedPath(MemberExpression? navigation, int inner)
                 return false;
             }
 
-            return counter.projectedPaths.TryAdd(PathRoot(row.Type) + path, inner);
+            var key = PathRoot(row.Type) + path;
+            if (!counter.projectedPaths.TryAdd(key, inner))
+            {
+                return false;
+            }
+
+            counter.AddPath(key, row.Type.Name + path);
+            return true;
         }
 
         void VisitSelectors(Expression node)

src/EfQueryComplexity/QueryComplexityViolation.cs

@@ -13,4 +13,11 @@ public sealed record QueryComplexityViolation(string Limit, int? Max, int? Actua
     /// every other level.
     /// </summary>
     public IReadOnlyList<Type>? RowTypes { get; init; }
+
+    /// <summary>
+    /// For MaxSingleQueryCollections, the collections the query loads side by side, in query order,
+    /// each named as the entity it is from and the navigations to it, for example
+    /// <c>Department.Employees</c>. Null for every other level.
+    /// </summary>
+    public IReadOnlyList<string>? Collections { get; init; }
 }

src/EfQueryComplexity/QueryShape.cs

@@ -8,6 +8,6 @@ readonly record struct QueryShape(
     int NavigationDepth,
     int Includes,
     int IncludeDepth,
-    int SingleQueryCollections,
+    IReadOnlyList<string> SingleQueryCollections,
     IReadOnlyList<Type> UnboundedTypes,
     IReadOnlyList<Type> UnboundedTypesWhenListsLimited);

src/EfQueryComplexity/Violations.cs

@@ -12,7 +12,16 @@ public static List<QueryComplexityViolation> ForShape(QueryShape shape, QueryCom
         Add(violations, nameof(QueryComplexityLimits.MaxNavigationDepth), limits.MaxNavigationDepth, shape.NavigationDepth);
         Add(violations, nameof(QueryComplexityLimits.MaxIncludes), limits.MaxIncludes, shape.Includes);
         Add(violations, nameof(QueryComplexityLimits.MaxIncludeDepth), limits.MaxIncludeDepth, shape.IncludeDepth);
-        Add(violations, nameof(QueryComplexityLimits.MaxSingleQueryCollections), limits.MaxSingleQueryCollections, shape.SingleQueryCollections);
+
+        var collections = shape.SingleQueryCollections;
+        if (collections.Count > limits.MaxSingleQueryCollections)
+        {
+            violations.Add(
+                new(nameof(QueryComplexityLimits.MaxSingleQueryCollections), limits.MaxSingleQueryCollections, collections.Count)
+                {
+                    Collections = collections
+                });
+        }
 
         // A key looked up in a list returns a row for each value in it, so only bounds the query when
         // MaxInValues limits the list
@@ -124,6 +133,12 @@ static string Describe(QueryComplexityViolation violation)
             return $"RejectUnbounded: the query returns {names} rows without a Take";
         }
 
+        if (violation.Collections != null)
+        {
+            var names = string.Join(", ", violation.Collections);
+            return $"{violation.Limit}: {violation.Actual} exceeds {violation.Max}. The query loads {names} side by side. Load one of them in a query of its own, or use AsSplitQuery()";
+        }
+
         return $"{violation.Limit}: {violation.Actual} exceeds {violation.Max}";
     }
 }

src/Tests/ShapeTests.SingleQueryCollectionsMessage.verified.txt

@@ -0,0 +1,23 @@
+{
+  Type: QueryComplexityException,
+  Violations: [
+    {
+      Limit: MaxSingleQueryCollections,
+      Max: 1,
+      Actual: 2,
+      Collections: [
+        Employee.Tasks,
+        Employee.Department.Employees
+      ]
+    }
+  ],
+  Message:
+Query complexity limits exceeded:
+ * MaxSingleQueryCollections: 2 exceeds 1. The query loads Employee.Tasks, Employee.Department.Employees side by side. Load one of them in a query of its own, or use AsSplitQuery()
+Change a level for this query with WithQueryComplexity(), skip every check with IgnoreQueryComplexity(), or change the levels passed to UseQueryComplexity().
+Query:
+DbSet<Employee>()
+    .Include(_ => _.Tasks)
+    .Include(_ => _.Department)
+    .ThenInclude(_ => _.Employees)
+}