Skip to content
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.

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

Use case-insensitive string comparisons by default on SQL Server #29955

Merged
merged 2 commits into from
Jan 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions src/EFCore.Relational/Storage/RelationalTypeMappingInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,15 @@ public bool IsKeyOrIndex
init => _coreTypeMappingInfo = _coreTypeMappingInfo with { IsKeyOrIndex = value };
}

/// <summary>
/// Indicates whether or not the mapping should be compared, etc. as if it is a key.
/// </summary>
public bool HasKeySemantics
{
get => _coreTypeMappingInfo.HasKeySemantics;
init => _coreTypeMappingInfo = _coreTypeMappingInfo with { HasKeySemantics = value };
}

/// <summary>
/// Indicates whether or not the mapping supports Unicode, or <see langword="null" /> if not defined.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public class SqlServerStringTypeMapping : StringTypeMapping
private const int UnicodeMax = 4000;
private const int AnsiMax = 8000;

private static readonly CaseInsensitiveValueComparer CaseInsensitiveValueComparer = new();

private readonly bool _isUtf16;
private readonly SqlDbType? _sqlDbType;
private readonly int _maxSpecificSize;
Expand All @@ -35,10 +37,14 @@ public SqlServerStringTypeMapping(
int? size = null,
bool fixedLength = false,
SqlDbType? sqlDbType = null,
StoreTypePostfix? storeTypePostfix = null)
StoreTypePostfix? storeTypePostfix = null,
bool useKeyComparison = false)
: this(
new RelationalTypeMappingParameters(
new CoreTypeMappingParameters(typeof(string)),
new CoreTypeMappingParameters(
typeof(string),
comparer: useKeyComparison ? CaseInsensitiveValueComparer : null,
keyComparer: useKeyComparison ? CaseInsensitiveValueComparer : null),
storeType ?? GetDefaultStoreName(unicode, fixedLength),
storeTypePostfix ?? StoreTypePostfix.Size,
GetDbType(unicode, fixedLength),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,8 @@ public SqlServerTypeMappingSource(
}

if (size == null
&& storeTypeName == null)
&& storeTypeName == null
&& !mappingInfo.HasKeySemantics)
{
return isAnsi
? isFixedLength
Expand All @@ -351,7 +352,8 @@ public SqlServerTypeMappingSource(
unicode: !isAnsi,
size: size,
fixedLength: isFixedLength,
storeTypePostfix: storeTypeName == null ? StoreTypePostfix.Size : StoreTypePostfix.None);
storeTypePostfix: storeTypeName == null ? StoreTypePostfix.Size : StoreTypePostfix.None,
useKeyComparison: mappingInfo.HasKeySemantics);
}

if (clrType == typeof(byte[]))
Expand Down
20 changes: 20 additions & 0 deletions src/EFCore/ChangeTracking/CaseInsensitiveValueComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.EntityFrameworkCore.ChangeTracking;

/// <summary>
/// Case-insensitive value comparison for strings.
/// </summary>
public class CaseInsensitiveValueComparer : ValueComparer<string?>
{
/// <summary>
/// Creates a value comparer instance.
/// </summary>
public CaseInsensitiveValueComparer()
: base(
(l, r) => string.Equals(l, r, StringComparison.OrdinalIgnoreCase),
v => v == null ? 0 : StringComparer.OrdinalIgnoreCase.GetHashCode(v))
{
}
}
14 changes: 12 additions & 2 deletions src/EFCore/Storage/TypeMappingInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ public TypeMappingInfo(
var mappingHints = customConverter?.MappingHints;
var property = principals[0];

IsKeyOrIndex = property.IsKey() || property.IsForeignKey() || property.IsIndex();
HasKeySemantics = property.IsKey() || property.IsForeignKey();
IsKeyOrIndex = HasKeySemantics || property.IsIndex();
Size = fallbackSize ?? mappingHints?.Size;
IsUnicode = fallbackUnicode ?? mappingHints?.IsUnicode;
IsRowVersion = property.IsConcurrencyToken && property.ValueGenerated == ValueGenerated.OnAddOrUpdate;
Expand Down Expand Up @@ -138,18 +139,21 @@ public TypeMappingInfo(
/// <param name="rowVersion">Specifies a row-version, or <see langword="null" /> for default.</param>
/// <param name="precision">Specifies a precision for the mapping, or <see langword="null" /> for default.</param>
/// <param name="scale">Specifies a scale for the mapping, or <see langword="null" /> for default.</param>
/// <param name="keySemantics">If <see langword="true" />, then a special mapping for a key or foreign key may be returned.</param>
public TypeMappingInfo(
Type? type = null,
bool keyOrIndex = false,
bool? unicode = null,
int? size = null,
bool? rowVersion = null,
int? precision = null,
int? scale = null)
int? scale = null,
bool keySemantics = false)
{
ClrType = type?.UnwrapNullableType();

IsKeyOrIndex = keyOrIndex;
HasKeySemantics = keySemantics;
Size = size;
IsUnicode = unicode;
IsRowVersion = rowVersion;
Expand All @@ -176,6 +180,7 @@ public TypeMappingInfo(
{
IsRowVersion = source.IsRowVersion;
IsKeyOrIndex = source.IsKeyOrIndex;
HasKeySemantics = source.HasKeySemantics;

var mappingHints = converter.MappingHints;

Expand All @@ -200,6 +205,11 @@ public TypeMappingInfo WithConverter(in ValueConverterInfo converterInfo)
/// </summary>
public bool IsKeyOrIndex { get; init; }

/// <summary>
/// Indicates whether or not the mapping should be compared, etc. as if it is a key.
/// </summary>
public bool HasKeySemantics { get; init; }

/// <summary>
/// Indicates the store-size to use for the mapping, or null if none.
/// </summary>
Expand Down
10 changes: 1 addition & 9 deletions test/EFCore.Specification.Tests/CustomConvertersTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1141,18 +1141,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
v => v.Skip(3).ToArray());
});

var caseInsensitiveComparer = new ValueComparer<string>(
(l, r) => (l == null || r == null) ? (l == r) : l.Equals(r, StringComparison.InvariantCultureIgnoreCase),
v => StringComparer.InvariantCultureIgnoreCase.GetHashCode(v),
v => v);

modelBuilder.Entity<StringKeyDataType>(
b =>
{
var property = b.Property(e => e.Id)
.HasConversion(v => "KeyValue=" + v, v => v.Substring(9)).Metadata;

property.SetValueComparer(caseInsensitiveComparer);
});

modelBuilder.Entity<StringForeignKeyDataType>(
Expand All @@ -1161,8 +1154,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.Property(e => e.StringKeyDataTypeId)
.HasConversion(
v => "KeyValue=" + v,
v => v.Substring(9),
caseInsensitiveComparer);
v => v.Substring(9));
});

modelBuilder.Entity<MaxLengthDataTypes>(
Expand Down