Count only side by side collections in MaxSingleQueryCollections

claude.md

@@ -40,7 +40,7 @@ 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, and nor does an Include that Entity Framework ignores, because the query returns no entity for it to load into |
+| `CollectionCounter.cs` | The collections one SQL query loads side by side, a nested chain counting as one, 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 |

readme.md

@@ -21,7 +21,7 @@ The checks bound what one query can ask for, whichever layer built it:
 | Requesting every row | `RejectUnbounded`, `MaxTake` |
 | Deeply nested or very large queries | `MaxNodes`, `MaxDepth`, `MaxOperators` |
 | Long navigation chains and includes, which multiply joins | `MaxNavigationDepth`, `MaxIncludes`, `MaxIncludeDepth` |
-| Several collections in one query, which multiply rows | `MaxSingleQueryCollections` |
+| Collections side by side in one query, which multiply rows | `MaxSingleQueryCollections` |
 | Huge `IN` lists | `MaxInValues` |
 | A query that passes every check but is still expensive | [SQL Server cost limit](#sql-server-cost-limit) |
 
@@ -123,7 +123,7 @@ A query is checked against the throw levels before the log levels, so a throw le
 | `MaxNavigationDepth` | Navigations in one member access chain | While compiled | 3 |
 | `MaxIncludes` | `Include` calls | While compiled | 6 |
 | `MaxIncludeDepth` | Navigations in one `Include` chain | While compiled | 3 |
-| `MaxSingleQueryCollections` | Collections one SQL query loads | While compiled | 1 |
+| `MaxSingleQueryCollections` | Collections one SQL query loads side by side | 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` or lookup by key | While compiled | `All` |
@@ -143,9 +143,11 @@ It does not count:
 
 ### Collections in a single query
 
-A single SQL query joins every collection it loads, so each multiplies the rows returned for the others. Loading 10 departments with 50 employees and 20 projects each returns 10,000 rows for 710 entities. This is a [cartesian explosion](https://learn.microsoft.com/en-us/ef/core/querying/single-split-queries).
+A single SQL query joins every collection it loads, so collections side by side multiply each other's rows. Loading 10 departments with 50 employees and 20 projects each returns 10,000 rows for 710 entities. This is a [cartesian explosion](https://learn.microsoft.com/en-us/ef/core/querying/single-split-queries).
 
-`MaxSingleQueryCollections` counts:
+A collection nested in another does not multiply. Loading departments, their employees, and each employee's tasks returns a row per task, so the chain counts as one. A collection holding several side by side counts as many as it holds, so departments with both employees and projects counts as two.
+
+`MaxSingleQueryCollections` counts, side by side in that way:
 
  * Collection navigations in `Include` and `ThenInclude`, including string paths. A collection restated to `ThenInclude` something below it counts once.
  * Collections a projection returns, for example `Employees = _.Employees.ToList()`, including collections nested in them.
@@ -159,7 +161,7 @@ It does not count:
  * 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.
+The log default of 1 allows one chain of nested collections, and logs as soon as two load side by side.
 
 
 ### Unbounded queries

src/Directory.Build.props

@@ -5,7 +5,7 @@
     <Nullable>enable</Nullable>
     <LangVersion>latest</LangVersion>
     <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
-    <Version>0.7.0</Version>
+    <Version>0.8.0</Version>
     <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
     <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
     <AssemblyVersion>1.0.0</AssemblyVersion>

src/EfQueryComplexity/CollectionCounter.cs

@@ -1,13 +1,15 @@
 /// <summary>
-/// Counts the collections one SQL query loads: collection Includes, and collections a projection
-/// returns.
+/// Counts the collections one SQL query loads side by side: collection Includes, and collections a
+/// projection returns.
 /// </summary>
 /// <remarks>
-/// 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. Nor is an Include that Entity Framework ignores, since the query
-/// returns no entity for it to load into.
+/// A single query joins every collection it loads. Collections side by side multiply each other's
+/// rows, a cartesian explosion. A collection nested in another does not: the query returns a row per
+/// item of the inner one, so a chain of nested collections counts as one. A collection holding
+/// several side by side counts as many as it holds. 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. 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
@@ -18,9 +20,11 @@ sealed class CollectionCounter(IModel model) :
     HashSet<MethodCallExpression> ignoredIncludes = [];
 
     // The collections a projection returns in place of the entity they belong to, keyed like
-    // includePaths. An Include of one of them only says what to load under it, so the two are one
-    // collection.
-    HashSet<string> projectedPaths = [];
+    // 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 = [];
+
+    // What projections load that has no path, so cannot be nested under an Include
     int projected;
 
     public static int Count(Expression query, IModel model, bool splitByDefault)
@@ -32,8 +36,55 @@ public static int Count(Expression query, IModel model, bool splitByDefault)
 
         var counter = new CollectionCounter(model);
         counter.Visit(query);
-        counter.includePaths.ExceptWith(counter.projectedPaths);
-        return counter.includePaths.Count + counter.projected;
+        return counter.CountPaths() + counter.projected;
+    }
+
+    // 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()
+    {
+        var held = new Dictionary<string, int>(projectedPaths);
+        foreach (var path in includePaths)
+        {
+            held.TryAdd(path, 0);
+        }
+
+        var total = 0;
+        foreach (var path in held.Keys.OrderByDescending(_ => _.Length).ToList())
+        {
+            var count = Math.Max(1, held[path]);
+            var holder = Holder(path, held);
+            if (holder == null)
+            {
+                total += count;
+            }
+            else
+            {
+                held[holder] += count;
+            }
+        }
+
+        return total;
+    }
+
+    static string? Holder(string path, Dictionary<string, int> paths)
+    {
+        var current = path;
+        while (true)
+        {
+            var index = current.LastIndexOf('.');
+            if (index < 0)
+            {
+                return null;
+            }
+
+            current = current[..index];
+            if (paths.ContainsKey(current))
+            {
+                return current;
+            }
+        }
     }
 
     // AsSplitQuery and AsSingleQuery apply to the whole query, and override the context default
@@ -89,7 +140,9 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
             }
 
             Visit(node.Arguments[0]);
-            new ProjectionCounter(this, RowNotReturned(selector)).Visit(selector);
+            var projection = new ProjectionCounter(this, RowNotReturned(selector));
+            projection.Visit(selector);
+            projected += projection.Count;
             return node;
         }
 
@@ -316,11 +369,15 @@ bool IsCollectionNavigation(Type type)
     }
 
     /// <summary>
-    /// Counts the collections a projection returns, including collections nested in them.
+    /// Counts the collections a projection returns side by side, a collection counting as what it
+    /// holds when that is more than one.
     /// </summary>
     sealed class ProjectionCounter(CollectionCounter counter, ParameterExpression? row) :
         ExpressionVisitor
     {
+        // The collections with no path, which an Include cannot be nested under
+        public int Count { get; private set; }
+
         public override Expression? Visit(Expression? node)
         {
             if (node == null)
@@ -331,11 +388,15 @@ sealed class ProjectionCounter(CollectionCounter counter, ParameterExpression? r
             if (Sequences.IsSequence(node.Type) &&
                 LoadsRows(node, out var navigation))
             {
-                counter.projected++;
-                AddProjectedPath(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))
+                {
+                    Count += Math.Max(1, inner.Count);
+                }
 
-                // A projection inside the collection can return further collections
-                VisitSelectors(node);
                 return node;
             }
 
@@ -399,13 +460,13 @@ 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
-        void AddProjectedPath(MemberExpression? navigation)
+        // would name it with. A collection projected twice is loaded twice, so the second has no path.
+        bool AddProjectedPath(MemberExpression? navigation, int inner)
         {
             if (navigation == null ||
                 row == null)
             {
-                return;
+                return false;
             }
 
             var path = "";
@@ -429,10 +490,12 @@ void AddProjectedPath(MemberExpression? navigation)
                 break;
             }
 
-            if (current == row)
+            if (current != row)
             {
-                counter.projectedPaths.Add(PathRoot(row.Type) + path);
+                return false;
             }
+
+            return counter.projectedPaths.TryAdd(PathRoot(row.Type) + path, inner);
         }
 
         void VisitSelectors(Expression node)

src/EfQueryComplexity/QueryComplexityLimits.cs

@@ -18,8 +18,10 @@
 /// <param name="MaxIncludes">Maximum number of Include calls.</param>
 /// <param name="MaxIncludeDepth">Maximum number of navigations in one Include chain.</param>
 /// <param name="MaxSingleQueryCollections">
-/// Maximum number of collections one SQL query loads, through collection Includes and collections in
-/// a projection. Each multiplies the rows returned for the others. A split query counts none.
+/// Maximum number of collections one SQL query loads side by side, through collection Includes and
+/// collections in a projection. Collections side by side multiply each other's rows. A chain of nested
+/// collections counts as one, since it returns a row per item of the innermost. A split query counts
+/// none.
 /// </param>
 /// <param name="MaxTake">Maximum value passed to Take. Checked on every execution.</param>
 /// <param name="MaxInValues">Maximum number of values in a list the query sends, such as a Contains list. Checked on every execution.</param>

src/Tests/ShapeTests.cs

@@ -243,7 +243,44 @@ public async Task LogsOncePerShape()
 
     [Test]
     public Task SingleQueryCollectionsLevel() =>
-        AssertLevel(context => context.Companies.Include(_ => _.Departments).ThenInclude(_ => _.Employees), SingleQueryCollections);
+        AssertLevel(context => context.Employees.Include(_ => _.Tasks).Include(_ => _.Department.Employees), SingleQueryCollections);
+
+    // Tasks and the department's employees are side by side, so multiply each other's rows
+    [Test]
+    public async Task SingleQueryCollectionsCountsSideBySideIncludes() =>
+        await Assert.That(
+                Measure(
+                    context => context.Employees
+                        .Include(_ => _.Tasks)
+                        .Include(_ => _.Department)
+                        .ThenInclude(_ => _.Employees),
+                    SingleQueryCollections))
+            .IsEqualTo(2);
+
+    // A row per task, so the chain is one collection
+    [Test]
+    public async Task SingleQueryCollectionsCountsNestedIncludesOnce() =>
+        await Assert.That(
+                Measure(
+                    context => context.Companies
+                        .Include(_ => _.Departments)
+                        .ThenInclude(_ => _.Employees)
+                        .ThenInclude(_ => _.Tasks),
+                    SingleQueryCollections))
+            .IsEqualTo(1);
+
+    // Each department holds its employees and its company's departments side by side
+    [Test]
+    public async Task SingleQueryCollectionsCountsSideBySideIncludesInANestedOne() =>
+        await Assert.That(
+                Measure(
+                    context => context.Companies
+                        .Include(_ => _.Departments)
+                        .ThenInclude(_ => _.Employees)
+                        .Include(_ => _.Departments)
+                        .ThenInclude(_ => _.Company.Departments),
+                    SingleQueryCollections))
+            .IsEqualTo(2);
 
     // Departments is restated to ThenInclude below it, and is one collection
     [Test]
@@ -256,15 +293,15 @@ await Assert.That(
                         .Include(_ => _.Departments)
                         .ThenInclude(_ => _.Company),
                     SingleQueryCollections))
-            .IsEqualTo(2);
+            .IsEqualTo(1);
 
     [Test]
     public async Task SingleQueryCollectionsCountsStringInclude() =>
         await Assert.That(
                 Measure(
-                    context => context.Companies.Include("Departments.Employees.Tasks"),
+                    context => context.Employees.Include("Tasks").Include("Department.Employees.Tasks"),
                     SingleQueryCollections))
-            .IsEqualTo(3);
+            .IsEqualTo(2);
 
     [Test]
     public async Task SingleQueryCollectionsCountsProjectedCollections() =>
@@ -283,6 +320,45 @@ await Assert.That(
                                 .ToList()
                         }),
                     SingleQueryCollections))
+            .IsEqualTo(1);
+
+    [Test]
+    public async Task SingleQueryCollectionsCountsSideBySideProjectedCollections() =>
+        await Assert.That(
+                Measure(
+                    context => context.Departments.Select(
+                        _ => new
+                        {
+                            _.Name,
+                            Employees = _.Employees.ToList(),
+                            Siblings = _.Company.Departments
+                                .Select(department => new
+                                {
+                                    department.Name,
+                                    Employees = department.Employees.ToList()
+                                })
+                                .ToList()
+                        }),
+                    SingleQueryCollections))
+            .IsEqualTo(2);
+
+    // Each department returns its employees and their tasks side by side
+    [Test]
+    public async Task SingleQueryCollectionsCountsSideBySideCollectionsInAProjectedOne() =>
+        await Assert.That(
+                Measure(
+                    context => context.Companies.Select(
+                        _ => new
+                        {
+                            Departments = _.Departments
+                                .Select(department => new
+                                {
+                                    Employees = department.Employees.ToList(),
+                                    Tasks = department.Employees.SelectMany(employee => employee.Tasks).ToList()
+                                })
+                                .ToList()
+                        }),
+                    SingleQueryCollections))
             .IsEqualTo(2);
 
     // AsSingleQuery overrides a context that splits by default
@@ -399,7 +475,7 @@ await Assert.That(
                                 Company = _
                             }),
                     SingleQueryCollections))
-            .IsEqualTo(2);
+            .IsEqualTo(1);
 
     // Entity Framework carries the ThenInclude onto the department the projection returns
     [Test]
@@ -423,7 +499,7 @@ await Assert.That(
                         .ThenInclude(_ => _.Employees)
                         .Select(_ => Describe(_)),
                     SingleQueryCollections))
-            .IsEqualTo(2);
+            .IsEqualTo(1);
 
     // The Include only says what to load under the collection the projection returns
     [Test]
@@ -454,7 +530,7 @@ await Assert.That(
                                 Departments = _.Departments.ToList()
                             }),
                     SingleQueryCollections))
-            .IsEqualTo(2);
+            .IsEqualTo(1);
 
     // The Include loads the tasks into the employee, and the projection loads them again
     [Test]