Skip to content

Set the key comparer as well as the normal comparer when mapping a primitive collection #31661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ajcvickers merged 1 commit into from
Sep 9, 2023
Merged

Set the key comparer as well as the normal comparer when mapping a primitive collection #31661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/EFCore.Cosmos/Storage/Internal/CosmosTypeMapping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ protected CosmosTypeMapping(CoreTypeMappingParameters parameters)
public override CoreTypeMapping Clone(
ValueConverter? converter,
ValueComparer? comparer = null,
ValueComparer? keyComparer = null,
CoreTypeMapping? elementMapping = null,
JsonValueReaderWriter? jsonValueReaderWriter = null)
=> new CosmosTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));
=> new CosmosTypeMapping(Parameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore.InMemory/Storage/Internal/InMemoryTypeMapping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ private InMemoryTypeMapping(CoreTypeMappingParameters parameters)
public override CoreTypeMapping Clone(
ValueConverter? converter,
ValueComparer? comparer = null,
ValueComparer? keyComparer = null,
CoreTypeMapping? elementMapping = null,
JsonValueReaderWriter? jsonValueReaderWriter = null)
=> new InMemoryTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));
=> new InMemoryTypeMapping(Parameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter));

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
8 changes: 3 additions & 5 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,8 +808,7 @@ when entityType.IsMappedToJson():
// (OPENJSON, json_each, etc), but we can't use it for distinct, as it would warp the results.
// Instead, we will treat every non-key property as identifier.

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

// TODO: hack/workaround, see #31398
foreach (var property in primaryKey.Properties.Where(x => x.GetRelationalTypeMapping().ElementTypeMapping == null))
foreach (var property in primaryKey.Properties)
{
typeProjectionIdentifiers.Add(entityProjection.BindProperty(property));
typeProjectionValueComparers.Add(property.GetKeyValueComparer());
Expand Down Expand Up @@ -1851,7 +1849,7 @@ ConstantExpression AddStructuralTypeProjection(StructuralTypeProjectionExpressio
{
ownerEntity = ownership.PrincipalEntityType;
}
}
}
while (ownerEntity.IsMappedToJson());

var keyPropertyCount = ownerEntity.FindPrimaryKey()!.Properties.Count;
Expand Down
7 changes: 5 additions & 2 deletions src/EFCore.Relational/Storage/RelationalTypeMapping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,16 +231,18 @@ public RelationalTypeMappingParameters WithScale(int? scale)
/// </summary>
/// <param name="converter">The converter.</param>
/// <param name="comparer">The comparer.</param>
/// <param name="keyComparer">The key comparer.</param>
/// <param name="elementMapping">The element mapping, or <see langword="null" /> for non-collection mappings.</param>
/// <param name="jsonValueReaderWriter">The JSON reader/writer, or <see langword="null" /> to leave unchanged.</param>
/// <returns>The new parameter object.</returns>
public RelationalTypeMappingParameters WithComposedConverter(
ValueConverter? converter,
ValueComparer? comparer,
ValueComparer? keyComparer,
CoreTypeMapping? elementMapping,
JsonValueReaderWriter? jsonValueReaderWriter)
=> new(
CoreParameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter),
CoreParameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter),
StoreType,
StoreTypePostfix,
DbType,
Expand Down Expand Up @@ -431,9 +433,10 @@ public virtual RelationalTypeMapping Clone(int? precision, int? scale)
public override CoreTypeMapping Clone(
ValueConverter? converter,
ValueComparer? comparer = null,
ValueComparer? keyComparer = null,
CoreTypeMapping? elementMapping = null,
JsonValueReaderWriter? jsonValueReaderWriter = null)
=> Clone(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));
=> Clone(Parameters.WithComposedConverter(converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter));

/// <summary>
/// Clones the type mapping to update facets from the mapping info, if needed.
Expand Down
13 changes: 8 additions & 5 deletions src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,18 +231,21 @@ protected override CoreTypeMapping FindMapping(in TypeMappingInfo mappingInfo)
{
var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!;

var comparer = (ValueComparer?)Activator.CreateInstance(
elementType.IsNullableValueType()
? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType())
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
elementMapping!.Comparer);

return (RelationalTypeMapping)FindMapping(
info.WithConverter(
// Note that the converter info is only used temporarily here and never creates an instance.
new ValueConverterInfo(modelType, typeof(string), _ => null!)))!
.Clone(
(ValueConverter)Activator.CreateInstance(
typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!,
(ValueComparer?)Activator.CreateInstance(
elementType.IsNullableValueType()
? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType())
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
elementMapping!.Comparer),
comparer,
comparer,
elementMapping,
collectionReaderWriter);
}
Expand Down
6 changes: 5 additions & 1 deletion src/EFCore/Storage/CoreTypeMapping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,14 @@ public CoreTypeMappingParameters(
/// </summary>
/// <param name="converter">The converter.</param>
/// <param name="comparer">The comparer.</param>
/// <param name="keyComparer">The key comparer.</param>
/// <param name="elementMapping">The element mapping, or <see langword="null" /> for non-collection mappings.</param>
/// <param name="jsonValueReaderWriter">The JSON reader/writer, or <see langword="null" /> to leave unchanged.</param>
/// <returns>The new parameter object.</returns>
public CoreTypeMappingParameters WithComposedConverter(
ValueConverter? converter,
ValueComparer? comparer,
ValueComparer? keyComparer,
CoreTypeMapping? elementMapping,
JsonValueReaderWriter? jsonValueReaderWriter)
{
Expand All @@ -126,7 +128,7 @@ public CoreTypeMappingParameters WithComposedConverter(
ClrType,
converter ?? Converter,
comparer ?? Comparer,
KeyComparer,
keyComparer?? KeyComparer,
ProviderValueComparer,
ValueGeneratorFactory,
elementMapping ?? ElementTypeMapping,
Expand Down Expand Up @@ -266,12 +268,14 @@ public virtual ValueComparer ProviderValueComparer
/// </summary>
/// <param name="converter">The converter to use.</param>
/// <param name="comparer">The comparer to use, or <see langword="null" /> for to keep the default.</param>
/// <param name="keyComparer">The comparer to use when the value is a key, or <see langword="null" /> for to keep the default.</param>
/// <param name="elementMapping">The element mapping, or <see langword="null" /> for non-collection mappings.</param>
/// <param name="jsonValueReaderWriter">The JSON reader/writer, or <see langword="null" /> to leave unchanged.</param>
/// <returns>A new type mapping</returns>
public abstract CoreTypeMapping Clone(
ValueConverter? converter,
ValueComparer? comparer = null,
ValueComparer? keyComparer = null,
CoreTypeMapping? elementMapping = null,
JsonValueReaderWriter? jsonValueReaderWriter = null);

Expand Down
28 changes: 17 additions & 11 deletions src/EFCore/Storage/TypeMappingSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Concurrent;
using ValueComparer = Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer;

namespace Microsoft.EntityFrameworkCore.Storage;

Expand Down Expand Up @@ -182,25 +183,30 @@ protected TypeMappingSource(TypeMappingSourceDependencies dependencies)
Type? providerType,
CoreTypeMapping? elementMapping)
{
var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!;
if (TryFindJsonCollectionMapping(
info, modelType, providerType, ref elementMapping, out var collectionReaderWriter))
{
var elementType = modelType.TryGetElementType(typeof(IEnumerable<>))!;
var comparer = (ValueComparer?)Activator.CreateInstance(
elementType.IsNullableValueType()
? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType())
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
elementMapping!.Comparer);

return TryFindJsonCollectionMapping(
info, modelType, providerType, ref elementMapping, out var collectionReaderWriter)
? FindMapping(
return FindMapping(
info.WithConverter(
// Note that the converter info is only used temporarily here and never creates an instance.
new ValueConverterInfo(modelType, typeof(string), _ => null!)))!
.Clone(
(ValueConverter)Activator.CreateInstance(
typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!,
(ValueComparer?)Activator.CreateInstance(
elementType.IsNullableValueType()
? typeof(NullableValueTypeListComparer<>).MakeGenericType(elementType.UnwrapNullableType())
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
elementMapping!.Comparer),
comparer,
comparer,
elementMapping,
collectionReaderWriter)
: null;
collectionReaderWriter);
}

return null;
}

/// <summary>
Expand Down
12 changes: 7 additions & 5 deletions test/EFCore.SqlServer.FunctionalTests/JsonTypesCustomMappingSqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ private class TestSqlServerTypeMappingSource(
info.CoreTypeMappingInfo, modelType, providerType, ref elementMapping, out var collectionReaderWriter))
{
var elementType = TryGetElementType(modelType, typeof(IEnumerable<>))!;
var comparer = (ValueComparer?)Activator.CreateInstance(
IsNullableValueType(elementType)
? typeof(NullableValueTypeListComparer<>).MakeGenericType(UnwrapNullableType(elementType))
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
elementMapping!.Comparer);

return (RelationalTypeMapping)FindMapping(
info.WithConverter(
Expand All @@ -63,11 +68,8 @@ private class TestSqlServerTypeMappingSource(
.Clone(
(ValueConverter)Activator.CreateInstance(
typeof(CollectionToJsonStringConverter<>).MakeGenericType(elementType), collectionReaderWriter!)!,
(ValueComparer?)Activator.CreateInstance(
IsNullableValueType(elementType)
? typeof(NullableValueTypeListComparer<>).MakeGenericType(UnwrapNullableType(elementType))
: typeof(ListComparer<>).MakeGenericType(elementMapping!.Comparer.Type),
elementMapping!.Comparer),
comparer,
comparer,
elementMapping,
collectionReaderWriter);
}
Expand Down
6 changes: 3 additions & 3 deletions test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1338,7 +1338,7 @@ [OwnedCollectionBranch] nvarchar(max) '$.OwnedCollectionBranch' AS JSON,
[OwnedReferenceBranch] nvarchar(max) '$.OwnedReferenceBranch' AS JSON
) AS [o]
) AS [t]
ORDER BY [j].[Id], [t].[Name]
ORDER BY [j].[Id], [t].[Name], [t].[Names], [t].[Number]
""");
}

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

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

Expand Down
4 changes: 3 additions & 1 deletion test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,11 @@ public ConcreteTypeMapping(Type clrType, ValueConverter converter, ValueComparer
public override CoreTypeMapping Clone(
ValueConverter converter,
ValueComparer comparer = null,
ValueComparer keyComparer = null,
CoreTypeMapping elementMapping = null,
JsonValueReaderWriter jsonValueReaderWriter = null)
=> new ConcreteTypeMapping(Parameters.WithComposedConverter(converter, comparer, elementMapping, jsonValueReaderWriter));
=> new ConcreteTypeMapping(Parameters.WithComposedConverter(
converter, comparer, keyComparer, elementMapping, jsonValueReaderWriter));

protected override CoreTypeMapping Clone(CoreTypeMappingParameters parameters)
=> new ConcreteTypeMapping(parameters);
Expand Down