Skip to content

Commit a9cea3e

Browse files
ajcvickersajcvickers
authored
[release/8.0] Stop using case-insensitive comparisons for indexed properties (#32899) (#32919)
* Stop using case-insensitive comparisons for indexed properties (#32899) Fixes #32898 Note that we didn't change what we do in the update pipeline, which appears to already be using provider values. * Fix merge; add quirks
1 parent badb28f commit a9cea3e

File tree

5 files changed

+240
-9
lines changed

5 files changed

+240
-9
lines changed

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

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public RelationalTypeMappingInfo(
151151
int? scale)
152152
{
153153
// Note: Empty string is allowed for store type name because SQLite
154-
_coreTypeMappingInfo = new TypeMappingInfo(null, null, false, unicode, size, null, precision, scale);
154+
_coreTypeMappingInfo = new TypeMappingInfo(null, null, false, unicode, size, null, precision, scale, false);
155155
StoreTypeName = storeTypeName;
156156
StoreTypeNameBase = storeTypeNameBase;
157157
IsFixedLength = null;
@@ -225,6 +225,42 @@ public RelationalTypeMappingInfo(
225225
/// <param name="precision">Specifies a precision for the mapping, or <see langword="null" /> for default.</param>
226226
/// <param name="scale">Specifies a scale for the mapping, or <see langword="null" /> for default.</param>
227227
/// <param name="dbType">The suggested <see cref="DbType" />, or <see langword="null" /> for default.</param>
228+
[Obsolete("Use overload that takes 'key' parameter.")]
229+
public RelationalTypeMappingInfo(
230+
Type? type,
231+
RelationalTypeMapping? elementTypeMapping,
232+
string? storeTypeName,
233+
string? storeTypeNameBase,
234+
bool keyOrIndex,
235+
bool? unicode,
236+
int? size,
237+
bool? rowVersion,
238+
bool? fixedLength,
239+
int? precision,
240+
int? scale,
241+
DbType? dbType)
242+
: this(
243+
type, elementTypeMapping, storeTypeName, storeTypeNameBase, keyOrIndex, unicode, size, rowVersion, fixedLength,
244+
precision, scale, dbType, keyOrIndex)
245+
{
246+
}
247+
248+
/// <summary>
249+
/// Creates a new instance of <see cref="TypeMappingInfo" />.
250+
/// </summary>
251+
/// <param name="type">The CLR type in the model for which mapping is needed.</param>
252+
/// <param name="elementTypeMapping">The type mapping for elements, if known.</param>
253+
/// <param name="storeTypeName">The database type name.</param>
254+
/// <param name="storeTypeNameBase">The provider-specific relational type name, with any facets removed.</param>
255+
/// <param name="keyOrIndex">If <see langword="true" />, then a special mapping for a key or index may be returned.</param>
256+
/// <param name="unicode">Specifies Unicode or ANSI mapping, or <see langword="null" /> for default.</param>
257+
/// <param name="size">Specifies a size for the mapping, or <see langword="null" /> for default.</param>
258+
/// <param name="rowVersion">Specifies a row-version, or <see langword="null" /> for default.</param>
259+
/// <param name="fixedLength">Specifies a fixed length mapping, or <see langword="null" /> for default.</param>
260+
/// <param name="precision">Specifies a precision for the mapping, or <see langword="null" /> for default.</param>
261+
/// <param name="scale">Specifies a scale for the mapping, or <see langword="null" /> for default.</param>
262+
/// <param name="dbType">The suggested <see cref="DbType" />, or <see langword="null" /> for default.</param>
263+
/// <param name="key">If <see langword="true" />, then a special mapping for a key may be returned.</param>
228264
public RelationalTypeMappingInfo(
229265
Type? type = null,
230266
RelationalTypeMapping? elementTypeMapping = null,
@@ -237,9 +273,10 @@ public RelationalTypeMappingInfo(
237273
bool? fixedLength = null,
238274
int? precision = null,
239275
int? scale = null,
240-
DbType? dbType = null)
276+
DbType? dbType = null,
277+
bool key = false)
241278
{
242-
_coreTypeMappingInfo = new TypeMappingInfo(type, elementTypeMapping, keyOrIndex, unicode, size, rowVersion, precision, scale);
279+
_coreTypeMappingInfo = new TypeMappingInfo(type, elementTypeMapping, keyOrIndex, unicode, size, rowVersion, precision, scale, key);
243280

244281
IsFixedLength = fixedLength;
245282
StoreTypeName = storeTypeName;
@@ -301,7 +338,16 @@ public int? Scale
301338
public DbType? DbType { get; init; }
302339

303340
/// <summary>
304-
/// Indicates whether or not the mapping is part of a key or index.
341+
/// Indicates whether or not the mapping is part of a key or foreign key.
342+
/// </summary>
343+
public bool IsKey
344+
{
345+
get => _coreTypeMappingInfo.IsKey;
346+
init => _coreTypeMappingInfo = _coreTypeMappingInfo with { IsKey = value };
347+
}
348+
349+
/// <summary>
350+
/// Indicates whether or not the mapping is part of a key, foreign key, or index.
305351
/// </summary>
306352
public bool IsKeyOrIndex
307353
{

‎src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal;
1515
/// </summary>
1616
public class SqlServerTypeMappingSource : RelationalTypeMappingSource
1717
{
18+
private static readonly bool UseOldBehavior32898 =
19+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32898", out var enabled32898) && enabled32898;
20+
1821
private static readonly SqlServerFloatTypeMapping RealAlias
1922
= new("placeholder", storeTypePostfix: StoreTypePostfix.None);
2023

@@ -338,7 +341,7 @@ public SqlServerTypeMappingSource(
338341
size: size,
339342
fixedLength: isFixedLength,
340343
storeTypePostfix: storeTypeName == null ? StoreTypePostfix.Size : StoreTypePostfix.None,
341-
useKeyComparison: mappingInfo.IsKeyOrIndex);
344+
useKeyComparison: UseOldBehavior32898 ? mappingInfo.IsKeyOrIndex : mappingInfo.IsKey);
342345
}
343346

344347
if (clrType == typeof(byte[]))

‎src/EFCore/Storage/TypeMappingInfo.cs‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ public TypeMappingInfo(
182182
var property = principals[0];
183183

184184
ElementTypeMapping = property.GetElementType()?.FindTypeMapping();
185-
IsKeyOrIndex = property.IsKey() || property.IsForeignKey() || property.IsIndex();
185+
IsKey = property.IsKey() || property.IsForeignKey();
186+
IsKeyOrIndex = IsKey || property.IsIndex();
186187
Size = fallbackSize ?? mappingHints?.Size;
187188
IsUnicode = fallbackUnicode ?? mappingHints?.IsUnicode;
188189
IsRowVersion = property is { IsConcurrencyToken: true, ValueGenerated: ValueGenerated.OnAddOrUpdate };
@@ -227,6 +228,32 @@ public TypeMappingInfo(
227228
/// <param name="rowVersion">Specifies a row-version, or <see langword="null" /> for default.</param>
228229
/// <param name="precision">Specifies a precision for the mapping, or <see langword="null" /> for default.</param>
229230
/// <param name="scale">Specifies a scale for the mapping, or <see langword="null" /> for default.</param>
231+
[Obsolete("Use overload that takes 'key' parameter.")]
232+
public TypeMappingInfo(
233+
Type? type,
234+
CoreTypeMapping? elementTypeMapping,
235+
bool keyOrIndex,
236+
bool? unicode,
237+
int? size,
238+
bool? rowVersion,
239+
int? precision,
240+
int? scale)
241+
: this(type, elementTypeMapping, keyOrIndex, unicode, size, rowVersion, precision, scale, false)
242+
{
243+
}
244+
245+
/// <summary>
246+
/// Creates a new instance of <see cref="TypeMappingInfo" />.
247+
/// </summary>
248+
/// <param name="type">The CLR type in the model for which mapping is needed.</param>
249+
/// <param name="elementTypeMapping">The type mapping for elements, if known.</param>
250+
/// <param name="keyOrIndex">If <see langword="true" />, then a special mapping for a key or index may be returned.</param>
251+
/// <param name="unicode">Specifies Unicode or ANSI mapping, or <see langword="null" /> for default.</param>
252+
/// <param name="size">Specifies a size for the mapping, or <see langword="null" /> for default.</param>
253+
/// <param name="rowVersion">Specifies a row-version, or <see langword="null" /> for default.</param>
254+
/// <param name="precision">Specifies a precision for the mapping, or <see langword="null" /> for default.</param>
255+
/// <param name="scale">Specifies a scale for the mapping, or <see langword="null" /> for default.</param>
256+
/// <param name="key">If <see langword="true" />, then a special mapping for a key may be returned.</param>
230257
public TypeMappingInfo(
231258
Type? type = null,
232259
CoreTypeMapping? elementTypeMapping = null,
@@ -235,11 +262,12 @@ public TypeMappingInfo(
235262
int? size = null,
236263
bool? rowVersion = null,
237264
int? precision = null,
238-
int? scale = null)
265+
int? scale = null,
266+
bool key = false)
239267
{
240268
ClrType = type?.UnwrapNullableType();
241269
ElementTypeMapping = elementTypeMapping;
242-
270+
IsKey = key;
243271
IsKeyOrIndex = keyOrIndex;
244272
Size = size;
245273
IsUnicode = unicode;
@@ -266,6 +294,7 @@ public TypeMappingInfo(
266294
int? scale = null)
267295
{
268296
IsRowVersion = source.IsRowVersion;
297+
IsKey = source.IsKey;
269298
IsKeyOrIndex = source.IsKeyOrIndex;
270299

271300
var mappingHints = converter.MappingHints;
@@ -295,7 +324,12 @@ public TypeMappingInfo WithConverter(in ValueConverterInfo converterInfo)
295324
=> new(this, converterInfo);
296325

297326
/// <summary>
298-
/// Indicates whether or not the mapping is part of a key or index.
327+
/// Indicates whether or not the mapping is part of a key or foreign key.
328+
/// </summary>
329+
public bool IsKey { get; init; }
330+
331+
/// <summary>
332+
/// Indicates whether or not the mapping is part of a key, foreign key, or index.
299333
/// </summary>
300334
public bool IsKeyOrIndex { get; init; }
301335

‎test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerOwnedTest.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,17 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
572572
b.Property(e => e.IdUserState).HasDefaultValue(1).HasSentinel(667);
573573
b.HasOne(e => e.UserState).WithMany(e => e.Users).HasForeignKey(e => e.IdUserState);
574574
});
575+
576+
modelBuilder.Entity<StringKeyAndIndexParent>(
577+
b =>
578+
{
579+
b.HasAlternateKey(e => e.AlternateId);
580+
b.OwnsOne(
581+
x => x.Child, b =>
582+
{
583+
b.WithOwner(e => e.Parent).HasForeignKey(e => e.ParentId);
584+
});
585+
});
575586
}
576587
}
577588
}

‎test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerTestBase.cs‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,134 @@ protected GraphUpdatesSqlServerTestBase(TFixture fixture)
1111
{
1212
}
1313

14+
[ConditionalFact] // Issue #32638
15+
public virtual void Key_and_index_properties_use_appropriate_comparer()
16+
{
17+
var parent = new StringKeyAndIndexParent
18+
{
19+
Id = "Parent",
20+
AlternateId = "Parent",
21+
Index = "Index",
22+
UniqueIndex = "UniqueIndex"
23+
};
24+
25+
var child = new StringKeyAndIndexChild
26+
{
27+
Id = "Child",
28+
ParentId = "parent"
29+
};
30+
31+
using var context = CreateContext();
32+
context.AttachRange(parent, child);
33+
34+
Assert.Same(child, parent.Child);
35+
Assert.Same(parent, child.Parent);
36+
37+
parent.Id = "parent";
38+
parent.AlternateId = "parent";
39+
parent.Index = "index";
40+
parent.UniqueIndex = "uniqueIndex";
41+
child.Id = "child";
42+
child.ParentId = "Parent";
43+
44+
context.ChangeTracker.DetectChanges();
45+
46+
var parentEntry = context.Entry(parent);
47+
Assert.Equal(EntityState.Modified, parentEntry.State);
48+
Assert.False(parentEntry.Property(e => e.Id).IsModified);
49+
Assert.False(parentEntry.Property(e => e.AlternateId).IsModified);
50+
Assert.True(parentEntry.Property(e => e.Index).IsModified);
51+
Assert.True(parentEntry.Property(e => e.UniqueIndex).IsModified);
52+
53+
var childEntry = context.Entry(child);
54+
55+
if (childEntry.Metadata.IsOwned())
56+
{
57+
Assert.Equal(EntityState.Modified, childEntry.State);
58+
Assert.True(childEntry.Property(e => e.Id).IsModified); // Not a key for the owned type
59+
Assert.False(childEntry.Property(e => e.ParentId).IsModified);
60+
}
61+
else
62+
{
63+
Assert.Equal(EntityState.Unchanged, childEntry.State);
64+
Assert.False(childEntry.Property(e => e.Id).IsModified);
65+
Assert.False(childEntry.Property(e => e.ParentId).IsModified);
66+
}
67+
68+
}
69+
70+
protected class StringKeyAndIndexParent : NotifyingEntity
71+
{
72+
private string _id;
73+
private string _alternateId;
74+
private string _uniqueIndex;
75+
private string _index;
76+
private StringKeyAndIndexChild _child;
77+
78+
public string Id
79+
{
80+
get => _id;
81+
set => SetWithNotify(value, ref _id);
82+
}
83+
84+
public string AlternateId
85+
{
86+
get => _alternateId;
87+
set => SetWithNotify(value, ref _alternateId);
88+
}
89+
90+
public string Index
91+
{
92+
get => _index;
93+
set => SetWithNotify(value, ref _index);
94+
}
95+
96+
public string UniqueIndex
97+
{
98+
get => _uniqueIndex;
99+
set => SetWithNotify(value, ref _uniqueIndex);
100+
}
101+
102+
public StringKeyAndIndexChild Child
103+
{
104+
get => _child;
105+
set => SetWithNotify(value, ref _child);
106+
}
107+
}
108+
109+
protected class StringKeyAndIndexChild : NotifyingEntity
110+
{
111+
private string _id;
112+
private string _parentId;
113+
private int _foo;
114+
private StringKeyAndIndexParent _parent;
115+
116+
public string Id
117+
{
118+
get => _id;
119+
set => SetWithNotify(value, ref _id);
120+
}
121+
122+
public string ParentId
123+
{
124+
get => _parentId;
125+
set => SetWithNotify(value, ref _parentId);
126+
}
127+
128+
129+
public int Foo
130+
{
131+
get => _foo;
132+
set => SetWithNotify(value, ref _foo);
133+
}
134+
135+
public StringKeyAndIndexParent Parent
136+
{
137+
get => _parent;
138+
set => SetWithNotify(value, ref _parent);
139+
}
140+
}
141+
14142
protected override IQueryable<Root> ModifyQueryRoot(IQueryable<Root> query)
15143
=> query.AsSplitQuery();
16144

@@ -59,6 +187,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
59187

60188
modelBuilder.Entity<SomethingOfCategoryA>().Property<int>("CategoryId").HasDefaultValue(1);
61189
modelBuilder.Entity<SomethingOfCategoryB>().Property(e => e.CategoryId).HasDefaultValue(2);
190+
191+
modelBuilder.Entity<StringKeyAndIndexParent>(
192+
b =>
193+
{
194+
b.HasOne(e => e.Child)
195+
.WithOne(e => e.Parent)
196+
.HasForeignKey<StringKeyAndIndexChild>(e => e.ParentId)
197+
.HasPrincipalKey<StringKeyAndIndexParent>(e => e.AlternateId);
198+
});
62199
}
63200
}
64201
}

0 commit comments

Comments
 (0)