Skip to content

EF-359: Fix Count/LongCount over embedded collections in projections - #339

Merged
ajcvickers merged 2 commits into
mongodb:mainfrom
ajcvickers:EF-359
Oct 19, 2026
Merged

ajcvickers merged 2 commits into
mongodb:mainfrom
ajcvickers:EF-359

Conversation

@ajcvickers

Copy link
Copy Markdown
Collaborator

Supersedes #337

b.Posts.Count(p => p.Rank > 0) and b.Posts.Count() inside a Select projection over an owned (embedded) collection threw InvalidOperationException at translation time in every MongoQueryMode, because MongoProjectionBindingExpressionVisitor had no case for the lowered Queryable.Count(Queryable.AsQueryable(EF.Property(...))) shape and fell into the generic fallback, which tried to visit the predicate lambda's own parameter as an unbound reference.

Bind Count/LongCount (with or without a predicate) to the original method-call subtree instead of the materializing shaper, so it stays push-down-able to the driver's own LINQ v3 translation rather than materializing owned entities (which EF Core's tracking-materializer rejects without an owner in the result).

That push-down path also has a null-safety gap for the bare/unfiltered form: the driver renders it as a server-side $size, which throws on a missing/null stored array (unlike the predicated form, which renders as a null-tolerant $map/$sum). Add a narrowly-scoped rewrite in
MongoEFToLinqTranslatingExpressionVisitor that null-guards genuinely embedded navigations with $cond, without touching other AsQueryable(...) shapes (e.g. IGrouping sources in GroupBy/Union pipelines).

This also fixes and subsumes EF-357 (bare/arithmetic-wrapped Count over an embedded collection), including a tracking-query case EF-357's own fix did not handle (its client-side materialization approach hits the same owned-entity-without-owner rejection under a tracking query).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes server-side translation of Count and LongCount projections over embedded collections.

Changes:

  • Binds collection counts directly for LINQ v3 translation.
  • Adds null guards for unfiltered counts.
  • Adds functional coverage for filtered, arithmetic, tracking, and null cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
MongoProjectionBindingExpressionVisitor.cs Binds embedded collection count projections.
MongoEFToLinqTranslatingExpressionVisitor.cs Rewrites unfiltered counts with null handling.
OwnedEntityTests.cs Adds embedded collection count regression tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ajcvickers ajcvickers added the move-with-main Tag to indicate which PRs need to be rebased when main moves. label Aug 18, 2026
@ajcvickers
ajcvickers force-pushed the EF-359 branch 3 times, most recently from a2f66ea to 17a5f25 Compare August 24, 2026 10:14
ajcvickers added a commit to ajcvickers/mongo-efcore-provider that referenced this pull request Aug 24, 2026
ajcvickers added a commit to ajcvickers/mongo-efcore-provider that referenced this pull request Sep 30, 2026
@damieng

damieng commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

@damieng damieng left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Interface-typed owned collections still throw. Activator.CreateInstance in MongoEFToLinqTranslatingExpressionVisitor.cs can't build IList<T>, ICollection<T>, IEnumerable<T> or T[]. A repro threw MissingMethodException on EF10 and EF8. The suggested fix is Array.CreateInstance(elementType, 0) for those types, plus an IList<T> test.
  2. Entity plus Count still fails. Select(b => new { b, N = b.Posts.Count }) throws The property 'Blog.N' could not be found, because the mixed-projection visitor doesn't recognise the new Count leaf.
  3. Nested owned Count still fails (nit). b.Posts.Select(p => p.Tags.Count()) throws the "owned entity without a corresponding owner" error.

All three threw before the PR too, so none is a regression but we probably want this to be comprehensive.

@ajcvickers

Copy link
Copy Markdown
Collaborator Author

@damieng Comment 3 is complicated to fix and not a regression. If it's really more than just a nit, then we should file an issue for it.

@ajcvickers
ajcvickers requested a review from damieng October 6, 2026 14:54
@ajcvickers

Copy link
Copy Markdown
Collaborator Author

@damieng Looks like I still need a re-review of this.

ajcvickers and others added 2 commits October 19, 2026 11:16
- Build the empty fallback collection for interface-typed owned collections
  (IList<T>, ICollection<T>, IEnumerable<T>) 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 <noreply@anthropic.com>
@ajcvickers
ajcvickers merged commit 26113e6 into mongodb:main Oct 19, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

move-with-main Tag to indicate which PRs need to be rebased when main moves.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants