Skip to content

Commit

Permalink
SqlServer: Ignore Fks whose principal table data is not found (#25852)
Browse files Browse the repository at this point in the history
Resolves #23359
  • Loading branch information
smitpatel authored Sep 3, 2021
1 parent d7392b7 commit 85e064c
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,14 @@ public class SqlServerLoggingDefinitions : RelationalLoggingDefinitions
/// </summary>
public EventDefinitionBase? LogDuplicateForeignKeyConstraintIgnored;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public EventDefinitionBase? LogPrincipalTableInformationNotFound;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
10 changes: 9 additions & 1 deletion src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ private enum Id
ForeignKeyPrincipalColumnMissingWarning,
ReflexiveConstraintIgnored,
DuplicateForeignKeyConstraintIgnored,
ColumnWithoutTypeWarning
ColumnWithoutTypeWarning,
ForeignKeyReferencesUnknownPrincipalTableWarning
}

private static readonly string _validationPrefix = DbLoggerCategory.Model.Validation.Name + ".";
Expand Down Expand Up @@ -184,6 +185,13 @@ private static EventId MakeScaffoldingId(Id id)
public static readonly EventId ForeignKeyReferencesMissingPrincipalTableWarning =
MakeScaffoldingId(Id.ForeignKeyReferencesMissingPrincipalTableWarning);

/// <summary>
/// A foreign key references a unknown table at the principal end.
/// This event is in the <see cref="DbLoggerCategory.Scaffolding" /> category.
/// </summary>
public static readonly EventId ForeignKeyReferencesUnknownPrincipalTableWarning =
MakeScaffoldingId(Id.ForeignKeyReferencesUnknownPrincipalTableWarning);

/// <summary>
/// A table was found.
/// This event is in the <see cref="DbLoggerCategory.Scaffolding" /> category.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,27 @@ public static void IndexFound(
// No DiagnosticsSource events because these are purely design-time messages
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public static void ForeignKeyReferencesUnknownPrincipalTableWarning(
this IDiagnosticsLogger<DbLoggerCategory.Scaffolding> diagnostics,
string? foreignKeyName,
string? tableName)
{
var definition = SqlServerResources.LogPrincipalTableInformationNotFound(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, foreignKeyName, tableName);
}

// No DiagnosticsSource events because these are purely design-time messages
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down Expand Up @@ -544,7 +565,7 @@ public static void DuplicateForeignKeyConstraintIgnored(
string tableName,
string duplicateForeignKeyName)
{
var definition = SqlServerResources.DuplicateForeignKeyConstraintIgnored(diagnostics);
var definition = SqlServerResources.LogDuplicateForeignKeyConstraintIgnored(diagnostics);

if (diagnostics.ShouldLog(definition))
{
Expand Down
78 changes: 53 additions & 25 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 8 additions & 4 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@
<value>No store type was specified for the decimal property '{property}' on entity type '{entityType}'. This will cause values to be silently truncated if they do not fit in the default precision and scale. Explicitly specify the SQL server column type that can accommodate all the values in 'OnModelCreating' using 'HasColumnType', specify precision and scale using 'HasPrecision', or configure a value converter using 'HasConversion'.</value>
<comment>Warning SqlServerEventId.DecimalTypeDefaultWarning string string</comment>
</data>
<data name="LogDuplicateForeignKeyConstraintIgnored" xml:space="preserve">
<value>Skipping foreign key '{foreignKeyName}' on table '{tableName}' since it is a duplicate of '{duplicateForeignKeyName}'.</value>
<comment>Warning SqlServerEventId.DuplicateForeignKeyConstraintIgnored string string string</comment>
</data>
<data name="LogFoundColumn" xml:space="preserve">
<value>Found column with table: {tableName}, column name: {columnName}, ordinal: {ordinal}, data type: {dataType}, maximum length: {maxLength}, precision: {precision}, scale: {scale}, nullable: {nullable}, identity: {identity}, default value: {defaultValue}, computed value: {computedValue}, computed value is stored: {stored}.</value>
<comment>Debug SqlServerEventId.ColumnFound string string int string int int int bool bool string string bool?</comment>
Expand Down Expand Up @@ -248,6 +252,10 @@
<value>Skipping foreign key with identity '{id}' on table '{tableName}', since the principal column '{principalColumnName}' on the foreign key's principal table, '{principalTableName}', was not found in the model.</value>
<comment>Warning SqlServerEventId.ForeignKeyPrincipalColumnMissingWarning string string string string</comment>
</data>
<data name="LogPrincipalTableInformationNotFound" xml:space="preserve">
<value>Skipping foreign key '{foreignKeyName}' on table '{tableName}' since principal table information is not available. This usually happens when the user doesn't have permission to read data about principal table.</value>
<comment>Warning SqlServerEventId.ForeignKeyReferencesUnknownPrincipalTableWarning string? string?</comment>
</data>
<data name="LogPrincipalTableNotInSelectionSet" xml:space="preserve">
<value>Skipping foreign key '{foreignKeyName}' on table '{tableName}' since principal table '{principalTableName}' was not found in the model. This usually happens when the principal table was not included in the selection set.</value>
<comment>Warning SqlServerEventId.ForeignKeyReferencesMissingPrincipalTableWarning string? string? string?</comment>
Expand All @@ -256,10 +264,6 @@
<value>Skipping foreign key '{foreignKeyName}' on table '{tableName}' since all of its columns reference themselves.</value>
<comment>Debug SqlServerEventId.ReflexiveConstraintIgnored string string</comment>
</data>
<data name="DuplicateForeignKeyConstraintIgnored" xml:space="preserve">
<value>Skipping foreign key '{foreignKeyName}' on table '{tableName}' since it is a duplicate of '{duplicateForeignKeyName}'.</value>
<comment>Warning SqlServerEventId.DuplicateForeignKeyConstraintIgnored string string string</comment>
</data>
<data name="LogSavepointsDisabledBecauseOfMARS" xml:space="preserve">
<value>Savepoints are disabled because Multiple Active Result Sets (MARS) is enabled. If 'SaveChanges' fails, then the transaction cannot be automatically rolled back to a known clean state. Instead, the transaction should be rolled back by the application before retrying 'SaveChanges'. See https://go.microsoft.com/fwlink/?linkid=2149338 for more information. To identify the code which triggers this warning, call 'ConfigureWarnings(w =&gt; w.Throw(SqlServerEventId.SavepointsDisabledBecauseOfMARS))'.</value>
<comment>Warning SqlServerEventId.SavepointsDisabledBecauseOfMARS</comment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ private void GetColumns(
{
commandText += @",
[c].[generated_always_type]";
}
}

commandText += @"FROM
(
Expand Down Expand Up @@ -1196,7 +1196,7 @@ FROM [sys].[foreign_keys] AS [f]
.GroupBy(
c => (Name: c.GetValueOrDefault<string>("name"),
PrincipalTableSchema: c.GetValueOrDefault<string>("principal_table_schema"),
PrincipalTableName: c.GetFieldValue<string>("principal_table_name"),
PrincipalTableName: c.GetValueOrDefault<string>("principal_table_name"),
OnDeleteAction: c.GetValueOrDefault<string>("delete_referential_action_desc")));

foreach (var foreignKeyGroup in foreignKeyGroups)
Expand All @@ -1206,9 +1206,18 @@ FROM [sys].[foreign_keys] AS [f]
var principalTableName = foreignKeyGroup.Key.PrincipalTableName;
var onDeleteAction = foreignKeyGroup.Key.OnDeleteAction;

if (principalTableName == null)
{
_logger.ForeignKeyReferencesUnknownPrincipalTableWarning(
fkName,
DisplayName(table.Schema, table.Name));

continue;
}

_logger.ForeignKeyFound(
fkName!,
DisplayName(table.Schema, table.Name!),
DisplayName(table.Schema, table.Name),
DisplayName(principalTableSchema, principalTableName),
onDeleteAction!);

Expand All @@ -1217,13 +1226,13 @@ FROM [sys].[foreign_keys] AS [f]
&& t.Name == principalTableName)
?? tables.FirstOrDefault(
t => t.Schema?.Equals(principalTableSchema, StringComparison.OrdinalIgnoreCase) == true
&& t.Name!.Equals(principalTableName, StringComparison.OrdinalIgnoreCase));
&& t.Name.Equals(principalTableName, StringComparison.OrdinalIgnoreCase));

if (principalTable == null)
{
_logger.ForeignKeyReferencesMissingPrincipalTableWarning(
fkName,
DisplayName(table.Schema, table.Name!),
DisplayName(table.Schema, table.Name),
DisplayName(principalTableSchema, principalTableName));

continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2422,7 +2422,7 @@ CONSTRAINT MYFK3 FOREIGN KEY (ForeignKeyId) REFERENCES OtherPrincipalTable(Id),
Fixture.ListLoggerFactory.Log, t => t.Id == SqlServerEventId.DuplicateForeignKeyConstraintIgnored);
Assert.Equal(LogLevel.Warning, level);
Assert.Equal(
SqlServerResources.DuplicateForeignKeyConstraintIgnored(new TestLogger<SqlServerLoggingDefinitions>())
SqlServerResources.LogDuplicateForeignKeyConstraintIgnored(new TestLogger<SqlServerLoggingDefinitions>())
.GenerateMessage("MYFK2", "dbo.DependentTable", "MYFK1"), message);
var table = dbModel.Tables.Single(t => t.Name == "DependentTable");
Expand Down

0 comments on commit 85e064c

Please sign in to comment.