Skip to content

Commit d358209

Browse files
rojiroji
authored
Fix primitive/complex collection handling on subtypes (#37482)
Fixes #37478
1 parent d15ee81 commit d358209

16 files changed

Lines changed: 224 additions & 119 deletions

‎src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs‎

Lines changed: 57 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -80,68 +80,73 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
8080

8181
var innerExpression = root.UnwrapTypeConversion(out var convertedType);
8282
var structuralTypeReference = UnwrapStructuralTypeReference(innerExpression);
83-
var entityReference = structuralTypeReference as EntityReference;
84-
if (entityReference is not null)
83+
84+
ITypeBase structuralType;
85+
86+
switch (structuralTypeReference)
8587
{
86-
var entityType = entityReference.EntityType;
87-
if (convertedType != null)
88-
{
89-
entityType = entityType.GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive())
90-
.FirstOrDefault(et => et.ClrType == convertedType);
91-
if (entityType == null)
88+
case EntityReference { EntityType: var entityType } entityReference:
89+
if (convertedType != null)
9290
{
93-
return null;
91+
entityType = entityType.GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive())
92+
.FirstOrDefault(et => et.ClrType == convertedType);
93+
if (entityType == null)
94+
{
95+
return null;
96+
}
9497
}
95-
}
9698

97-
var navigation = memberIdentity.MemberInfo is not null
98-
? entityType.FindNavigation(memberIdentity.MemberInfo)
99-
: memberIdentity.Name is not null
100-
? entityType.FindNavigation(memberIdentity.Name)
101-
: null;
102-
if (navigation is not null)
103-
{
104-
return ExpandNavigation(root, entityReference, navigation, convertedType is not null);
105-
}
99+
// Attempt to bind navigations; these are only relevant to entity types
100+
var navigation = memberIdentity.MemberInfo is not null
101+
? entityType.FindNavigation(memberIdentity.MemberInfo)
102+
: memberIdentity.Name is not null
103+
? entityType.FindNavigation(memberIdentity.Name)
104+
: null;
105+
if (navigation is not null)
106+
{
107+
return ExpandNavigation(root, entityReference, navigation, convertedType is not null);
108+
}
106109

107-
var skipNavigation = memberIdentity.MemberInfo is not null
108-
? entityType.FindSkipNavigation(memberIdentity.MemberInfo)
109-
: memberIdentity.Name is not null
110-
? entityType.FindSkipNavigation(memberIdentity.Name)
111-
: null;
112-
if (skipNavigation is not null)
113-
{
114-
return ExpandSkipNavigation(root, entityReference, skipNavigation, convertedType is not null);
115-
}
110+
var skipNavigation = memberIdentity.MemberInfo is not null
111+
? entityType.FindSkipNavigation(memberIdentity.MemberInfo)
112+
: memberIdentity.Name is not null
113+
? entityType.FindSkipNavigation(memberIdentity.Name)
114+
: null;
115+
if (skipNavigation is not null)
116+
{
117+
return ExpandSkipNavigation(root, entityReference, skipNavigation, convertedType is not null);
118+
}
119+
120+
structuralType = entityType;
121+
break;
122+
123+
case ComplexTypeReference { ComplexType: var complexType }:
124+
structuralType = complexType;
125+
break;
126+
127+
default:
128+
return null;
116129
}
117130

118-
var structuralType = entityReference is not null
119-
? (ITypeBase)entityReference.EntityType
120-
: structuralTypeReference is ComplexTypeReference complexTypeReference
121-
? complexTypeReference.ComplexType
131+
// Attempt to bind complex and primitive collection properties; these are common to both entity and complex types
132+
var complexProperty = memberIdentity.MemberInfo != null
133+
? structuralType.FindComplexProperty(memberIdentity.MemberInfo)
134+
: memberIdentity.Name is not null
135+
? structuralType.FindComplexProperty(memberIdentity.Name)
122136
: null;
123-
124-
if (structuralType is not null)
137+
if (complexProperty is not null)
125138
{
126-
var complexProperty = memberIdentity.MemberInfo != null
127-
? structuralType.FindComplexProperty(memberIdentity.MemberInfo)
128-
: memberIdentity.Name is not null
129-
? structuralType.FindComplexProperty(memberIdentity.Name)
130-
: null;
131-
if (complexProperty is not null)
132-
{
133-
return new ComplexPropertyReference(root, complexProperty, originalExpression);
134-
}
139+
return new ComplexPropertyReference(root, complexProperty, originalExpression);
140+
}
135141

136-
var property = memberIdentity.MemberInfo != null
137-
? structuralType.FindProperty(memberIdentity.MemberInfo)
138-
: memberIdentity.Name is not null
139-
? structuralType.FindProperty(memberIdentity.Name)
140-
: null;
141-
if (property?.IsPrimitiveCollection == true)
142-
{
143-
return new PrimitiveCollectionReference(root, property);
144-
}
142+
var property = memberIdentity.MemberInfo != null
143+
? structuralType.FindProperty(memberIdentity.MemberInfo)
144+
: memberIdentity.Name is not null
145+
? structuralType.FindProperty(memberIdentity.Name)
146+
: null;
147+
if (property?.IsPrimitiveCollection == true)
148+
{
149+
return new PrimitiveCollectionReference(root, property);
145150
}
146151

147152
return null;

‎test/EFCore.Cosmos.FunctionalTests/Query/Inheritance/InheritanceQueryCosmosTest.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,20 @@ public override Task Using_is_operator_with_of_type_on_multiple_type_with_no_res
681681
SELECT VALUE c
682682
FROM root c
683683
WHERE ((c["Discriminator"] IN ("Eagle", "Kiwi") AND (c["Discriminator"] = "Kiwi")) AND (c["Discriminator"] = "Eagle"))
684+
""");
685+
});
686+
687+
public override Task Primitive_collection_on_subtype(bool async)
688+
=> Fixture.NoSyncTest(
689+
async, async a =>
690+
{
691+
await base.Primitive_collection_on_subtype(a);
692+
693+
AssertSql(
694+
"""
695+
SELECT VALUE c
696+
FROM root c
697+
WHERE (c["Discriminator"] IN (0, 1, 2, 3) AND (ARRAY_LENGTH(c["Ints"]) > 0))
684698
""");
685699
});
686700

‎test/EFCore.Specification.Tests/Query/Inheritance/InheritanceQueryFixtureBase.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ public InheritanceQueryFixtureBase()
340340
Assert.Equal(ee.SugarGrams, aa.SugarGrams);
341341
Assert.Equal(ee.CaffeineGrams, aa.CaffeineGrams);
342342
Assert.Equal(ee.Carbonation, aa.Carbonation);
343+
Assert.Equal(ee.Ints, aa.Ints);
343344

344345
AssertComplexType(ee.ParentComplexType, aa.ParentComplexType);
345346
AssertComplexType(ee.ChildComplexType, aa.ChildComplexType);

‎test/EFCore.Specification.Tests/Query/Inheritance/InheritanceQueryTestBase.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,13 @@ public virtual Task GetType_in_hierarchy_in_leaf_type_with_sibling2_not_equal(bo
469469
async,
470470
ss => ss.Set<Animal>().Where(e => typeof(Kiwi) != e.GetType()));
471471

472+
[ConditionalTheory, MemberData(nameof(IsAsyncData))]
473+
public virtual Task Primitive_collection_on_subtype(bool async)
474+
=> AssertQuery(
475+
async,
476+
ss => ss.Set<Drink>().Where(d => ((Coke)d).Ints.Any()),
477+
ss => ss.Set<Drink>().Where(d => d is Coke && ((Coke)d).Ints.Any()));
478+
472479
protected InheritanceContext CreateContext()
473480
=> Fixture.CreateContext();
474481

‎test/EFCore.Specification.Tests/TestModels/InheritanceModel/Coke.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ public class Coke : Drink, ISugary
99
public int CaffeineGrams { get; set; }
1010
public int Carbonation { get; set; }
1111

12-
// Samep roperty name as on Tea, to test uniquification
12+
public required int[] Ints { get; set; }
13+
14+
// Same property name as on Tea, to test uniquification
1315
public ComplexType? ChildComplexType { get; set; }
1416
}

‎test/EFCore.Specification.Tests/TestModels/InheritanceModel/InheritanceData.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ public static IReadOnlyList<Drink> CreateDrinks(bool useGeneratedKeys)
218218
SugarGrams = 6,
219219
CaffeineGrams = 4,
220220
Carbonation = 5,
221+
Ints = [8, 9],
221222
ChildComplexType = new ComplexType
222223
{
223224
UniqueInt = 100,

‎test/EFCore.SqlServer.FunctionalTests/Query/Inheritance/IncompleteMappingInheritanceQuerySqlServerTest.cs‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public override async Task Can_query_when_shared_column(bool async)
4242

4343
AssertSql(
4444
"""
45-
SELECT TOP(2) [d].[Id], [d].[Discriminator], [d].[SortIndex], [d].[CaffeineGrams], [d].[CokeCO2], [d].[SugarGrams], [d].[ComplexTypeCollection], [d].[ParentComplexType_Int], [d].[ParentComplexType_UniqueInt], [d].[ParentComplexType_Nested_NestedInt], [d].[ParentComplexType_Nested_UniqueInt], [d].[ChildComplexType_Int], [d].[ChildComplexType_UniqueInt], [d].[ChildComplexType_Nested_NestedInt], [d].[ChildComplexType_Nested_UniqueInt]
45+
SELECT TOP(2) [d].[Id], [d].[Discriminator], [d].[SortIndex], [d].[CaffeineGrams], [d].[CokeCO2], [d].[Ints], [d].[SugarGrams], [d].[ComplexTypeCollection], [d].[ParentComplexType_Int], [d].[ParentComplexType_UniqueInt], [d].[ParentComplexType_Nested_NestedInt], [d].[ParentComplexType_Nested_UniqueInt], [d].[ChildComplexType_Int], [d].[ChildComplexType_UniqueInt], [d].[ChildComplexType_Nested_NestedInt], [d].[ChildComplexType_Nested_UniqueInt]
4646
FROM [Drinks] AS [d]
4747
WHERE [d].[Discriminator] = 1
4848
""",
@@ -94,7 +94,7 @@ public override async Task Can_query_all_types_when_shared_column(bool async)
9494

9595
AssertSql(
9696
"""
97-
SELECT [d].[Id], [d].[Discriminator], [d].[SortIndex], [d].[CaffeineGrams], [d].[CokeCO2], [d].[SugarGrams], [d].[LiltCO2], [d].[HasMilk], [d].[ComplexTypeCollection], [d].[ParentComplexType_Int], [d].[ParentComplexType_UniqueInt], [d].[ParentComplexType_Nested_NestedInt], [d].[ParentComplexType_Nested_UniqueInt], [d].[ChildComplexType_Int], [d].[ChildComplexType_UniqueInt], [d].[ChildComplexType_Nested_NestedInt], [d].[ChildComplexType_Nested_UniqueInt], [d].[Tea_ChildComplexType_Int], [d].[Tea_ChildComplexType_UniqueInt], [d].[Tea_ChildComplexType_Nested_NestedInt], [d].[Tea_ChildComplexType_Nested_UniqueInt]
97+
SELECT [d].[Id], [d].[Discriminator], [d].[SortIndex], [d].[CaffeineGrams], [d].[CokeCO2], [d].[Ints], [d].[SugarGrams], [d].[LiltCO2], [d].[HasMilk], [d].[ComplexTypeCollection], [d].[ParentComplexType_Int], [d].[ParentComplexType_UniqueInt], [d].[ParentComplexType_Nested_NestedInt], [d].[ParentComplexType_Nested_UniqueInt], [d].[ChildComplexType_Int], [d].[ChildComplexType_UniqueInt], [d].[ChildComplexType_Nested_NestedInt], [d].[ChildComplexType_Nested_UniqueInt], [d].[Tea_ChildComplexType_Int], [d].[Tea_ChildComplexType_UniqueInt], [d].[Tea_ChildComplexType_Nested_NestedInt], [d].[Tea_ChildComplexType_Nested_UniqueInt]
9898
FROM [Drinks] AS [d]
9999
WHERE [d].[Discriminator] IN (0, 1, 2, 3)
100100
""");
@@ -769,6 +769,20 @@ WHERE [a].[Discriminator] IN (N'Eagle', N'Kiwi') AND [a].[Discriminator] <> N'Ki
769769
""");
770770
}
771771

772+
public override async Task Primitive_collection_on_subtype(bool async)
773+
{
774+
await base.Primitive_collection_on_subtype(async);
775+
776+
AssertSql(
777+
"""
778+
SELECT [d].[Id], [d].[Discriminator], [d].[SortIndex], [d].[CaffeineGrams], [d].[CokeCO2], [d].[Ints], [d].[SugarGrams], [d].[LiltCO2], [d].[HasMilk], [d].[ComplexTypeCollection], [d].[ParentComplexType_Int], [d].[ParentComplexType_UniqueInt], [d].[ParentComplexType_Nested_NestedInt], [d].[ParentComplexType_Nested_UniqueInt], [d].[ChildComplexType_Int], [d].[ChildComplexType_UniqueInt], [d].[ChildComplexType_Nested_NestedInt], [d].[ChildComplexType_Nested_UniqueInt], [d].[Tea_ChildComplexType_Int], [d].[Tea_ChildComplexType_UniqueInt], [d].[Tea_ChildComplexType_Nested_NestedInt], [d].[Tea_ChildComplexType_Nested_UniqueInt]
779+
FROM [Drinks] AS [d]
780+
WHERE [d].[Discriminator] IN (0, 1, 2, 3) AND EXISTS (
781+
SELECT 1
782+
FROM OPENJSON([d].[Ints]) AS [i])
783+
""");
784+
}
785+
772786
private void AssertSql(params string[] expected)
773787
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
774788
}

0 commit comments

Comments
 (0)