Skip to content

Commit 67209c4

Browse files
claudeclaude
committed
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Xvb9UMRqFJKpocKCCeYcc
1 parent a23efdf commit 67209c4

8 files changed

Lines changed: 182 additions & 13 deletions

‎src/EntityFrameworkCore.Projectables/Services/ProjectableExpressionReplacer.cs‎

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -341,15 +341,14 @@ private Expression _AddProjectableSelect(Expression node, IEntityType entityType
341341
return node;
342342
}
343343

344-
var properties = entityType.GetProperties()
344+
var members = entityType.GetProperties()
345345
.Where(x => !x.IsShadowProperty())
346-
.Select(x => x.GetMemberInfo(false, false))
346+
.Cast<IPropertyBase>()
347347
.Concat(entityType.GetNavigations()
348-
.Where(x => !x.IsShadowProperty())
349-
.Select(x => x.GetMemberInfo(false, false)))
348+
.Where(x => !x.IsShadowProperty()))
350349
.Concat(entityType.GetSkipNavigations()
351-
.Where(x => !x.IsShadowProperty())
352-
.Select(x => x.GetMemberInfo(false, false)))
350+
.Where(x => !x.IsShadowProperty()))
351+
.Select(x => x.GetMemberInfo(false, false))
353352
// Remove projectable properties from the ef properties. Since properties returned here for auto
354353
// properties (like `public string Test {get;set;}`) are generated fields, we also need to take them into account.
355354
.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
364363
Expression.Lambda(
365364
Expression.MemberInit(
366365
Expression.New(entityType.ClrType),
367-
properties.Select(x => Expression.Bind(x, Expression.MakeMemberAccess(xParam, x)))
366+
members.Select(x => _MakeBinding(xParam, x))
368367
.Concat(projectableProperties
369368
.Select(x => Expression.Bind(x, _GetAccessor(x, xParam)))
370369
)
@@ -374,6 +373,59 @@ private Expression _AddProjectableSelect(Expression node, IEntityType entityType
374373
);
375374
}
376375

376+
// Builds the member binding used to copy an EF-mapped member into the re-projected entity.
377+
//
378+
// EF's member metadata resolves auto-properties to their compiler-generated backing field.
379+
// Reading a mapped scalar or navigation through that backing field prevents EF's navigation
380+
// expansion from recognizing it as a plain column/navigation, so it instead materializes the
381+
// whole source entity — including any Include-ed collection navigations — once for every
382+
// rewritten member. That produces one duplicate JOIN per member (see issue #217).
383+
//
384+
// Reading through the CLR property lets EF treat scalars as columns and collapse a collection
385+
// navigation into a single join. When the property is also settable we bind to the property
386+
// itself so the generated projection keeps the natural member/column names instead of the
387+
// backing-field name.
388+
private static MemberAssignment _MakeBinding(ParameterExpression para, MemberInfo member)
389+
{
390+
var property = _ResolveClrProperty(member);
391+
if (property is not null && property.CanRead)
392+
{
393+
var read = Expression.MakeMemberAccess(para, property);
394+
return property.CanWrite
395+
? Expression.Bind(property, read)
396+
: Expression.Bind(member, read);
397+
}
398+
399+
return Expression.Bind(member, Expression.MakeMemberAccess(para, member));
400+
}
401+
402+
// Resolves the CLR property backing an EF member, whether EF handed us the property directly
403+
// or the compiler-generated "<Name>k__BackingField" for an auto-property.
404+
private static PropertyInfo? _ResolveClrProperty(MemberInfo member)
405+
{
406+
switch (member)
407+
{
408+
case PropertyInfo property:
409+
return property;
410+
case FieldInfo { Name: ['<', ..] } field when field.DeclaringType is { } declaringType:
411+
var closingBracket = field.Name.IndexOf('>');
412+
if (closingBracket > 1)
413+
{
414+
var propertyName = field.Name.Substring(1, closingBracket - 1);
415+
var property = declaringType.GetProperty(propertyName,
416+
BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
417+
if (property is not null && property.PropertyType == field.FieldType)
418+
{
419+
return property;
420+
}
421+
}
422+
423+
return null;
424+
default:
425+
return null;
426+
}
427+
}
428+
377429
private Expression _GetAccessor(PropertyInfo property, ParameterExpression para)
378430
{
379431
var lambda = _resolver.FindGeneratedExpression(property);

‎tests/EntityFrameworkCore.Projectables.FunctionalTests/Issue217Tests.cs‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.ComponentModel.DataAnnotations.Schema;
4+
using System.Linq;
5+
using Microsoft.EntityFrameworkCore;
6+
using Xunit;
7+
8+
namespace EntityFrameworkCore.Projectables.FunctionalTests
9+
{
10+
// https://github.com/EFNext/EntityFrameworkCore.Projectables/issues/217
11+
//
12+
// A writable [Projectable] property triggers a whole-entity re-projection so the property can be
13+
// populated during materialization. When the entity also has a collection navigation that is
14+
// Include-d, that re-projection previously read every other member through its backing field,
15+
// which forced EF to re-materialize the whole entity (and its Include-d collection) once per
16+
// member -- producing one duplicate JOIN for every rewritten member.
17+
public class Issue217Tests
18+
{
19+
public class Entity1
20+
{
21+
public virtual int Id { get; set; }
22+
public virtual string? Name1 { get; set; }
23+
24+
[NotMapped]
25+
[Projectable(NullConditionalRewriteSupport = NullConditionalRewriteSupport.Ignore, UseMemberBody = nameof(Name1LenQuery))]
26+
public virtual int? Name1Len { get; set; }
27+
28+
protected virtual int? Name1LenQuery => Name1 == null ? (int?)null : Name1.Length;
29+
30+
public virtual List<Entity2> Entity2s { get; set; } = new();
31+
}
32+
33+
public class Entity2
34+
{
35+
public virtual int Id { get; set; }
36+
public virtual string? Name2 { get; set; }
37+
38+
public virtual int? Entity1Id { get; set; }
39+
public virtual Entity1? Entity1 { get; set; }
40+
}
41+
42+
public class DemoDbContext : DbContext
43+
{
44+
private readonly string _dataSource;
45+
46+
public DemoDbContext(string dataSource) => _dataSource = dataSource;
47+
48+
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
49+
{
50+
optionsBuilder.UseSqlite($"Data Source={_dataSource}");
51+
optionsBuilder.UseProjectables();
52+
}
53+
54+
protected override void OnModelCreating(ModelBuilder modelBuilder)
55+
{
56+
modelBuilder.Entity<Entity1>();
57+
modelBuilder.Entity<Entity2>();
58+
}
59+
60+
public DbSet<Entity1> Entity1s => Set<Entity1>();
61+
public DbSet<Entity2> Entity2s => Set<Entity2>();
62+
}
63+
64+
[Fact]
65+
public void IncludingCollectionDoesNotProduceDuplicateJoins()
66+
{
67+
using var context = new DemoDbContext("issue217-sql.sqlite");
68+
69+
var sql = context.Entity1s
70+
.AsNoTrackingWithIdentityResolution()
71+
.Include(e1 => e1.Entity2s)
72+
.ToQueryString();
73+
74+
var joinCount = CountOccurrences(sql, "JOIN");
75+
76+
Assert.True(joinCount <= 1, $"Expected at most one JOIN but found {joinCount}.{Environment.NewLine}{sql}");
77+
}
78+
79+
[Fact]
80+
public void PopulatesWritablePropertyAndLoadsCollection()
81+
{
82+
using var context = new DemoDbContext("issue217-exec.sqlite");
83+
context.Database.EnsureDeleted();
84+
context.Database.EnsureCreated();
85+
86+
var entity = new Entity1 { Name1 = "hello" };
87+
entity.Entity2s.Add(new Entity2 { Name2 = "a" });
88+
entity.Entity2s.Add(new Entity2 { Name2 = "b" });
89+
context.Add(entity);
90+
context.SaveChanges();
91+
context.ChangeTracker.Clear();
92+
93+
var loaded = context.Entity1s
94+
.AsNoTrackingWithIdentityResolution()
95+
.Include(e1 => e1.Entity2s)
96+
.Single();
97+
98+
Assert.Equal(5, loaded.Name1Len);
99+
Assert.Equal(2, loaded.Entity2s.Count);
100+
101+
context.Database.EnsureDeleted();
102+
}
103+
104+
private static int CountOccurrences(string haystack, string needle)
105+
{
106+
var count = 0;
107+
var index = 0;
108+
while ((index = haystack.IndexOf(needle, index, StringComparison.Ordinal)) >= 0)
109+
{
110+
count++;
111+
index += needle.Length;
112+
}
113+
114+
return count;
115+
}
116+
}
117+
}

‎tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet10_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
SELECT [e].[Id], [e].[Id] * 5
1+
SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking]
22
FROM [Entity] AS [e]

‎tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet8_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
SELECT [e].[Id], [e].[Id] * 5
1+
SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking]
22
FROM [Entity] AS [e]

‎tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.AsNoTrackingQueryRootExpression.DotNet9_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
SELECT [e].[Id], [e].[Id] * 5
1+
SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking]
22
FROM [Entity] AS [e]

‎tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet10_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
SELECT [e].[Id], [e].[Id] * 5
1+
SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking]
22
FROM [Entity] AS [e]

‎tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet8_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
SELECT [e].[Id], [e].[Id] * 5
1+
SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking]
22
FROM [Entity] AS [e]

‎tests/EntityFrameworkCore.Projectables.FunctionalTests/QueryRootTests.UseMemberPropertyQueryRootExpression.DotNet9_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
SELECT [e].[Id], [e].[Id] * 5
1+
SELECT [e].[Id], [e].[Id] * 5 AS [ComputedWithBacking]
22
FROM [Entity] AS [e]

0 commit comments

Comments
 (0)