Skip to content

Commit cd69313

Browse files
committed
InMemory: Improve indexing in server expression
Implement left join as client method to reduce complexity To resolve the indexing issue stemming from #23934 Now all the projections are applied immediately to reshape the value buffer so that all our future bindings are always read value expressions which refers to a proper index. In order to do this, we apply select for entity type with hierarchy to avoid entity check conditional expression. For adding projection (through ReplaceProjectionMapping), - For client projection, we apply a select and re-generate client projection as read values - For projection mapping, we iterate the mappings, we apply a select and re-generate the mapping as read values - If projection mapping is empty then we add a dummy 1 so that it becomes non-range-variable When applying projection, we generate a selector lambda to form a value buffer and replace all the expressions to read from new value buffer. Overall this solves the issue of having complex expressions to map or pull. This also removed PushDownIntoSubquery method. In order to avoid the issue of indexes changing when generating join due to iterating projection mappings, we now also have projectionMappingExpressions which remembers the all expressions inside projectionMapping (which are all read value as we generated before). So now we don't need to iterate the mapping and we use the existing expressions directly. This keeps existing indexes. Resolves #13561 Resolves #17539 Resolves #18194 Resolves #18435 Resolves #19344 Resolves #19469 Resolves #19667 Resolves #19742 Resolves #19967 Resolves #20359 Resolves #21677 Resolves #23360 Resolves #17537 Resolves #18394 Resolves #23934
1 parent 2eabf1f commit cd69313

16 files changed

+1225
-915
lines changed

Diff for: src/EFCore.InMemory/Query/Internal/EntityProjectionExpression.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace Microsoft.EntityFrameworkCore.InMemory.Query.Internal
2323
/// </summary>
2424
public class EntityProjectionExpression : Expression, IPrintableExpression
2525
{
26-
private readonly IDictionary<IProperty, Expression> _readExpressionMap;
26+
private readonly IDictionary<IProperty, MethodCallExpression> _readExpressionMap;
2727

2828
private readonly IDictionary<INavigation, EntityShaperExpression> _navigationExpressionsCache
2929
= new Dictionary<INavigation, EntityShaperExpression>();
@@ -36,7 +36,7 @@ private readonly IDictionary<INavigation, EntityShaperExpression> _navigationExp
3636
/// </summary>
3737
public EntityProjectionExpression(
3838
[NotNull] IEntityType entityType,
39-
[NotNull] IDictionary<IProperty, Expression> readExpressionMap)
39+
[NotNull] IDictionary<IProperty, MethodCallExpression> readExpressionMap)
4040
{
4141
EntityType = entityType;
4242
_readExpressionMap = readExpressionMap;
@@ -83,7 +83,7 @@ public virtual EntityProjectionExpression UpdateEntityType([NotNull] IEntityType
8383
derivedType.DisplayName(), EntityType.DisplayName()));
8484
}
8585

86-
var readExpressionMap = new Dictionary<IProperty, Expression>();
86+
var readExpressionMap = new Dictionary<IProperty, MethodCallExpression>();
8787
foreach (var kvp in _readExpressionMap)
8888
{
8989
var property = kvp.Key;
@@ -103,7 +103,7 @@ public virtual EntityProjectionExpression UpdateEntityType([NotNull] IEntityType
103103
/// any release. You should only use it directly in your code with extreme caution and knowing that
104104
/// doing so can result in application failures when updating to a new Entity Framework Core release.
105105
/// </summary>
106-
public virtual Expression BindProperty([NotNull] IProperty property)
106+
public virtual MethodCallExpression BindProperty([NotNull] IProperty property)
107107
{
108108
if (!EntityType.IsAssignableFrom(property.DeclaringEntityType)
109109
&& !property.DeclaringEntityType.IsAssignableFrom(EntityType))

Diff for: src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs

+4-7
Original file line numberDiff line numberDiff line change
@@ -1188,15 +1188,12 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression)
11881188

11891189
// if the result type change was just nullability change e.g from int to int?
11901190
// we want to preserve the new type for null propagation
1191-
if (result.Type != type
1191+
return result.Type != type
11921192
&& !(result.Type.IsNullableType()
11931193
&& !type.IsNullableType()
1194-
&& result.Type.UnwrapNullableType() == type))
1195-
{
1196-
result = Expression.Convert(result, type);
1197-
}
1198-
1199-
return result;
1194+
&& result.Type.UnwrapNullableType() == type)
1195+
? Expression.Convert(result, type)
1196+
: (Expression)result;
12001197
}
12011198

12021199
if (entityReferenceExpression.SubqueryEntity != null)

Diff for: src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs

+531-740
Large diffs are not rendered by default.

Diff for: src/EFCore.InMemory/Query/Internal/InMemoryQueryableMethodTranslatingExpressionVisitor.cs

+6-16
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@ private static ShapedQueryExpression CreateShapedQueryExpressionStatic(IEntityTy
156156
if (source.ShaperExpression is GroupByShaperExpression)
157157
{
158158
inMemoryQueryExpression.ReplaceProjectionMapping(new Dictionary<ProjectionMember, Expression>());
159-
inMemoryQueryExpression.PushdownIntoSubquery();
160159
}
161160

162161
inMemoryQueryExpression.UpdateServerQueryExpression(
@@ -191,7 +190,6 @@ private static ShapedQueryExpression CreateShapedQueryExpressionStatic(IEntityTy
191190
if (source.ShaperExpression is GroupByShaperExpression)
192191
{
193192
inMemoryQueryExpression.ReplaceProjectionMapping(new Dictionary<ProjectionMember, Expression>());
194-
inMemoryQueryExpression.PushdownIntoSubquery();
195193
}
196194

197195
inMemoryQueryExpression.UpdateServerQueryExpression(
@@ -304,7 +302,6 @@ private static ShapedQueryExpression CreateShapedQueryExpressionStatic(IEntityTy
304302
if (source.ShaperExpression is GroupByShaperExpression)
305303
{
306304
inMemoryQueryExpression.ReplaceProjectionMapping(new Dictionary<ProjectionMember, Expression>());
307-
inMemoryQueryExpression.PushdownIntoSubquery();
308305
}
309306

310307
inMemoryQueryExpression.UpdateServerQueryExpression(
@@ -346,7 +343,6 @@ private static ShapedQueryExpression CreateShapedQueryExpressionStatic(IEntityTy
346343

347344
var inMemoryQueryExpression = (InMemoryQueryExpression)source.QueryExpression;
348345

349-
inMemoryQueryExpression.PushdownIntoSubquery();
350346
inMemoryQueryExpression.UpdateServerQueryExpression(
351347
Expression.Call(
352348
EnumerableMethods.Distinct.MakeGenericMethod(inMemoryQueryExpression.CurrentParameter.Type),
@@ -430,13 +426,14 @@ private static ShapedQueryExpression CreateShapedQueryExpressionStatic(IEntityTy
430426
var translatedKey = TranslateGroupingKey(remappedKeySelector);
431427
if (translatedKey != null)
432428
{
433-
if (elementSelector != null)
429+
var inMemoryQueryExpression = (InMemoryQueryExpression)source.QueryExpression;
430+
var defaultElementSelector = elementSelector == null || elementSelector.Body == elementSelector.Parameters[0];
431+
if (!defaultElementSelector)
434432
{
435-
source = TranslateSelect(source, elementSelector);
433+
source = TranslateSelect(source, elementSelector!);
436434
}
437435

438-
var inMemoryQueryExpression = (InMemoryQueryExpression)source.QueryExpression;
439-
var groupByShaper = inMemoryQueryExpression.ApplyGrouping(translatedKey, source.ShaperExpression);
436+
var groupByShaper = inMemoryQueryExpression.ApplyGrouping(translatedKey, source.ShaperExpression, defaultElementSelector);
440437

441438
if (resultSelector == null)
442439
{
@@ -452,7 +449,6 @@ private static ShapedQueryExpression CreateShapedQueryExpressionStatic(IEntityTy
452449

453450
newResultSelectorBody = ExpandWeakEntities(inMemoryQueryExpression, newResultSelectorBody);
454451
var newShaper = _projectionBindingExpressionVisitor.Translate(inMemoryQueryExpression, newResultSelectorBody);
455-
inMemoryQueryExpression.PushdownIntoSubquery();
456452

457453
return source.UpdateShaperExpression(newShaper);
458454
}
@@ -806,7 +802,6 @@ static bool IsConvertedToNullable(Expression outer, Expression inner)
806802
if (source.ShaperExpression is GroupByShaperExpression)
807803
{
808804
inMemoryQueryExpression.ReplaceProjectionMapping(new Dictionary<ProjectionMember, Expression>());
809-
inMemoryQueryExpression.PushdownIntoSubquery();
810805
}
811806

812807
inMemoryQueryExpression.UpdateServerQueryExpression(
@@ -974,14 +969,9 @@ protected override ShapedQueryExpression TranslateSelect(ShapedQueryExpression s
974969
var newSelectorBody = ReplacingExpressionVisitor.Replace(
975970
selector.Parameters.Single(), source.ShaperExpression, selector.Body);
976971

977-
var groupByQuery = source.ShaperExpression is GroupByShaperExpression;
978972
var queryExpression = (InMemoryQueryExpression)source.QueryExpression;
979973

980974
var newShaper = _projectionBindingExpressionVisitor.Translate(queryExpression, newSelectorBody);
981-
if (groupByQuery)
982-
{
983-
queryExpression.PushdownIntoSubquery();
984-
}
985975

986976
return source.UpdateShaperExpression(newShaper);
987977
}
@@ -1384,7 +1374,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
13841374
private Expression? TryExpand(Expression? source, MemberIdentity member)
13851375
{
13861376
source = source.UnwrapTypeConversion(out var convertedType);
1387-
if (!(source is EntityShaperExpression entityShaperExpression))
1377+
if (source is not EntityShaperExpression entityShaperExpression)
13881378
{
13891379
return null;
13901380
}

Diff for: test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs

-30
Original file line numberDiff line numberDiff line change
@@ -14,35 +14,5 @@ public ComplexNavigationsQueryInMemoryTest(ComplexNavigationsQueryInMemoryFixtur
1414
{
1515
//TestLoggerFactory.TestOutputHelper = testOutputHelper;
1616
}
17-
18-
[ConditionalFact(Skip = "issue #18194")]
19-
public override void Member_pushdown_chain_3_levels_deep_entity()
20-
{
21-
base.Member_pushdown_chain_3_levels_deep_entity();
22-
}
23-
24-
[ConditionalTheory(Skip = "issue #17620")]
25-
public override Task Lift_projection_mapping_when_pushing_down_subquery(bool async)
26-
{
27-
return base.Lift_projection_mapping_when_pushing_down_subquery(async);
28-
}
29-
30-
[ConditionalTheory(Skip = "issue #19344")]
31-
public override Task Select_subquery_single_nested_subquery(bool async)
32-
{
33-
return base.Select_subquery_single_nested_subquery(async);
34-
}
35-
36-
[ConditionalTheory(Skip = "issue #19344")]
37-
public override Task Select_subquery_single_nested_subquery2(bool async)
38-
{
39-
return base.Select_subquery_single_nested_subquery2(async);
40-
}
41-
42-
[ConditionalTheory(Skip = "issue #17539")]
43-
public override Task Union_over_entities_with_different_nullability(bool async)
44-
{
45-
return base.Union_over_entities_with_different_nullability(async);
46-
}
4717
}
4818
}

Diff for: test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsSharedTypeQueryInMemoryTest.cs

-78
Original file line numberDiff line numberDiff line change
@@ -18,83 +18,5 @@ public ComplexNavigationsSharedTypeQueryInMemoryTest(
1818
{
1919
//TestLoggerFactory.TestOutputHelper = testOutputHelper;
2020
}
21-
22-
[ConditionalTheory(Skip = "Issue#17539")]
23-
public override Task Join_navigations_in_inner_selector_translated_without_collision(bool async)
24-
{
25-
return base.Join_navigations_in_inner_selector_translated_without_collision(async);
26-
}
27-
28-
[ConditionalTheory(Skip = "Issue#17539")]
29-
public override Task Join_with_navigations_in_the_result_selector1(bool async)
30-
{
31-
return base.Join_with_navigations_in_the_result_selector1(async);
32-
}
33-
34-
[ConditionalTheory(Skip = "Issue#17539")]
35-
public override Task Where_nav_prop_reference_optional1_via_DefaultIfEmpty(bool async)
36-
{
37-
return base.Where_nav_prop_reference_optional1_via_DefaultIfEmpty(async);
38-
}
39-
40-
[ConditionalTheory(Skip = "Issue#17539")]
41-
public override Task Where_nav_prop_reference_optional2_via_DefaultIfEmpty(bool async)
42-
{
43-
return base.Where_nav_prop_reference_optional2_via_DefaultIfEmpty(async);
44-
}
45-
46-
[ConditionalTheory(Skip = "Issue#17539")]
47-
public override Task Optional_navigation_propagates_nullability_to_manually_created_left_join2(bool async)
48-
{
49-
return base.Optional_navigation_propagates_nullability_to_manually_created_left_join2(async);
50-
}
51-
52-
[ConditionalTheory(Skip = "issue #17620")]
53-
public override Task Lift_projection_mapping_when_pushing_down_subquery(bool async)
54-
{
55-
return base.Lift_projection_mapping_when_pushing_down_subquery(async);
56-
}
57-
58-
[ConditionalTheory(Skip = "issue #18912")]
59-
public override Task OrderBy_collection_count_ThenBy_reference_navigation(bool async)
60-
{
61-
return base.OrderBy_collection_count_ThenBy_reference_navigation(async);
62-
}
63-
64-
[ConditionalTheory(Skip = "issue #19344")]
65-
public override Task Select_subquery_single_nested_subquery(bool async)
66-
{
67-
return base.Select_subquery_single_nested_subquery(async);
68-
}
69-
70-
[ConditionalTheory(Skip = "issue #19344")]
71-
public override Task Select_subquery_single_nested_subquery2(bool async)
72-
{
73-
return base.Select_subquery_single_nested_subquery2(async);
74-
}
75-
76-
[ConditionalTheory(Skip = "issue #19967")]
77-
public override Task SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(bool async)
78-
{
79-
return base.SelectMany_with_outside_reference_to_joined_table_correctly_translated_to_apply(async);
80-
}
81-
82-
[ConditionalTheory(Skip = "issue #19967")]
83-
public override Task Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(bool async)
84-
{
85-
return base.Nested_SelectMany_correlated_with_join_table_correctly_translated_to_apply(async);
86-
}
87-
88-
[ConditionalTheory(Skip = "issue #19742")]
89-
public override Task Contains_over_optional_navigation_with_null_column(bool async)
90-
{
91-
return base.Contains_over_optional_navigation_with_null_column(async);
92-
}
93-
94-
[ConditionalTheory(Skip = "issue #19742")]
95-
public override Task Contains_over_optional_navigation_with_null_entity_reference(bool async)
96-
{
97-
return base.Contains_over_optional_navigation_with_null_entity_reference(async);
98-
}
9921
}
10022
}

Diff for: test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs

-16
Original file line numberDiff line numberDiff line change
@@ -23,32 +23,16 @@ public override Task Client_member_and_unsupported_string_Equals_in_the_same_que
2323
CoreStrings.QueryUnableToTranslateMember(nameof(Gear.IsMarcus), nameof(Gear)));
2424
}
2525

26-
[ConditionalFact(Skip = "issue #17537")]
27-
public override void Include_on_GroupJoin_SelectMany_DefaultIfEmpty_with_coalesce_result1()
28-
=> base.Include_on_GroupJoin_SelectMany_DefaultIfEmpty_with_coalesce_result1();
29-
30-
[ConditionalFact(Skip = "issue #17537")]
31-
public override void Include_on_GroupJoin_SelectMany_DefaultIfEmpty_with_coalesce_result2()
32-
=> base.Include_on_GroupJoin_SelectMany_DefaultIfEmpty_with_coalesce_result2();
33-
3426
[ConditionalTheory(Skip = "issue #17540")]
3527
public override Task
3628
Null_semantics_is_correctly_applied_for_function_comparisons_that_take_arguments_from_optional_navigation_complex(bool async)
3729
=> base.Null_semantics_is_correctly_applied_for_function_comparisons_that_take_arguments_from_optional_navigation_complex(
3830
async);
3931

40-
[ConditionalTheory(Skip = "issue #17620")]
41-
public override Task Select_subquery_projecting_single_constant_inside_anonymous(bool async)
42-
=> base.Select_subquery_projecting_single_constant_inside_anonymous(async);
43-
4432
[ConditionalTheory(Skip = "issue #19683")]
4533
public override Task Group_by_on_StartsWith_with_null_parameter_as_argument(bool async)
4634
=> base.Group_by_on_StartsWith_with_null_parameter_as_argument(async);
4735

48-
[ConditionalTheory(Skip = "issue #17537")]
49-
public override Task SelectMany_predicate_with_non_equality_comparison_with_Take_doesnt_convert_to_join(bool async)
50-
=> base.SelectMany_predicate_with_non_equality_comparison_with_Take_doesnt_convert_to_join(async);
51-
5236
[ConditionalTheory(Skip = "issue #19584")]
5337
public override Task Cast_to_derived_followed_by_include_and_FirstOrDefault(bool async)
5438
=> base.Cast_to_derived_followed_by_include_and_FirstOrDefault(async);

Diff for: test/EFCore.InMemory.FunctionalTests/Query/NorthwindGroupByQueryInMemoryTest.cs

-6
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,5 @@ public NorthwindGroupByQueryInMemoryTest(
1818
{
1919
//TestLoggerFactory.TestOutputHelper = testOutputHelper;
2020
}
21-
22-
[ConditionalTheory(Skip = "Issue#17536")]
23-
public override Task Join_GroupBy_Aggregate_with_left_join(bool async)
24-
{
25-
return base.Join_GroupBy_Aggregate_with_left_join(async);
26-
}
2721
}
2822
}

Diff for: test/EFCore.InMemory.FunctionalTests/Query/OwnedQueryInMemoryTest.cs

-6
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,6 @@ public OwnedQueryInMemoryTest(OwnedQueryInMemoryFixture fixture, ITestOutputHelp
1616
//TestLoggerFactory.TestOutputHelper = testOutputHelper;
1717
}
1818

19-
[ConditionalTheory(Skip = "issue #19742")]
20-
public override Task Projecting_collection_correlated_with_keyless_entity_after_navigation_works_using_parent_identifiers(bool async)
21-
{
22-
return base.Projecting_collection_correlated_with_keyless_entity_after_navigation_works_using_parent_identifiers(async);
23-
}
24-
2519
public class OwnedQueryInMemoryFixture : OwnedQueryFixtureBase
2620
{
2721
protected override ITestStoreFactory TestStoreFactory

0 commit comments

Comments
 (0)