diff --git a/src/EntityFrameworkCore.Projectables/Services/ProjectableExpressionReplacer.cs b/src/EntityFrameworkCore.Projectables/Services/ProjectableExpressionReplacer.cs index 72bc0f8..be3c96a 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 0000000..11c71da --- /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 3b8b9ae..88b47d7 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 3b8b9ae..88b47d7 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 3b8b9ae..88b47d7 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 3b8b9ae..88b47d7 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 3b8b9ae..88b47d7 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 3b8b9ae..88b47d7 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