Skip to content

Commit d76db0a

Browse files
rojiAndriySvyryd
andauthored
Redo model validation to stop looping for every check (#37646)
Closes #37645 Co-authored-by: Andriy Svyryd <Andriy.Svyryd@microsoft.com>
1 parent 37edffa commit d76db0a

File tree

23 files changed

+2837
-2457
lines changed

23 files changed

+2837
-2457
lines changed

.github/copilot-instructions.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ If you are not sure, do not guess, just tell that you don't know or ask clarifyi
7676
- Use nullable reference types
7777
- Use proper null-checking patterns
7878
- Use the null-conditional operator (`?.`) and null-coalescing operator (`??`) when appropriate
79+
- Don't disable nullability with a preprocessor directive (`#nullable disable`)
7980

8081
## Architecture and Design Patterns
8182

src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs

Lines changed: 308 additions & 257 deletions
Large diffs are not rendered by default.

src/EFCore.Relational/Diagnostics/RelationalEventId.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ private enum Id
9999
// Model validation events
100100
ModelValidationKeyDefaultValueWarning = CoreEventId.RelationalBaseId + 600,
101101
BoolWithDefaultWarning,
102-
AllIndexPropertiesNotToMappedToAnyTable,
102+
AllIndexPropertiesNotMappedToAnyTable,
103103
IndexPropertiesBothMappedAndNotMappedToTable,
104104
IndexPropertiesMappedToNonOverlappingTables,
105105
ForeignKeyPropertiesMappedToUnrelatedTables,
@@ -914,8 +914,8 @@ private static EventId MakeValidationId(Id id)
914914
/// This event uses the <see cref="IndexEventData" /> payload when used with a <see cref="DiagnosticSource" />.
915915
/// </para>
916916
/// </remarks>
917-
public static readonly EventId AllIndexPropertiesNotToMappedToAnyTable =
918-
MakeValidationId(Id.AllIndexPropertiesNotToMappedToAnyTable);
917+
public static readonly EventId AllIndexPropertiesNotMappedToAnyTable =
918+
MakeValidationId(Id.AllIndexPropertiesNotMappedToAnyTable);
919919

920920
/// <summary>
921921
/// An index specifies properties some of which are mapped and some of which are not mapped to a column in a table.

src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2868,19 +2868,19 @@ private static string BatchSmallerThanMinBatchSize(EventDefinitionBase definitio
28682868
}
28692869

28702870
/// <summary>
2871-
/// Logs the <see cref="RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable" /> event.
2871+
/// Logs the <see cref="RelationalEventId.AllIndexPropertiesNotMappedToAnyTable" /> event.
28722872
/// </summary>
28732873
/// <param name="diagnostics">The diagnostics logger to use.</param>
28742874
/// <param name="entityType">The entity type on which the index is defined.</param>
28752875
/// <param name="index">The index on the entity type.</param>
2876-
public static void AllIndexPropertiesNotToMappedToAnyTable(
2876+
public static void AllIndexPropertiesNotMappedToAnyTable(
28772877
this IDiagnosticsLogger<DbLoggerCategory.Model.Validation> diagnostics,
28782878
IEntityType entityType,
28792879
IIndex index)
28802880
{
28812881
if (index.Name == null)
28822882
{
2883-
var definition = RelationalResources.LogUnnamedIndexAllPropertiesNotToMappedToAnyTable(diagnostics);
2883+
var definition = RelationalResources.LogUnnamedIndexAllPropertiesNotMappedToAnyTable(diagnostics);
28842884

28852885
if (diagnostics.ShouldLog(definition))
28862886
{
@@ -2904,7 +2904,7 @@ public static void AllIndexPropertiesNotToMappedToAnyTable(
29042904
}
29052905
else
29062906
{
2907-
var definition = RelationalResources.LogNamedIndexAllPropertiesNotToMappedToAnyTable(diagnostics);
2907+
var definition = RelationalResources.LogNamedIndexAllPropertiesNotMappedToAnyTable(diagnostics);
29082908

29092909
if (diagnostics.ShouldLog(definition))
29102910
{

src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
518518
/// doing so can result in application failures when updating to a new Entity Framework Core release.
519519
/// </summary>
520520
[EntityFrameworkInternal]
521-
public EventDefinitionBase? LogNamedIndexAllPropertiesNotToMappedToAnyTable;
521+
public EventDefinitionBase? LogNamedIndexAllPropertiesNotMappedToAnyTable;
522522

523523
/// <summary>
524524
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -527,7 +527,7 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
527527
/// doing so can result in application failures when updating to a new Entity Framework Core release.
528528
/// </summary>
529529
[EntityFrameworkInternal]
530-
public EventDefinitionBase? LogUnnamedIndexAllPropertiesNotToMappedToAnyTable;
530+
public EventDefinitionBase? LogUnnamedIndexAllPropertiesNotMappedToAnyTable;
531531

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

src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,20 @@ public static class RelationalIndexExtensions
3535
/// <param name="storeObject">The identifier of the store object.</param>
3636
/// <returns>The name of the index in the database.</returns>
3737
public static string? GetDatabaseName(this IReadOnlyIndex index, in StoreObjectIdentifier storeObject)
38-
=> index.GetDatabaseName(storeObject, null);
38+
{
39+
if (storeObject.StoreObjectType != StoreObjectType.Table)
40+
{
41+
return null;
42+
}
43+
44+
var defaultName = index.GetDefaultDatabaseName(storeObject);
45+
var annotation = index.FindAnnotation(RelationalAnnotationNames.Name);
46+
return annotation != null && defaultName != null
47+
? (string?)annotation.Value
48+
: defaultName != null
49+
? index.Name ?? defaultName
50+
: defaultName;
51+
}
3952

4053
/// <summary>
4154
/// Returns the default name that would be used for this index.
@@ -67,7 +80,60 @@ public static class RelationalIndexExtensions
6780
/// <param name="storeObject">The identifier of the store object.</param>
6881
/// <returns>The default name that would be used for this index.</returns>
6982
public static string? GetDefaultDatabaseName(this IReadOnlyIndex index, in StoreObjectIdentifier storeObject)
70-
=> index.GetDefaultDatabaseName(storeObject, null);
83+
{
84+
if (storeObject.StoreObjectType != StoreObjectType.Table)
85+
{
86+
return null;
87+
}
88+
89+
var columnNames = index.Properties.GetColumnNames(storeObject);
90+
if (columnNames == null)
91+
{
92+
return null;
93+
}
94+
95+
var rootIndex = index;
96+
97+
// Limit traversal to avoid getting stuck in a cycle (validation will throw for these later)
98+
// Using a hashset is detrimental to the perf when there are no cycles
99+
for (var i = 0; i < Metadata.Internal.RelationalEntityTypeExtensions.MaxEntityTypesSharingTable; i++)
100+
{
101+
IReadOnlyIndex? linkedIndex = null;
102+
foreach (var otherIndex in rootIndex.DeclaringEntityType
103+
.FindRowInternalForeignKeys(storeObject)
104+
.SelectMany(fk => fk.PrincipalEntityType.GetIndexes()))
105+
{
106+
var otherColumnNames = otherIndex.Properties.GetColumnNames(storeObject);
107+
if ((otherColumnNames != null)
108+
&& otherColumnNames.SequenceEqual(columnNames))
109+
{
110+
linkedIndex = otherIndex;
111+
break;
112+
}
113+
}
114+
115+
if (linkedIndex == null)
116+
{
117+
break;
118+
}
119+
120+
rootIndex = linkedIndex;
121+
}
122+
123+
if (rootIndex != index)
124+
{
125+
return rootIndex.GetDatabaseName(storeObject);
126+
}
127+
128+
var baseName = new StringBuilder()
129+
.Append("IX_")
130+
.Append(storeObject.Name)
131+
.Append('_')
132+
.AppendJoin(columnNames, "_")
133+
.ToString();
134+
135+
return Uniquifier.Truncate(baseName, index.DeclaringEntityType.Model.GetMaxIdentifierLength());
136+
}
71137

72138
/// <summary>
73139
/// Sets the name of the index in the database.

0 commit comments

Comments
 (0)