Merge pull request #218 from EFNext/claude/github-issue-217-9odp2y

Fix duplicate joins when materializing writable projectables with includes

Author: PhenX

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<IPropertyBase>()
                 .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 "<Name>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);

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<Entity2> 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<Entity1>();
+                modelBuilder.Entity<Entity2>();
+            }
+
+            public DbSet<Entity1> Entity1s => Set<Entity1>();
+            public DbSet<Entity2> Entity2s => Set<Entity2>();
+        }
+
+        [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;
+        }
+    }
+}

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]

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]

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]

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]

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]

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]