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

Resiliency to columns with missing types in SQL Server scaffolding #25748

Merged
merged 1 commit into from
Aug 28, 2021
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
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
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);
}
}
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

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
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
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(
smitpatel marked this conversation as resolved.
Show resolved Hide resolved
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