From 0e8a6e46c394314e0b4005ed6ea098ea99ece32c Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Mon, 24 Aug 2026 10:59:48 +0100 Subject: [PATCH 1/2] EF-359: Fix Count/LongCount over embedded collections in projections --- ...ngoEFToLinqTranslatingExpressionVisitor.cs | 67 +++++++ ...MongoProjectionBindingExpressionVisitor.cs | 33 ++++ .../Query/OwnedEntityTests.cs | 170 ++++++++++++++++++ 3 files changed, 270 insertions(+) diff --git a/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoEFToLinqTranslatingExpressionVisitor.cs b/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoEFToLinqTranslatingExpressionVisitor.cs index 1cea8cc83..056ad8131 100644 --- a/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoEFToLinqTranslatingExpressionVisitor.cs +++ b/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoEFToLinqTranslatingExpressionVisitor.cs @@ -865,6 +865,15 @@ protected override Expression VisitMethodCall(MethodCallExpression node) return zeroTakeRewrite; } + // A bare/unfiltered embedded (owned) collection-navigation Count, e.g. `b.Posts.Count`, lowered by + // EF Core to Queryable.Count(Queryable.AsQueryable(EF.Property(shaper, "Posts"))). See + // TryRewriteEmbeddedCollectionNavigationCount for why this needs a null/missing-safe normalization + // the driver's own rendering of a bare Count (a server-side $size) doesn't provide. + if (TryRewriteEmbeddedCollectionNavigationCount(node, out var embeddedCountRewrite)) + { + return embeddedCountRewrite; + } + return base.VisitMethodCall(node); } @@ -908,6 +917,64 @@ private void MarkJoinKeyOperands(Expression key) Expression.Quote(falsePredicate))); } + /// + /// Rewrites a bare embedded (owned) collection-navigation Count/LongCount (e.g. + /// b.Posts.Count) into Enumerable.Count over a ??-normalized array read — needed + /// because the driver's bare-Count translation is a server-side $size, which throws on a + /// missing or null array (unlike a predicated Count, translated as null-tolerant $map/$sum). + /// + private bool TryRewriteEmbeddedCollectionNavigationCount(MethodCallExpression node, out Expression result) + { + result = null!; + + if (node.Method.DeclaringType != typeof(Queryable) + || node.Arguments.Count != 1 + || node.Method.Name is not (nameof(Queryable.Count) or nameof(Queryable.LongCount))) + { + return false; + } + + if (node.Arguments[0] is not MethodCallExpression + { + Method: { Name: nameof(Queryable.AsQueryable), DeclaringType: var asQueryableDeclaring } + } asQueryableCall + || asQueryableDeclaring != typeof(Queryable)) + { + return false; + } + + // Narrow to genuinely embedded (owned) collection navigations only: AsQueryable(...) also shows + // up wrapping other enumerable sources (e.g. an IGrouping in a GroupBy/Union pipeline) that are + // not a document field read at all, so rewriting unconditionally miscompiles those shapes. + if (asQueryableCall.Arguments[0] is not MethodCallExpression efPropertyCall + || !efPropertyCall.Method.IsEFPropertyMethod() + || efPropertyCall.Arguments[1] is not ConstantExpression { Value: string propertyName } + || _queryContext.Context.Model.FindEntityType(efPropertyCall.Arguments[0].Type) is not { } sourceEntityType + || sourceEntityType.FindNavigation(propertyName) is not { } navigation + || !navigation.IsEmbedded()) + { + return false; + } + + var fieldAccess = Visit(asQueryableCall.Arguments[0]); + var elementType = fieldAccess?.Type.TryGetItemType(); + if (elementType == null) + { + return false; + } + + var countMethod = (node.Method.Name == nameof(Queryable.LongCount) + ? EnumerableLongCountMethod + : EnumerableCountMethod) + .MakeGenericMethod(elementType); + + var emptyCollection = Expression.Constant(Activator.CreateInstance(fieldAccess!.Type), fieldAccess.Type); + var normalizedFieldAccess = Expression.Coalesce(fieldAccess, emptyCollection); + + result = Expression.Call(null, countMethod, normalizedFieldAccess); + return true; + } + /// /// Rewrites Queryable.Count(Queryable.Where(DbSet<Target>(), fkPredicate)) (and the /// LongCount variant) — the lowered form of a projected/filtered collection-navigation count diff --git a/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoProjectionBindingExpressionVisitor.cs b/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoProjectionBindingExpressionVisitor.cs index 7443b8ca1..02b25fd22 100644 --- a/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoProjectionBindingExpressionVisitor.cs +++ b/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoProjectionBindingExpressionVisitor.cs @@ -451,6 +451,39 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp EnumerableMethods.Select.MakeGenericMethod(method.GetGenericArguments()), shaper, lambda); + + // A Count/LongCount over an embedded/owned collection navigation in a projection (e.g. + // `b.Posts.Count(p => p.Rank > 0)`). Kept bound to the original method-call subtree, not + // the shaper, so it push-downs to the driver's native $size/$filter — routing through the + // shaper would materialize owned Post entities, which the tracking-materializer rejects. + case nameof(Queryable.Count) when genericMethod == QueryableMethods.CountWithoutPredicate: + case nameof(Queryable.LongCount) when genericMethod == QueryableMethods.LongCountWithoutPredicate: + case nameof(Queryable.Count) when genericMethod == QueryableMethods.CountWithPredicate: + case nameof(Queryable.LongCount) when genericMethod == QueryableMethods.LongCountWithPredicate: + if (visitedSource is not CollectionShaperExpression + || methodCallExpression.Arguments[0] is not MethodCallExpression + { + Method: { Name: nameof(Queryable.AsQueryable), DeclaringType: var asQueryableDeclaring } + } asQueryableCall + || asQueryableDeclaring != typeof(Queryable)) + { + return null; + } + + // A missing/null stored array materializes to a null reference, not an empty + // collection (same gap as `.Select(e => e.Foo)`), so Count/LongCount must tolerate a + // null source ($size/$filter both reject null) — guard on the raw field access, not + // the AsQueryable-wrapped source. + var countSource = asQueryableCall.Arguments[0]; + Expression nullSafeCount = Expression.Condition( + Expression.Equal(countSource, Expression.Constant(null, countSource.Type)), + Expression.Default(methodCallExpression.Type), + methodCallExpression); + + var countProjectionMember = GetCurrentProjectionMember(); + _projectionMapping[countProjectionMember] = nullSafeCount; + + return new ProjectionBindingExpression(_queryExpression, countProjectionMember, methodCallExpression.Type); } } diff --git a/tests/MongoDB.EntityFrameworkCore.FunctionalTests/Query/OwnedEntityTests.cs b/tests/MongoDB.EntityFrameworkCore.FunctionalTests/Query/OwnedEntityTests.cs index 4d6e6b0b9..79e98e804 100644 --- a/tests/MongoDB.EntityFrameworkCore.FunctionalTests/Query/OwnedEntityTests.cs +++ b/tests/MongoDB.EntityFrameworkCore.FunctionalTests/Query/OwnedEntityTests.cs @@ -15,7 +15,9 @@ using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.Extensions.Logging; using MongoDB.Bson; +using MongoDB.EntityFrameworkCore.Diagnostics; using MongoDB.EntityFrameworkCore.Extensions; namespace MongoDB.EntityFrameworkCore.FunctionalTests.Query; @@ -1573,6 +1575,174 @@ public void OwnedEntity_can_go_multiple_levels_deep_querying_reference() } } + [Fact] + public void OwnedEntity_filtered_count_in_projection_translates() + { + var collection = database.CreateCollection(); + + { + using var db = SingleEntityDbContext.Create(collection); + db.Entities.Add(new CountBlog + { + _id = "1", + Title = "Blog1", + Posts = [new CountPost { Rank = 1 }, new CountPost { Rank = 0 }, new CountPost { Rank = 2 }] + }); + db.SaveChanges(); + } + + { + // Assert the count is evaluated server-side (a $map/$sum over the array, equivalent to + // filter+size) rather than by materializing the owned CountPost entities and counting client-side. + var (loggerFactory, spyLogger) = SpyLoggerProvider.Create(); + using var db = SingleEntityDbContext.Create(collection, loggerFactory, + optionsBuilderAction: o => o.EnableSensitiveDataLogging()); + var result = Assert.Single(db.Entities.Select(b => new { b.Title, N = b.Posts.Count(p => p.Rank > 0) })); + + Assert.Equal("Blog1", result.Title); + Assert.Equal(2, result.N); + + var message = spyLogger.GetLogMessageByEventId(MongoEventId.ExecutedMqlQuery); + Assert.Contains("$map", message); + Assert.Contains("$sum", message); + } + } + + [Fact] + public void OwnedEntity_unfiltered_count_in_projection_translates() + { + var collection = database.CreateCollection(); + + { + using var db = SingleEntityDbContext.Create(collection); + db.Entities.Add(new CountBlog + { + _id = "1", + Title = "Blog1", + Posts = [new CountPost { Rank = 1 }, new CountPost { Rank = 0 }] + }); + db.SaveChanges(); + } + + { + using var db = SingleEntityDbContext.Create(collection); + var result = Assert.Single(db.Entities.Select(b => new { b.Title, N = b.Posts.Count() })); + + Assert.Equal("Blog1", result.Title); + Assert.Equal(2, result.N); + } + } + + [Fact] + public void OwnedEntity_collection_bare_count_projection_returns_element_count() + { + var collection = database.CreateCollection(); + + { + using var db = SingleEntityDbContext.Create(collection); + db.Entities.AddRange( + new A { _id = "1", children = [new B { name = "child1" }, new B { name = "child2" }] }, + new A { _id = "2", children = [] }, + new A { _id = "3", children = null! }); + db.SaveChanges(); + } + + // Deliberately a default *tracking* query (unlike EF-357's own tests, which required + // AsNoTracking): the bare Count must stay a server-side scalar, not a materialized owned-entity + // shaper, or EF Core's tracking-materializer rejects it ("owned entity without a corresponding + // owner"). + var (loggerFactory, spyLogger) = SpyLoggerProvider.Create(); + using var db2 = SingleEntityDbContext.Create(collection, loggerFactory, + optionsBuilderAction: o => o.EnableSensitiveDataLogging()); + + var counts = db2.Entities.OrderBy(e => e._id).Select(e => e.children.Count).ToList(); + Assert.Equal([2, 0, 0], counts); + + // A null stored array must report 0, not throw: the driver renders a bare Count as a server-side + // $size, which rejects a null array, so this needs an $ifNull normalization first. + var message = spyLogger.GetLogMessageByEventId(MongoEventId.ExecutedMqlQuery); + Assert.Contains("$ifNull", message); + Assert.Contains("$size", message); + + var longCounts = db2.Entities.OrderBy(e => e._id).Select(e => e.children.LongCount()).ToList(); + Assert.Equal([2L, 0L, 0L], longCounts); + + var filteredLongCounts = db2.Entities.OrderBy(e => e._id) + .Select(e => e.children.LongCount(c => c.name == "child1")) + .ToList(); + Assert.Equal([1L, 0L, 0L], filteredLongCounts); + } + + [Fact] + public void OwnedEntity_collection_bare_count_projection_over_missing_element_returns_zero() + { + // A document where the array element is OMITTED entirely (not stored as BSON null) is a distinct + // case from an explicit null: in an aggregation expression a missing field is not equal to BSON + // null, so a plain null-equality guard does not catch it and $size still throws on "missing". Only + // $ifNull (which normalizes missing the same as null) handles both. + var collection = database.CreateCollection(); + database.GetCollection(collection.CollectionNamespace).InsertOne(new BsonDocument("_id", "1")); + + using var db = SingleEntityDbContext.Create(collection); + var counts = db.Entities.Select(e => e.children.Count).ToList(); + + Assert.Equal([0], counts); + } + + [Fact] + public void OwnedEntity_collection_count_projection_wrapped_in_arithmetic_returns_element_count() + { + var collection = database.CreateCollection(); + + { + using var db = SingleEntityDbContext.Create(collection); + db.Entities.AddRange( + new A { _id = "1", children = [new B { name = "child1" }, new B { name = "child2" }] }, + new A { _id = "2", children = null! }); + db.SaveChanges(); + } + + using var db2 = SingleEntityDbContext.Create(collection); + var doubled = db2.Entities.OrderBy(e => e._id) + .Select(e => new { e._id, N = e.children.Count * 2 }) + .ToList(); + + Assert.Equal([4, 0], doubled.Select(r => r.N).ToArray()); + } + + [Fact] + public void OwnedEntity_collection_filtered_count_projection_with_null_children_returns_zero() + { + var collection = database.CreateCollection(); + + { + using var db = SingleEntityDbContext.Create(collection); + db.Entities.AddRange( + new A { _id = "1", children = [new B { name = "child1" }, new B { name = "child2" }] }, + new A { _id = "2", children = null! }); + db.SaveChanges(); + } + + using var db2 = SingleEntityDbContext.Create(collection); + var counts = db2.Entities.OrderBy(e => e._id) + .Select(e => e.children.Count(c => c.name == "child1")) + .ToList(); + + Assert.Equal([1, 0], counts); + } + + record CountBlog + { + public string _id { get; set; } + public string Title { get; set; } + public List Posts { get; set; } + } + + record CountPost + { + public int Rank { get; set; } + } + record A { public string _id { get; set; } From d7e23eefb7b2b3061532485bcad5c7ece1cee0f9 Mon Sep 17 00:00:00 2001 From: ajcvickers Date: Tue, 6 Oct 2026 14:52:58 +0100 Subject: [PATCH 2/2] EF-359: Address review feedback on embedded collection Count - Build the empty fallback collection for interface-typed owned collections (IList, ICollection, IEnumerable) instead of Activator.CreateInstance. - Bind an unfiltered embedded Count/LongCount alongside a whole entity in the mixed projection path by counting the stored array client-side. - Add tests for both. Co-Authored-By: Claude Sonnet 5.5 --- ...ngoEFToLinqTranslatingExpressionVisitor.cs | 9 ++- ...jectionBindingRemovingExpressionVisitor.cs | 47 +++++++++++++ .../Query/OwnedEntityTests.cs | 68 +++++++++++++++++++ 3 files changed, 123 insertions(+), 1 deletion(-) diff --git a/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoEFToLinqTranslatingExpressionVisitor.cs b/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoEFToLinqTranslatingExpressionVisitor.cs index 056ad8131..4a1a57274 100644 --- a/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoEFToLinqTranslatingExpressionVisitor.cs +++ b/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoEFToLinqTranslatingExpressionVisitor.cs @@ -968,13 +968,20 @@ private bool TryRewriteEmbeddedCollectionNavigationCount(MethodCallExpression no : EnumerableCountMethod) .MakeGenericMethod(elementType); - var emptyCollection = Expression.Constant(Activator.CreateInstance(fieldAccess!.Type), fieldAccess.Type); + var emptyCollection = Expression.Constant(CreateEmptyCollection(fieldAccess!.Type, elementType), fieldAccess.Type); var normalizedFieldAccess = Expression.Coalesce(fieldAccess, emptyCollection); result = Expression.Call(null, countMethod, normalizedFieldAccess); return true; } + // Interface-typed (IList, ICollection, IEnumerable, ...) collection properties can't be + // built by Activator.CreateInstance; an empty T[] is assignable to all of them. + private static object CreateEmptyCollection(Type collectionType, Type elementType) + => collectionType.IsInterface + ? Array.CreateInstance(elementType, 0) + : Activator.CreateInstance(collectionType)!; + /// /// Rewrites Queryable.Count(Queryable.Where(DbSet<Target>(), fkPredicate)) (and the /// LongCount variant) — the lowered form of a projected/filtered collection-navigation count diff --git a/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoMixedProjectionBindingRemovingExpressionVisitor.cs b/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoMixedProjectionBindingRemovingExpressionVisitor.cs index fd0a207b8..48f31badb 100644 --- a/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoMixedProjectionBindingRemovingExpressionVisitor.cs +++ b/src/MongoDB.EntityFrameworkCore/Query/Visitors/MongoMixedProjectionBindingRemovingExpressionVisitor.cs @@ -14,6 +14,7 @@ */ using System; +using System.Linq; using System.Linq.Expressions; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Diagnostics; @@ -21,6 +22,7 @@ using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Query; using MongoDB.Bson; +using MongoDB.EntityFrameworkCore.Extensions; using MongoDB.EntityFrameworkCore.Query.Expressions; using MongoDB.EntityFrameworkCore.Storage; @@ -108,6 +110,14 @@ protected override Expression VisitExtension(Expression extensionExpression) return navMemberRead; } + // An unfiltered embedded-collection Count/LongCount (e.g. select new { b, N = b.Posts.Count }) + // mixed alongside a whole entity reference: the driver-LINQ Select was stripped, so count the + // stored array client-side from the materialized document. + if (TryBindEmbeddedCollectionCountLeaf(sourceExpression, projectionBindingExpression.Type, out var countRead)) + { + return countRead; + } + // A computed-arithmetic leaf (e.g. select new { c, Total = c.Age * c.Score }) mixed alongside // a whole entity reference. MongoProjectionBindingExpressionVisitor registers the raw binary // expression as a single projection-mapping leaf (see its arithmetic BinaryExpression case); @@ -216,6 +226,43 @@ private bool TryBindNavigationMemberAccess(Expression? mappedExpression, Type re return true; } + /// + /// Binds an unfiltered embedded (owned) collection Count/LongCount projection leaf (e.g. + /// select new { b, N = b.Posts.Count }). maps + /// such a leaf as source == null ? default : Count(AsQueryable(EF.Property(shaper, nav))); here the + /// stored array is read from the materialized document and its length taken, treating a missing or null + /// array as empty. Returns for any other shape (including a filtered count). + /// + private bool TryBindEmbeddedCollectionCountLeaf(Expression? mappedExpression, Type resultType, out Expression result) + { + result = null!; + + if (mappedExpression is not ConditionalExpression + { + IfFalse: MethodCallExpression + { + Method.Name: nameof(Queryable.Count) or nameof(Queryable.LongCount), + Arguments: [MethodCallExpression { Method.Name: nameof(Queryable.AsQueryable), Arguments: [var source] }] + } + } + || source is not MethodCallExpression efPropertyCall + || !efPropertyCall.Method.IsEFPropertyMethod() + || efPropertyCall.Arguments[1] is not ConstantExpression { Value: string navigationName } + || efPropertyCall.Arguments[0] is not StructuralTypeShaperExpression { StructuralType: IEntityType owner } + || owner != _rootEntityType + || owner.FindNavigation(navigationName) is not { } navigation + || !navigation.IsEmbedded()) + { + return false; + } + + var array = BsonBinding.CreateGetBsonArray(_docParameter, navigation.TargetEntityType.GetContainingElementName()!); + Expression count = Expression.Property( + Expression.Coalesce(array, Expression.New(typeof(BsonArray))), nameof(BsonArray.Count)); + result = count.Type == resultType ? count : Expression.Convert(count, resultType); + return true; + } + /// /// Binds a computed-arithmetic projection leaf (e.g. select new { c, Total = c.Age * c.Score }). /// registers such a leaf as the raw diff --git a/tests/MongoDB.EntityFrameworkCore.FunctionalTests/Query/OwnedEntityTests.cs b/tests/MongoDB.EntityFrameworkCore.FunctionalTests/Query/OwnedEntityTests.cs index 79e98e804..1774d9105 100644 --- a/tests/MongoDB.EntityFrameworkCore.FunctionalTests/Query/OwnedEntityTests.cs +++ b/tests/MongoDB.EntityFrameworkCore.FunctionalTests/Query/OwnedEntityTests.cs @@ -1731,6 +1731,74 @@ public void OwnedEntity_collection_filtered_count_projection_with_null_children_ Assert.Equal([1, 0], counts); } + [Fact] + public void OwnedEntity_collection_count_projection_over_interface_typed_collections() + { + var collection = database.CreateCollection(); + + { + using var db = SingleEntityDbContext.Create(collection); + db.Entities.AddRange( + new InterfaceCountBlog + { + _id = "1", + IListPosts = [new CountPost(), new CountPost()], + ICollectionPosts = [new CountPost()], + IEnumerablePosts = [new CountPost(), new CountPost(), new CountPost()] + }, + new InterfaceCountBlog { _id = "2" }); + db.SaveChanges(); + } + + using var db2 = SingleEntityDbContext.Create(collection); + var results = db2.Entities.OrderBy(e => e._id) + .Select(e => new + { + A = e.IListPosts.Count, + B = e.ICollectionPosts.Count, + C = e.IEnumerablePosts.Count() + }) + .ToList(); + + Assert.Equal(2, results[0].A); + Assert.Equal(1, results[0].B); + Assert.Equal(3, results[0].C); + Assert.Equal(0, results[1].A); + Assert.Equal(0, results[1].B); + Assert.Equal(0, results[1].C); + } + + [Fact] + public void OwnedEntity_entity_and_collection_count_in_same_projection() + { + var collection = database.CreateCollection(); + + { + using var db = SingleEntityDbContext.Create(collection); + db.Entities.AddRange( + new CountBlog { _id = "1", Title = "t1", Posts = [new CountPost { Rank = 1 }, new CountPost { Rank = 2 }] }, + new CountBlog { _id = "2", Title = "t2", Posts = null! }); + db.SaveChanges(); + } + + using var db2 = SingleEntityDbContext.Create(collection); + var results = db2.Entities.OrderBy(e => e._id) + .Select(b => new { Blog = b, N = b.Posts.Count }) + .ToList(); + + Assert.Equal(["t1", "t2"], results.Select(r => r.Blog.Title).ToArray()); + Assert.Equal([2, 0], results.Select(r => r.N).ToArray()); + Assert.Equal(2, results[0].Blog.Posts.Count); + } + + record InterfaceCountBlog + { + public string _id { get; set; } + public IList IListPosts { get; set; } + public ICollection ICollectionPosts { get; set; } + public IEnumerable IEnumerablePosts { get; set; } + } + record CountBlog { public string _id { get; set; }