Skip to content

Commit 6a67326

Browse files
ajcvickersajcvickers
authored
Set the key comparer as well as the normal comparer when mapping a primitive collection (#31661)
1 parent 806c3a2 commit 6a67326

File tree

10 files changed

+55
-35
lines changed

10 files changed

+55
-35
lines changed

‎src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,10 @@ protected CosmosTypeMapping(CoreTypeMappingParameters parameters)
6363
public override CoreTypeMapping Clone(
6464
ValueConverter? converter,
6565
ValueComparer? comparer = null,
66+
ValueComparer? keyComparer = null,
6667
CoreTypeMapping? elementMapping = null,
6768
JsonValueReaderWriter? jsonValueReaderWriter = null)
68-
=> new CosmosTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));
69+
=> new CosmosTypeMapping(Parameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter));
6970

7071
/// <summary>
7172
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to

‎src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,10 @@ private InMemoryTypeMapping(CoreTypeMappingParameters parameters)
5656
public override CoreTypeMapping Clone(
5757
ValueConverter? converter,
5858
ValueComparer? comparer = null,
59+
ValueComparer? keyComparer = null,
5960
CoreTypeMapping? elementMapping = null,
6061
JsonValueReaderWriter? jsonValueReaderWriter = null)
61-
=> new InMemoryTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));
62+
=> new InMemoryTypeMapping(Parameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter));
6263

6364
/// <summary>
6465
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to

‎src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -808,8 +808,7 @@ when entityType.IsMappedToJson():
808808
// (OPENJSON, json_each, etc), but we can't use it for distinct, as it would warp the results.
809809
// Instead, we will treat every non-key property as identifier.
810810

811-
// TODO: hack/workaround, see #31398
812-
foreach (var property in entityType.GetDeclaredProperties().Where(p => !p.IsPrimaryKey() && p.GetRelationalTypeMapping().ElementTypeMapping == null))
811+
foreach (var property in entityType.GetDeclaredProperties().Where(p => !p.IsPrimaryKey()))
813812
{
814813
typeProjectionIdentifiers.Add(entityProjection.BindProperty(property));
815814
typeProjectionValueComparers.Add(property.GetKeyValueComparer());
@@ -826,8 +825,7 @@ when entityType.IsMappedToJson():
826825
// entity type would have wiped the identifiers when generating the join.
827826
Check.DebugAssert(primaryKey != null, "primary key is null.");
828827

829-
// TODO: hack/workaround, see #31398
830-
foreach (var property in primaryKey.Properties.Where(x => x.GetRelationalTypeMapping().ElementTypeMapping == null))
828+
foreach (var property in primaryKey.Properties)
831829
{
832830
typeProjectionIdentifiers.Add(entityProjection.BindProperty(property));
833831
typeProjectionValueComparers.Add(property.GetKeyValueComparer());
@@ -1851,7 +1849,7 @@ ConstantExpression AddStructuralTypeProjection(StructuralTypeProjectionExpressio
18511849
{
18521850
ownerEntity = ownership.PrincipalEntityType;
18531851
}
1854-
}
1852+
}
18551853
while (ownerEntity.IsMappedToJson());
18561854

18571855
var keyPropertyCount = ownerEntity.FindPrimaryKey()!.Properties.Count;

‎src/EFCore.Relational/Storage/RelationalTypeMapping.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,16 +231,18 @@ public RelationalTypeMappingParameters WithScale(int? scale)
231231
/// </summary>
232232
/// <param name="converter">The converter.</param>
233233
/// <param name="comparer">The comparer.</param>
234+
/// <param name="keyComparer">The key comparer.</param>
234235
/// <param name="elementMapping">The element mapping, or <see langword="null" /> for non-collection mappings.</param>
235236
/// <param name="jsonValueReaderWriter">The JSON reader/writer, or <see langword="null" /> to leave unchanged.</param>
236237
/// <returns>The new parameter object.</returns>
237238
public RelationalTypeMappingParameters WithComposedConverter(
238239
ValueConverter? converter,
239240
ValueComparer? comparer,
241+
ValueComparer? keyComparer,
240242
CoreTypeMapping? elementMapping,
241243
JsonValueReaderWriter? jsonValueReaderWriter)
242244
=> new(
243-
CoreParameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter),
245+
CoreParameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter),
244246
StoreType,
245247
StoreTypePostfix,
246248
DbType,
@@ -431,9 +433,10 @@ public virtual RelationalTypeMapping Clone(int? precision, int? scale)
431433
public override CoreTypeMapping Clone(
432434
ValueConverter? converter,
433435
ValueComparer? comparer = null,
436+
ValueComparer? keyComparer = null,
434437
CoreTypeMapping? elementMapping = null,
435438
JsonValueReaderWriter? jsonValueReaderWriter = null)
436-
=> Clone(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));
439+
=> Clone(Parameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter));
437440

438441
/// <summary>
439442
/// Clones the type mapping to update facets from the mapping info, if needed.

‎src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,18 +231,21 @@ protected override CoreTypeMapping FindMapping(in TypeMappingInfo mappingInfo)
231231
{
232232
var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!;
233233

234+
var comparer = (ValueComparer?)Activator.CreateInstance(
235+
elementType.IsNullableValueType()
236+
? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType())
237+
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
238+
elementMapping!.Comparer);
239+
234240
return (RelationalTypeMapping)FindMapping(
235241
info.WithConverter(
236242
// Note that the converter info is only used temporarily here and never creates an instance.
237243
new ValueConverterInfo(modelType, typeof(string), _ => null!)))!
238244
.Clone(
239245
(ValueConverter)Activator.CreateInstance(
240246
typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!,
241-
(ValueComparer?)Activator.CreateInstance(
242-
elementType.IsNullableValueType()
243-
? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType())
244-
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
245-
elementMapping!.Comparer),
247+
comparer,
248+
comparer,
246249
elementMapping,
247250
collectionReaderWriter);
248251
}

‎src/EFCore/Storage/CoreTypeMapping.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,14 @@ public CoreTypeMappingParameters(
111111
/// </summary>
112112
/// <param name="converter">The converter.</param>
113113
/// <param name="comparer">The comparer.</param>
114+
/// <param name="keyComparer">The key comparer.</param>
114115
/// <param name="elementMapping">The element mapping, or <see langword="null" /> for non-collection mappings.</param>
115116
/// <param name="jsonValueReaderWriter">The JSON reader/writer, or <see langword="null" /> to leave unchanged.</param>
116117
/// <returns>The new parameter object.</returns>
117118
public CoreTypeMappingParameters WithComposedConverter(
118119
ValueConverter? converter,
119120
ValueComparer? comparer,
121+
ValueComparer? keyComparer,
120122
CoreTypeMapping? elementMapping,
121123
JsonValueReaderWriter? jsonValueReaderWriter)
122124
{
@@ -126,7 +128,7 @@ public CoreTypeMappingParameters WithComposedConverter(
126128
ClrType,
127129
converter ?? Converter,
128130
comparer ?? Comparer,
129-
KeyComparer,
131+
keyComparer?? KeyComparer,
130132
ProviderValueComparer,
131133
ValueGeneratorFactory,
132134
elementMapping ?? ElementTypeMapping,
@@ -266,12 +268,14 @@ public virtual ValueComparer ProviderValueComparer
266268
/// </summary>
267269
/// <param name="converter">The converter to use.</param>
268270
/// <param name="comparer">The comparer to use, or <see langword="null" /> for to keep the default.</param>
271+
/// <param name="keyComparer">The comparer to use when the value is a key, or <see langword="null" /> for to keep the default.</param>
269272
/// <param name="elementMapping">The element mapping, or <see langword="null" /> for non-collection mappings.</param>
270273
/// <param name="jsonValueReaderWriter">The JSON reader/writer, or <see langword="null" /> to leave unchanged.</param>
271274
/// <returns>A new type mapping</returns>
272275
public abstract CoreTypeMapping Clone(
273276
ValueConverter? converter,
274277
ValueComparer? comparer = null,
278+
ValueComparer? keyComparer = null,
275279
CoreTypeMapping? elementMapping = null,
276280
JsonValueReaderWriter? jsonValueReaderWriter = null);
277281

‎src/EFCore/Storage/TypeMappingSource.cs‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Concurrent;
5+
using ValueComparer = Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer;
56

67
namespace Microsoft.EntityFrameworkCore.Storage;
78

@@ -182,25 +183,30 @@ protected TypeMappingSource(TypeMappingSourceDependencies dependencies)
182183
Type? providerType,
183184
CoreTypeMapping? elementMapping)
184185
{
185-
var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!;
186+
if (TryFindJsonCollectionMapping(
187+
info, modelType, providerType, ref elementMapping, out var collectionReaderWriter))
188+
{
189+
var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!;
190+
var comparer = (ValueComparer?)Activator.CreateInstance(
191+
elementType.IsNullableValueType()
192+
? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType())
193+
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
194+
elementMapping!.Comparer);
186195

187-
return TryFindJsonCollectionMapping(
188-
info, modelType, providerType, ref elementMapping, out var collectionReaderWriter)
189-
? FindMapping(
196+
return FindMapping(
190197
info.WithConverter(
191198
// Note that the converter info is only used temporarily here and never creates an instance.
192199
new ValueConverterInfo(modelType, typeof(string), _ => null!)))!
193200
.Clone(
194201
(ValueConverter)Activator.CreateInstance(
195202
typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!,
196-
(ValueComparer?)Activator.CreateInstance(
197-
elementType.IsNullableValueType()
198-
? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType())
199-
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
200-
elementMapping!.Comparer),
203+
comparer,
204+
comparer,
201205
elementMapping,
202-
collectionReaderWriter)
203-
: null;
206+
collectionReaderWriter);
207+
}
208+
209+
return null;
204210
}
205211

206212
/// <summary>

‎test/EFCore.SqlServer.FunctionalTests/JsonTypesCustomMappingSqlServerTest.cs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ private class TestSqlServerTypeMappingSource(
5555
info.CoreTypeMappingInfo, modelType, providerType, ref elementMapping, out var collectionReaderWriter))
5656
{
5757
var elementType = TryGetElementType(modelType, typeof(IEnumerable<>))!;
58+
var comparer = (ValueComparer?)Activator.CreateInstance(
59+
IsNullableValueType(elementType)
60+
? typeof(NullableValueTypeListComparer<>).MakeGenericType(UnwrapNullableType(elementType))
61+
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
62+
elementMapping!.Comparer);
5863

5964
return (RelationalTypeMapping)FindMapping(
6065
info.WithConverter(
@@ -63,11 +68,8 @@ private class TestSqlServerTypeMappingSource(
6368
.Clone(
6469
(ValueConverter)Activator.CreateInstance(
6570
typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!,
66-
(ValueComparer?)Activator.CreateInstance(
67-
IsNullableValueType(elementType)
68-
? typeof(NullableValueTypeListComparer<>).MakeGenericType(UnwrapNullableType(elementType))
69-
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
70-
elementMapping!.Comparer),
71+
comparer,
72+
comparer,
7173
elementMapping,
7274
collectionReaderWriter);
7375
}

‎test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,7 +1338,7 @@ [OwnedCollectionBranch] nvarchar(max) '$.OwnedCollectionBranch' AS JSON,
13381338
[OwnedReferenceBranch] nvarchar(max) '$.OwnedReferenceBranch' AS JSON
13391339
) AS [o]
13401340
) AS [t]
1341-
ORDER BY [j].[Id], [t].[Name]
1341+
ORDER BY [j].[Id], [t].[Name], [t].[Names], [t].[Number]
13421342
""");
13431343
}
13441344

@@ -1400,7 +1400,7 @@ WHERE CAST(JSON_VALUE([o2].[value], '$.Date') AS datetime2) <> '2000-01-01T00:00
14001400
) AS [t2]
14011401
) AS [t1]
14021402
LEFT JOIN [JsonEntitiesBasicForCollection] AS [j0] ON [j].[Id] = [j0].[ParentId]
1403-
ORDER BY [j].[Id], [t].[c], [t].[key], [t0].[Name], [t0].[Number], [t1].[c1], [t1].[key], [t1].[c10], [t1].[key0]
1403+
ORDER BY [j].[Id], [t].[c], [t].[key], [t0].[Name], [t0].[Names], [t0].[Number], [t0].[Numbers], [t1].[c1], [t1].[key], [t1].[c10], [t1].[key0]
14041404
""");
14051405
}
14061406

@@ -1426,7 +1426,7 @@ [OwnedReferenceLeaf] nvarchar(max) '$.OwnedReferenceLeaf' AS JSON
14261426
) AS [o]
14271427
) AS [t]
14281428
LEFT JOIN [JsonEntitiesBasicForCollection] AS [j0] ON [j].[Id] = [j0].[ParentId]
1429-
ORDER BY [j].[Id], [t].[Date], [t].[Enum], [t].[Fraction], [t].[NullableEnum]
1429+
ORDER BY [j].[Id], [t].[Date], [t].[Enum], [t].[Enums], [t].[Fraction], [t].[NullableEnum], [t].[NullableEnums]
14301430
""");
14311431
}
14321432

‎test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,9 +314,11 @@ public ConcreteTypeMapping(Type clrType, ValueConverter converter, ValueComparer
314314
public override CoreTypeMapping Clone(
315315
ValueConverter converter,
316316
ValueComparer comparer = null,
317+
ValueComparer keyComparer = null,
317318
CoreTypeMapping elementMapping = null,
318319
JsonValueReaderWriter jsonValueReaderWriter = null)
319-
=> new ConcreteTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));
320+
=> new ConcreteTypeMapping(Parameters.WithComposedConverter(
321+
converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter));
320322

321323
protected override CoreTypeMapping Clone(CoreTypeMappingParameters parameters)
322324
=> new ConcreteTypeMapping(parameters);

0 commit comments

Comments
 (0)