diff --git a/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs b/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs index 512a28960b6..2cea968ff22 100644 --- a/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs +++ b/src/EFCore.SqlServer/Diagnostics/Internal/SqlServerLoggingDefinitions.cs @@ -13,6 +13,14 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Diagnostics.Internal /// public class SqlServerLoggingDefinitions : RelationalLoggingDefinitions { + /// + /// 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. + /// + public EventDefinitionBase LogDecimalTypeKey; + /// /// 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 diff --git a/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs b/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs index a1a816e8b6b..4699da6543b 100644 --- a/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs +++ b/src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs @@ -27,6 +27,7 @@ private enum Id DecimalTypeDefaultWarning = CoreEventId.ProviderBaseId, ByteIdentityColumnWarning, ConflictingValueGenerationStrategiesWarning, + DecimalTypeKeyWarning, // Scaffolding events ColumnFound = CoreEventId.ProviderDesignBaseId, @@ -64,6 +65,19 @@ private enum Id private static readonly string _validationPrefix = DbLoggerCategory.Model.Validation.Name + "."; private static EventId MakeValidationId(Id id) => new EventId((int)id, _validationPrefix + id); + /// + /// + /// Decimal column is part of the key. + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId DecimalTypeKeyWarning = MakeValidationId(Id.DecimalTypeKeyWarning); + /// /// /// No explicit type for a decimal column. diff --git a/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs b/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs index b7f2013b74d..a7c1f63adaa 100644 --- a/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs +++ b/src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs @@ -16,6 +16,43 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Internal /// public static class SqlServerLoggerExtensions { + /// + /// 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. + /// + public static void DecimalTypeKeyWarning( + [NotNull] this IDiagnosticsLogger diagnostics, + [NotNull] IProperty property) + { + var definition = SqlServerResources.LogDecimalTypeKey(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics, property.Name, property.DeclaringEntityType.DisplayName()); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new PropertyEventData( + definition, + DecimalTypeKeyWarning, + property); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string DecimalTypeKeyWarning(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + var p = (PropertyEventData)payload; + return d.GenerateMessage( + p.Property.Name, + p.Property.DeclaringEntityType.DisplayName()); + } + /// /// 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 diff --git a/src/EFCore.SqlServer/Internal/SqlServerModelValidator.cs b/src/EFCore.SqlServer/Internal/SqlServerModelValidator.cs index e4b0159b3f7..d627d8150ce 100644 --- a/src/EFCore.SqlServer/Internal/SqlServerModelValidator.cs +++ b/src/EFCore.SqlServer/Internal/SqlServerModelValidator.cs @@ -55,7 +55,7 @@ public override void Validate(IModel model, IDiagnosticsLogger - protected virtual void ValidateDefaultDecimalMapping( + protected virtual void ValidateDecimalColumns( [NotNull] IModel model, [NotNull] IDiagnosticsLogger logger) { - foreach (var property in model.GetEntityTypes() + foreach (IConventionProperty property in model.GetEntityTypes() .SelectMany(t => t.GetDeclaredProperties()) - .Where( - p => p.ClrType.UnwrapNullableType() == typeof(decimal) + .Where(p => p.ClrType.UnwrapNullableType() == typeof(decimal) && !p.IsForeignKey())) { - var valueConverterConfigurationSource = (property as IConventionProperty)?.GetValueConverterConfigurationSource(); + var valueConverterConfigurationSource = property.GetValueConverterConfigurationSource(); var valueConverterProviderType = property.GetValueConverter()?.ProviderClrType; if (!ConfigurationSource.Convention.Overrides(valueConverterConfigurationSource) && typeof(decimal) != valueConverterProviderType) @@ -83,15 +82,21 @@ protected virtual void ValidateDefaultDecimalMapping( continue; } - var columnTypeConfigurationSource = (property as IConventionProperty)?.GetColumnTypeConfigurationSource(); - var typeMappingConfigurationSource = (property as IConventionProperty)?.GetTypeMappingConfigurationSource(); - if ((columnTypeConfigurationSource == null - && ConfigurationSource.Convention.Overrides(typeMappingConfigurationSource)) + var columnTypeConfigurationSource = property.GetColumnTypeConfigurationSource(); + if (((columnTypeConfigurationSource == null + && ConfigurationSource.Convention.Overrides(property.GetTypeMappingConfigurationSource())) || (columnTypeConfigurationSource != null && ConfigurationSource.Convention.Overrides(columnTypeConfigurationSource))) + && (ConfigurationSource.Convention.Overrides(property.GetPrecisionConfigurationSource()) + || ConfigurationSource.Convention.Overrides(property.GetScaleConfigurationSource()))) { logger.DecimalTypeDefaultWarning(property); } + + if (property.IsKey()) + { + logger.DecimalTypeKeyWarning(property); + } } } diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 9e40458aac1..99d0cba0acf 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -258,7 +258,7 @@ private static readonly ResourceManager _resourceManager = new ResourceManager("Microsoft.EntityFrameworkCore.SqlServer.Properties.SqlServerStrings", typeof(SqlServerResources).Assembly); /// - /// No type was specified for the decimal column '{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 using 'HasColumnType()' or specify a ValueConverter. + /// No 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 using 'HasColumnType()', specify precision and scale using 'HasPrecision()' or configure a value converter using 'HasConversion()'. /// public static EventDefinition LogDefaultDecimalTypeColumn([NotNull] IDiagnosticsLogger logger) { @@ -658,5 +658,29 @@ public static EventDefinition LogConflictingValu return (EventDefinition)definition; } + + /// + /// The decimal property '{property}' is part of a key on entity type '{entityType}'. If the configured precision and scale don't match the column type in the database this will cause values to be silently truncated if they do not fit in the default precision and scale. Consider using a different property as the key or make sure that the database column type matches the model configuration and enable decimal rounding warnings using 'SET NUMERIC_ROUNDABORT ON'. + /// + public static EventDefinition LogDecimalTypeKey([NotNull] IDiagnosticsLogger logger) + { + var definition = ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogDecimalTypeKey; + if (definition == null) + { + definition = LazyInitializer.EnsureInitialized( + ref ((Diagnostics.Internal.SqlServerLoggingDefinitions)logger.Definitions).LogDecimalTypeKey, + () => new EventDefinition( + logger.Options, + SqlServerEventId.DecimalTypeKeyWarning, + LogLevel.Warning, + "SqlServerEventId.DecimalTypeKeyWarning", + level => LoggerMessage.Define( + level, + SqlServerEventId.DecimalTypeKeyWarning, + _resourceManager.GetString("LogDecimalTypeKey")))); + } + + return (EventDefinition)definition; + } } } diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index 6572720d21f..08c79309f9c 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -142,7 +142,7 @@ An exception has been raised that is likely due to a transient failure. Consider enabling transient error resiliency by adding 'EnableRetryOnFailure()' to the 'UseSqlServer' call. - No type was specified for the decimal column '{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 using 'HasColumnType()' or specify a ValueConverter. + No 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 using 'HasColumnType()', specify precision and scale using 'HasPrecision()' or configure a value converter using 'HasConversion()'. Warning SqlServerEventId.DecimalTypeDefaultWarning string string @@ -269,4 +269,8 @@ The indexes {index1} on '{entityType1}' and {index2} on '{entityType2}' are both mapped to '{table}.{indexName}' but with different fill factor configuration. + + The decimal property '{property}' is part of a key on entity type '{entityType}'. If the configured precision and scale don't match the column type in the database this will cause values to be silently truncated if they do not fit in the default precision and scale. Consider using a different property as the key or make sure that the database column type matches the model configuration and enable decimal rounding warnings using 'SET NUMERIC_ROUNDABORT ON'. + Warning SqlServerEventId.DecimalTypeKeyWarning string string + \ No newline at end of file diff --git a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs index e2ff7ef1d59..11b0fcfc7ad 100644 --- a/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs +++ b/test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs @@ -347,6 +347,19 @@ public virtual void Detects_incompatible_non_clustered_shared_key() modelBuilder.Model); } + [ConditionalFact] + public virtual void Detects_decimal_keys() + { + var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity() + .Property("Price").HasPrecision(18, 2); + modelBuilder.Entity().HasKey("Price"); + + VerifyWarning( + SqlServerResources.LogDecimalTypeKey(new TestLogger()) + .GenerateMessage("Price", nameof(Animal)), modelBuilder.Model); + } + [ConditionalFact] public virtual void Detects_default_decimal_mapping() { @@ -369,6 +382,18 @@ public virtual void Detects_default_nullable_decimal_mapping() .GenerateMessage("Price", nameof(Animal)), modelBuilder.Model); } + [ConditionalFact] + public virtual void Does_not_warn_if_decimal_column_has_precision_and_scale() + { + var modelBuilder = CreateConventionalModelBuilder(); + modelBuilder.Entity() + .Property("Price").HasPrecision(18, 2); + + VerifyLogDoesNotContain( + SqlServerResources.LogDefaultDecimalTypeColumn(new TestLogger()) + .GenerateMessage("Price", nameof(Animal)), modelBuilder.Model); + } + [ConditionalFact] public virtual void Does_not_warn_if_default_decimal_mapping_has_non_decimal_to_decimal_value_converter() {