Skip to content

Commit

Permalink
Resiliency to columns with missing types in SQL Server scaffolding
Browse files Browse the repository at this point in the history 
* Load columns even if they have no type, warn and skip.
* Skip indexes/constraints which reference unknown columns.

Fixes #25729
  • Loading branch information
@roji
roji committed Aug 27, 2021
1 parent feaf6ce commit 1585e7f
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 50 deletions.
8 changes: 8 additions & 0 deletions src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ public class SqlServerLoggingDefinitions : RelationalLoggingDefinitions
/// </summary>
public EventDefinitionBase? LogByteIdentityColumn;

/// <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? LogColumnWithoutType;

/// <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
7 changes: 7 additions & 0 deletions src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ private enum Id
ForeignKeyPrincipalColumnMissingWarning,
ReflexiveConstraintIgnored,
DuplicateForeignKeyConstraintIgnored,
ColumnWithoutTypeWarning
}

private static readonly string _validationPrefix = DbLoggerCategory.Model.Validation.Name + ".";
Expand Down Expand Up @@ -237,5 +238,11 @@ private static EventId MakeScaffoldingId(Id id)
/// This event is in the <see cref="DbLoggerCategory.Scaffolding" /> category.
/// </summary>
public static readonly EventId DuplicateForeignKeyConstraintIgnored = MakeScaffoldingId(Id.DuplicateForeignKeyConstraintIgnored);

/// <summary>
/// A column was skipped because its database type could not be found.
/// This event is in the <see cref="DbLoggerCategory.Scaffolding" /> category.
/// </summary>
public static readonly EventId ColumnWithoutTypeWarning = MakeScaffoldingId(Id.ColumnWithoutTypeWarning);
}
}
21 changes: 21 additions & 0 deletions src/EFCore.SqlServer/Extensions/Internal/SqlServerLoggerExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,27 @@ public static void MissingTableWarning(
// 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 ColumnWithoutTypeWarning(
this IDiagnosticsLogger<DbLoggerCategory.Scaffolding> diagnostics,
string tableName,
string columnName)
{
var definition = SqlServerResources.LogColumnWithoutType(diagnostics);

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

// 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
25 changes: 25 additions & 0 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs
View file

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

4 changes: 4 additions & 0 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@
<value>The property '{property}' on entity type '{entityType}' is of type 'byte', but is set up to use a SQL Server identity column; this requires that values starting at 255 and counting down will be used for temporary key values. A temporary key value is needed for every entity inserted in a single call to 'SaveChanges'. Care must be taken that these values do not collide with real key values.</value>
<comment>Warning SqlServerEventId.ByteIdentityColumnWarning string string</comment>
</data>
<data name="LogColumnWithoutType" xml:space="preserve">
<value>A database type for column '{columnName}' on table '{tableName}' could not be found, the column will be skipped.</value>
<comment>Warning SqlServerEventId.ColumnWithoutTypeWarning string string</comment>
</data>
<data name="LogConflictingValueGenerationStrategies" xml:space="preserve">
<value>Both the SqlServerValueGenerationStrategy '{generationStrategy}' and '{otherGenerationStrategy}' have been set on property '{propertyName}' on entity type '{entityName}'. Configuring two strategies is usually unintentional and will likely result in a database error.</value>
<comment>Warning SqlServerEventId.ConflictingValueGenerationStrategiesWarning string string string string</comment>
Expand Down
145 changes: 95 additions & 50 deletions src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Data;
using System.Data.Common;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq;
using System.Text;
Expand All @@ -19,6 +20,7 @@
using Microsoft.EntityFrameworkCore.SqlServer.Internal;
using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Utilities;
using Microsoft.Extensions.Logging;

namespace Microsoft.EntityFrameworkCore.SqlServer.Scaffolding.Internal
{
Expand Down Expand Up @@ -711,7 +713,7 @@ UNION ALL
commandText += @"
) o
JOIN [sys].[columns] AS [c] ON [o].[object_id] = [c].[object_id]
JOIN [sys].[types] AS [tp] ON [c].[user_type_id] = [tp].[user_type_id]
LEFT JOIN [sys].[types] AS [tp] ON [c].[user_type_id] = [tp].[user_type_id]
LEFT JOIN [sys].[extended_properties] AS [e] ON [e].[major_id] = [o].[object_id] AND [e].[minor_id] = [c].[column_id] AND [e].[class] = 1 AND [e].[name] = 'MS_Description'
LEFT JOIN [sys].[computed_columns] AS [cc] ON [c].[object_id] = [cc].[object_id] AND [c].[column_id] = [cc].[column_id]
LEFT JOIN [sys].[default_constraints] AS [dc] ON [c].[object_id] = [dc].[parent_object_id] AND [c].[column_id] = [dc].[parent_column_id]";
Expand Down Expand Up @@ -744,7 +746,7 @@ UNION ALL
var columnName = dataRecord.GetFieldValue<string>("column_name");
var ordinal = dataRecord.GetFieldValue<int>("ordinal");
var dataTypeSchemaName = dataRecord.GetValueOrDefault<string>("type_schema");
var dataTypeName = dataRecord.GetFieldValue<string>("type_name");
var dataTypeName = dataRecord.GetValueOrDefault<string>("type_name");
var maxLength = dataRecord.GetValueOrDefault<int>("max_length");
var precision = dataRecord.GetValueOrDefault<int>("precision");
var scale = dataRecord.GetValueOrDefault<int>("scale");
Expand All @@ -758,6 +760,12 @@ UNION ALL
var isSparse = dataRecord.GetValueOrDefault<bool>("is_sparse");
var generatedAlwaysType = SupportsTemporalTable() ? dataRecord.GetValueOrDefault<byte>("generated_always_type") : 0;

if (dataTypeName is null)
{
_logger.ColumnWithoutTypeWarning(DisplayName(tableSchema, tableName), columnName);
continue;
}

_logger.ColumnFound(
DisplayName(tableSchema, tableName),
columnName,
Expand Down Expand Up @@ -991,13 +999,62 @@ FROM [sys].[indexes] i
TypeDesc: ddr.GetValueOrDefault<string>("type_desc")))
.ToArray();

Check.DebugAssert(primaryKeyGroups.Length == 0 || primaryKeyGroups.Length == 1, "Multiple primary keys found");

if (primaryKeyGroups.Length == 1)
{
var primaryKeyGroup = primaryKeyGroups[0];
if (TryGetPrimaryKey(primaryKeyGroups[0], out var primaryKey))
{
_logger.PrimaryKeyFound(primaryKey.Name!, DisplayName(tableSchema, tableName));
table.PrimaryKey = primaryKey;
}
}

_logger.PrimaryKeyFound(primaryKeyGroup.Key.Name, DisplayName(tableSchema, tableName));
var uniqueConstraintGroups = tableIndexGroup
.Where(ddr => ddr.GetValueOrDefault<bool>("is_unique_constraint"))
.GroupBy(
ddr =>
(Name: ddr.GetValueOrDefault<string>("index_name"),
TypeDesc: ddr.GetValueOrDefault<string>("type_desc")))
.ToArray();

var primaryKey = new DatabasePrimaryKey { Table = table, Name = primaryKeyGroup.Key.Name };
foreach (var uniqueConstraintGroup in uniqueConstraintGroups)
{
if (TryGetUniqueConstraint(uniqueConstraintGroup, out var uniqueConstraint))
{
_logger.UniqueConstraintFound(uniqueConstraintGroup.Key.Name!, DisplayName(tableSchema, tableName));
table.UniqueConstraints.Add(uniqueConstraint);
}
}

var indexGroups = tableIndexGroup
.Where(
ddr => !ddr.GetValueOrDefault<bool>("is_primary_key")
&& !ddr.GetValueOrDefault<bool>("is_unique_constraint"))
.GroupBy(
ddr =>
(Name: ddr.GetValueOrDefault<string>("index_name"),
TypeDesc: ddr.GetValueOrDefault<string>("type_desc"),
IsUnique: ddr.GetValueOrDefault<bool>("is_unique"),
HasFilter: ddr.GetValueOrDefault<bool>("has_filter"),
FilterDefinition: ddr.GetValueOrDefault<string>("filter_definition"),
FillFactor: ddr.GetValueOrDefault<byte>("fill_factor")))
.ToArray();

foreach (var indexGroup in indexGroups)
{
if (TryGetIndex(indexGroup, out var index))
{
_logger.IndexFound(indexGroup.Key.Name!, DisplayName(tableSchema, tableName), indexGroup.Key.IsUnique);
table.Indexes.Add(index);
}
}

bool TryGetPrimaryKey(
IGrouping<(string Name, string? TypeDesc), DbDataRecord> primaryKeyGroup,
[NotNullWhen(true)] out DatabasePrimaryKey? primaryKey)
{
primaryKey = new DatabasePrimaryKey { Table = table, Name = primaryKeyGroup.Key.Name };

if (primaryKeyGroup.Key.TypeDesc == "NONCLUSTERED")
{
Expand All @@ -1008,29 +1065,25 @@ FROM [sys].[indexes] i
{
var columnName = dataRecord.GetValueOrDefault<string>("column_name");
var column = table.Columns.FirstOrDefault(c => c.Name == columnName)
?? table.Columns.FirstOrDefault(
c => c.Name!.Equals(columnName, StringComparison.OrdinalIgnoreCase));
Check.DebugAssert(column != null, "column is null.");
?? table.Columns.FirstOrDefault(
c => c.Name!.Equals(columnName, StringComparison.OrdinalIgnoreCase));

if (column is null)
{
return false;
}

primaryKey.Columns.Add(column);
}

table.PrimaryKey = primaryKey;
return true;
}

var uniqueConstraintGroups = tableIndexGroup
.Where(ddr => ddr.GetValueOrDefault<bool>("is_unique_constraint"))
.GroupBy(
ddr =>
(Name: ddr.GetValueOrDefault<string>("index_name"),
TypeDesc: ddr.GetValueOrDefault<string>("type_desc")))
.ToArray();

foreach (var uniqueConstraintGroup in uniqueConstraintGroups)
bool TryGetUniqueConstraint(
IGrouping<(string? Name, string? TypeDesc), DbDataRecord> uniqueConstraintGroup,
[NotNullWhen(true)] out DatabaseUniqueConstraint? uniqueConstraint)
{
_logger.UniqueConstraintFound(uniqueConstraintGroup.Key.Name!, DisplayName(tableSchema, tableName));

var uniqueConstraint = new DatabaseUniqueConstraint { Table = table, Name = uniqueConstraintGroup.Key.Name };
uniqueConstraint = new DatabaseUniqueConstraint { Table = table, Name = uniqueConstraintGroup.Key.Name };

if (uniqueConstraintGroup.Key.TypeDesc == "CLUSTERED")
{
Expand All @@ -1041,35 +1094,26 @@ FROM [sys].[indexes] i
{
var columnName = dataRecord.GetValueOrDefault<string>("column_name");
var column = table.Columns.FirstOrDefault(c => c.Name == columnName)
?? table.Columns.FirstOrDefault(
c => c.Name!.Equals(columnName, StringComparison.OrdinalIgnoreCase));
Check.DebugAssert(column != null, "column is null.");
?? table.Columns.FirstOrDefault(
c => c.Name!.Equals(columnName, StringComparison.OrdinalIgnoreCase));

if (column is null)
{
return false;
}

uniqueConstraint.Columns.Add(column);
}

table.UniqueConstraints.Add(uniqueConstraint);
return true;
}

var indexGroups = tableIndexGroup
.Where(
ddr => !ddr.GetValueOrDefault<bool>("is_primary_key")
&& !ddr.GetValueOrDefault<bool>("is_unique_constraint"))
.GroupBy(
ddr =>
(Name: ddr.GetValueOrDefault<string>("index_name"),
TypeDesc: ddr.GetValueOrDefault<string>("type_desc"),
IsUnique: ddr.GetValueOrDefault<bool>("is_unique"),
HasFilter: ddr.GetValueOrDefault<bool>("has_filter"),
FilterDefinition: ddr.GetValueOrDefault<string>("filter_definition"),
FillFactor: ddr.GetValueOrDefault<byte>("fill_factor")))
.ToArray();

foreach (var indexGroup in indexGroups)
bool TryGetIndex(
IGrouping<(string? Name, string? TypeDesc, bool IsUnique, bool HasFilter, string? FilterDefinition, byte FillFactor),
DbDataRecord> indexGroup,
[NotNullWhen(true)] out DatabaseIndex? index)
{
_logger.IndexFound(indexGroup.Key.Name!, DisplayName(tableSchema, tableName), indexGroup.Key.IsUnique);

var index = new DatabaseIndex
index = new DatabaseIndex
{
Table = table,
Name = indexGroup.Key.Name,
Expand Down Expand Up @@ -1098,17 +1142,18 @@ FROM [sys].[indexes] i
}

var column = table.Columns.FirstOrDefault(c => c.Name == columnName)
?? table.Columns.FirstOrDefault(
c => c.Name!.Equals(columnName, StringComparison.OrdinalIgnoreCase));
Check.DebugAssert(column != null, "column is null.");
?? table.Columns.FirstOrDefault(
c => c.Name!.Equals(columnName, StringComparison.OrdinalIgnoreCase));

if (column is null)
{
return false;
}

index.Columns.Add(column);
}

if (index.Columns.Count > 0)
{
table.Indexes.Add(index);
}
return index.Columns.Count > 0;
}
}
}
Expand Down

0 comments on commit 1585e7f

Please sign in to comment.