From 67209c4a7ab33a3980baaf162cf362db848ad01e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 05:58:49 +0000 Subject: [PATCH] Fix duplicate joins when materializing writable projectables with includes A writable [Projectable] property triggers a whole-entity re-projection so the property can be populated during materialization. That re-projection read every other mapped member through its compiler-generated backing field (what EF's member metadata resolves auto-properties to). Reading a mapped scalar or navigation through the backing field stops EF's navigation expansion from recognizing it as a plain column/navigation, so EF instead materializes the entire source entity -- including any Include-ed collection navigation -- once per rewritten member, emitting one duplicate JOIN per member (issue #217). Read each member through its CLR property instead, and bind to the property when it is settable so the projection keeps natural column names. EF then treats scalars as columns and collapses the collection navigation into a single join. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_016Xvb9UMRqFJKpocKCCeYcc --- .../Services/ProjectableExpressionReplacer.cs | 66 ++++++++-- .../Issue217Tests.cs | 117 ++++++++++++++++++ ...ueryRootExpression.DotNet10_0.verified.txt | 2 +- ...QueryRootExpression.DotNet8_0.verified.txt | 2 +- ...QueryRootExpression.DotNet9_0.verified.txt | 2 +- ...ueryRootExpression.DotNet10_0.verified.txt | 2 +- ...QueryRootExpression.DotNet8_0.verified.txt | 2 +- ...QueryRootExpression.DotNet9_0.verified.txt | 2 +- 8 files changed, 182 insertions(+), 13 deletions(-) create mode 100644 tests/EntityFrameworkCore.Projectables.FunctionalTests/Issue217Tests.cs diff --git a/src/EntityFrameworkCore.Projectables/Services/ProjectableExpressionReplacer.cs b/src/EntityFrameworkCore.Projectables/Services/ProjectableExpressionReplacer.cs index 72bc0f8a..be3c96a4 100644 --- a/src/EntityFrameworkCore.Projectables/Services/ProjectableExpressionReplacer.cs +++ b/src/EntityFrameworkCore.Projectables/Services/ProjectableExpressionReplacer.cs @@ -341,15 +341,14 @@ private Expression _AddProjectableSelect(Expression node, IEntityType entityType return node; } - var properties = entityType.GetProperties() + var members = entityType.GetProperties() .Where(x => !x.IsShadowProperty()) - .Select(x => x.GetMemberInfo(false, false)) + .Cast() .Concat(entityType.GetNavigations() - .Where(x => !x.IsShadowProperty()) - .Select(x => x.GetMemberInfo(false, false))) + .Where(x => !x.IsShadowProperty())) .Concat(entityType.GetSkipNavigations() - .Where(x => !x.IsShadowProperty()) - .Select(x => x.GetMemberInfo(false, false))) + .Where(x => !x.IsShadowProperty())) + .Select(x => x.GetMemberInfo(false, false)) // Remove projectable properties from the ef properties. Since properties returned here for auto // properties (like `public string Test {get;set;}`) are generated fields, we also need to take them into account. .Where(x => projectableProperties.All(y => x.Name != y.Name && x.Name != $"<{y.Name}>k__BackingField")); @@ -364,7 +363,7 @@ private Expression _AddProjectableSelect(Expression node, IEntityType entityType Expression.Lambda( Expression.MemberInit( Expression.New(entityType.ClrType), - properties.Select(x => Expression.Bind(x, Expression.MakeMemberAccess(xParam, x))) + members.Select(x => _MakeBinding(xParam, x)) .Concat(projectableProperties .Select(x => Expression.Bind(x, _GetAccessor(x, xParam))) ) @@ -374,6 +373,59 @@ private Expression _AddProjectableSelect(Expression node, IEntityType entityType ); } + // Builds the member binding used to copy an EF-mapped member into the re-projected entity. + // + // EF's member metadata resolves auto-properties to their compiler-generated backing field. + // Reading a mapped scalar or navigation through that backing field prevents EF's navigation + // expansion from recognizing it as a plain column/navigation, so it instead materializes the + // whole source entity — including any Include-ed collection navigations — once for every + // rewritten member. That produces one duplicate JOIN per member (see issue #217). + // + // Reading through the CLR property lets EF treat scalars as columns and collapse a collection + // navigation into a single join. When the property is also settable we bind to the property + // itself so the generated projection keeps the natural member/column names instead of the + // backing-field name. + private static MemberAssignment _MakeBinding(ParameterExpression para, MemberInfo member) + { + var property = _ResolveClrProperty(member); + if (property is not null && property.CanRead) + { + var read = Expression.MakeMemberAccess(para, property); + return property.CanWrite + ? Expression.Bind(property, read) + : Expression.Bind(member, read); + } + + return Expression.Bind(member, Expression.MakeMemberAccess(para, member)); + } + + // Resolves the CLR property backing an EF member, whether EF handed us the property directly + // or the compiler-generated "k__BackingField" for an auto-property. + private static PropertyInfo? _ResolveClrProperty(MemberInfo member) + { + switch (member) + { + case PropertyInfo property: + return property; + case FieldInfo { Name: ['<', ..] } field when field.DeclaringType is { } declaringType: + var closingBracket = field.Name.IndexOf('>'); + if (closingBracket > 1) + { + var propertyName = field.Name.Substring(1, closingBracket - 1); + var property = declaringType.GetProperty(propertyName, + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + if (property is not null && property.PropertyType == field.FieldType) + { + return property; + } + } + + return null; + default: + return null; + } + } + private Expression _GetAccessor(PropertyInfo property, ParameterExpression para) { var lambda = _resolver.FindGeneratedExpression(property); diff --git a/tests/EntityFrameworkCore.Projectables.FunctionalTests/Issue217Tests.cs b/tests/EntityFrameworkCore.Projectables.FunctionalTests/Issue217Tests.cs new file mode 100644 index 00000000..11c71da4 --- /dev/null +++ b/tests/EntityFrameworkCore.Projectables.FunctionalTests/Issue217Tests.cs @@ -0,0 +1,117 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations.Schema; +using System.Linq; +using Microsoft.EntityFrameworkCore; +using Xunit; + +namespace EntityFrameworkCore.Projectables.FunctionalTests +{ + // https://github.com/EFNext/EntityFrameworkCore.Projectables/issues/217 + // + // A writable [Projectable] property triggers a whole-entity re-projection so the property can be + // populated during materialization. When the entity also has a collection navigation that is + // Include-d, that re-projection previously read every other member through its backing field, + // which forced EF to re-materialize the whole entity (and its Include-d collection) once per + // member -- producing one duplicate JOIN for every rewritten member. + public class Issue217Tests + { + public class Entity1 + { + public virtual int Id { get; set; } + public virtual string? Name1 { get; set; } + + [NotMapped] + [Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore, UseMemberBody = nameof(Name1LenQuery))] + public virtual int? Name1Len { get; set; } + + protected virtual int? Name1LenQuery => Name1 == null ? (int?)null : Name1.Length; + + public virtual List Entity2s { get; set; } = new(); + } + + public class Entity2 + { + public virtual int Id { get; set; } + public virtual string? Name2 { get; set; } + + public virtual int? Entity1Id { get; set; } + public virtual Entity1? Entity1 { get; set; } + } + + public class DemoDbContext : DbContext + { + private readonly string _dataSource; + + public DemoDbContext(string dataSource) => _dataSource = dataSource; + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + { + optionsBuilder.UseSqlite($"Data Source={_dataSource}"); + optionsBuilder.UseProjectables(); + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(); + modelBuilder.Entity(); + } + + public DbSet Entity1s => Set(); + public DbSet Entity2s => Set(); + } + + [Fact] + public void IncludingCollectionDoesNotProduceDuplicateJoins() + { + using var context = new DemoDbContext("issue217-sql.sqlite"); + + var sql = context.Entity1s + .AsNoTrackingWithIdentityResolution() + .Include(e1 => e1.Entity2s) + .ToQueryString(); + + var joinCount = CountOccurrences(sql, "JOIN"); + + Assert.True(joinCount <= 1, $"Expected at most one JOIN but found {joinCount}.{Environment.NewLine}{sql}"); + } + + [Fact] + public void PopulatesWritablePropertyAndLoadsCollection() + { + using var context = new DemoDbContext("issue217-exec.sqlite"); + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + var entity = new Entity1 { Name1 = "hello" }; + entity.Entity2s.Add(new Entity2 { Name2 = "a" }); + entity.Entity2s.Add(new Entity2 { Name2 = "b" }); + context.Add(entity); + context.SaveChanges(); + context.ChangeTracker.Clear(); + + var loaded = context.Entity1s + .AsNoTrackingWithIdentityResolution() + .Include(e1 => e1.Entity2s) + .Single(); + + Assert.Equal(5, loaded.Name1Len); + Assert.Equal(2, loaded.Entity2s.Count); + + context.Database.EnsureDeleted(); + } + + private static int CountOccurrences(string haystack, string needle) + { + var count = 0; + var index = 0; + while ((index = haystack.IndexOf(needle, index, StringComparison.Ordinal)) >= 0) + { + count++; + index += needle.Length; + } + + return count; + } + } +} diff --git a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet10_0.verified.txt b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet10_0.verified.txt index 3b8b9ae3..88b47d7c 100644 --- a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet10_0.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet10_0.verified.txt @@ -1,2 +1,2 @@ -SELECT [e].[Id], [e].[Id] * 5 +SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking] FROM [Entity] AS [e] \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet8_0.verified.txt b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet8_0.verified.txt index 3b8b9ae3..88b47d7c 100644 --- a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet8_0.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet8_0.verified.txt @@ -1,2 +1,2 @@ -SELECT [e].[Id], [e].[Id] * 5 +SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking] FROM [Entity] AS [e] \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet9_0.verified.txt b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet9_0.verified.txt index 3b8b9ae3..88b47d7c 100644 --- a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet9_0.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet9_0.verified.txt @@ -1,2 +1,2 @@ -SELECT [e].[Id], [e].[Id] * 5 +SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking] FROM [Entity] AS [e] \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet10_0.verified.txt b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet10_0.verified.txt index 3b8b9ae3..88b47d7c 100644 --- a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet10_0.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet10_0.verified.txt @@ -1,2 +1,2 @@ -SELECT [e].[Id], [e].[Id] * 5 +SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking] FROM [Entity] AS [e] \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet8_0.verified.txt b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet8_0.verified.txt index 3b8b9ae3..88b47d7c 100644 --- a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet8_0.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet8_0.verified.txt @@ -1,2 +1,2 @@ -SELECT [e].[Id], [e].[Id] * 5 +SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking] FROM [Entity] AS [e] \ No newline at end of file diff --git a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet9_0.verified.txt b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet9_0.verified.txt index 3b8b9ae3..88b47d7c 100644 --- a/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet9_0.verified.txt +++ b/tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet9_0.verified.txt @@ -1,2 +1,2 @@ -SELECT [e].[Id], [e].[Id] * 5 +SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking] FROM [Entity] AS [e] \ No newline at end of file