Allow to set provider value comparer when configuring value conversion

Use client value comparer as provider value comparer if possible The client value comparer will not be used if the model type is a nullable value type Fix some API issues Remove some unused code Fixes #27738 Fixes #27850 Fixes #27791 Part of #27588
dotnet · May 3, 2022 · a61d598 · a61d598
1 parent 9a7e792
commit a61d598
Show file tree

Hide file tree

Showing 49 changed files with 1,300 additions and 335 deletions.
diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs
@@ -678,6 +678,15 @@ private void Create(
                     property.DeclaringEntityType.ShortName(), property.Name, nameof(PropertyBuilder.HasConversion)));
         }
 
+        var providerValueComparerType = (Type?)property[CoreAnnotationNames.ProviderValueComparerType];
+        if (providerValueComparerType == null
+            && property[CoreAnnotationNames.ProviderValueComparer] != null)
+        {
+            throw new InvalidOperationException(
+                DesignStrings.CompiledModelValueComparer(
+                    property.DeclaringEntityType.ShortName(), property.Name, nameof(PropertyBuilder.HasConversion)));
+        }
+
         var valueConverterType = (Type?)property[CoreAnnotationNames.ValueConverterType];
         if (valueConverterType == null
             && property.GetValueConverter() != null)
@@ -809,6 +818,16 @@ private void Create(
                 .Append("()");
         }
 
+        if (providerValueComparerType != null)
+        {
+            AddNamespace(providerValueComparerType, parameters.Namespaces);
+
+            mainBuilder.AppendLine(",")
+                .Append("providerValueComparer: new ")
+                .Append(_code.Reference(providerValueComparerType))
+                .Append("()");
+        }
+
         mainBuilder
             .AppendLine(");")
             .DecrementIndent();

diff --git a/src/EFCore.Relational/Metadata/IColumn.cs b/src/EFCore.Relational/Metadata/IColumn.cs
@@ -147,6 +147,14 @@ public virtual string? Collation
         => PropertyMappings.First().Property
             .GetCollation(StoreObjectIdentifier.Table(Table.Name, Table.Schema));
 
+    /// <summary>
+    ///     Gets the <see cref="ValueComparer" /> for this column.
+    /// </summary>
+    /// <returns>The comparer.</returns>
+    public virtual ValueComparer ProviderValueComparer
+        => PropertyMappings.First().Property
+            .GetProviderValueComparer();
+
     /// <summary>
     ///     Returns the property mapping for the given entity type.
     /// </summary>

diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
@@ -2062,7 +2062,7 @@ protected virtual void DiffData(
 
                     var sourceValue = sourceColumnModification.OriginalValue;
                     var targetValue = targetColumnModification.Value;
-                    var comparer = targetMapping.TypeMapping.ProviderValueComparer;
+                    var comparer = targetColumn.ProviderValueComparer;
                     if (sourceColumn.ProviderClrType == targetColumn.ProviderClrType
                         && comparer.Equals(sourceValue, targetValue))
                     {

diff --git a/src/EFCore.Relational/Storage/RelationalGeometryTypeMapping.cs b/src/EFCore.Relational/Storage/RelationalGeometryTypeMapping.cs
@@ -27,7 +27,6 @@ protected RelationalGeometryTypeMapping(
         : base(CreateRelationalTypeMappingParameters(storeType))
     {
         SpatialConverter = converter;
-        SetProviderValueComparer();
     }
 
     /// <summary>
@@ -38,25 +37,19 @@ protected RelationalGeometryTypeMapping(
     protected RelationalGeometryTypeMapping(
         RelationalTypeMappingParameters parameters,
         ValueConverter<TGeometry, TProvider>? converter)
-        : base(parameters)
+        : base(parameters.WithCoreParameters(parameters.CoreParameters with
+            {
+                ProviderValueComparer = parameters.CoreParameters.ProviderValueComparer
+                    ?? CreateProviderValueComparer(parameters.CoreParameters.Converter?.ProviderClrType ?? parameters.CoreParameters.ClrType)
+            }))
     {
         SpatialConverter = converter;
-        SetProviderValueComparer();
     }
 
-    private void SetProviderValueComparer()
-    {
-        var providerType = Converter?.ProviderClrType ?? ClrType;
-        if (providerType.IsAssignableTo(typeof(TGeometry)))
-        {
-            ProviderValueComparer = (ValueComparer)Activator.CreateInstance(typeof(GeometryValueComparer<>).MakeGenericType(providerType))!;
-        }
-    }
-
-    /// <summary>
-    ///     The underlying Geometry converter.
-    /// </summary>
-    protected virtual ValueConverter<TGeometry, TProvider>? SpatialConverter { get; }
+    private static ValueComparer? CreateProviderValueComparer(Type providerType)
+        => providerType.IsAssignableTo(typeof(TGeometry))
+            ? (ValueComparer)Activator.CreateInstance(typeof(GeometryValueComparer<>).MakeGenericType(providerType))!
+            : null;
 
     private static RelationalTypeMappingParameters CreateRelationalTypeMappingParameters(string storeType)
     {
@@ -67,10 +60,16 @@ private static RelationalTypeMappingParameters CreateRelationalTypeMappingParame
                 typeof(TGeometry),
                 null,
                 comparer,
-                comparer),
+                comparer,
+                CreateProviderValueComparer(typeof(TGeometry))),
             storeType);
     }
 
+    /// <summary>
+    ///     The underlying Geometry converter.
+    /// </summary>
+    protected virtual ValueConverter<TGeometry, TProvider>? SpatialConverter { get; }
+
     /// <summary>
     ///     Creates a <see cref="DbParameter" /> with the appropriate type information configured.
     /// </summary>

diff --git a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs
@@ -108,6 +108,24 @@ public RelationalTypeMappingParameters(
         /// </summary>
         public StoreTypePostfix StoreTypePostfix { get; }
 
+        /// <summary>
+        ///     Creates a new <see cref="RelationalTypeMappingParameters" /> parameter object with the given
+        ///     core parameters.
+        /// </summary>
+        /// <param name="coreParameters">Parameters for the <see cref="CoreTypeMapping" /> base class.</param>
+        /// <returns>The new parameter object.</returns>
+        public RelationalTypeMappingParameters WithCoreParameters(in CoreTypeMappingParameters coreParameters)
+            => new(
+                coreParameters,
+                StoreType,
+                StoreTypePostfix,
+                DbType,
+                Unicode,
+                Size,
+                FixedLength,
+                Precision,
+                Scale);
+
         /// <summary>
         ///     Creates a new <see cref="RelationalTypeMappingParameters" /> parameter object with the given
         ///     mapping info.
@@ -261,8 +279,6 @@ protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters p
             => this;
     }
 
-    private ValueComparer? _providerValueComparer;
-
     /// <summary>
     ///     Initializes a new instance of the <see cref="RelationalTypeMapping" /> class.
     /// </summary>
@@ -380,19 +396,6 @@ public virtual bool IsFixedLength
     protected virtual string SqlLiteralFormatString
         => "{0}";
 
-    /// <summary>
-    ///     A <see cref="ValueComparer" /> for the provider CLR type values.
-    /// </summary>
-    public virtual ValueComparer ProviderValueComparer
-    {
-        get => NonCapturingLazyInitializer.EnsureInitialized(
-                   ref _providerValueComparer,
-                   this,
-                   static c => ValueComparer.CreateDefault(c.Converter?.ProviderClrType ?? c.ClrType, favorStructuralComparisons: true));
-
-        protected set => _providerValueComparer = value;
-    }
-
     /// <summary>
     ///     Creates a copy of this mapping.
     /// </summary>

diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs
@@ -605,7 +605,7 @@ private static bool IsModified(IReadOnlyList<IColumn> columns, IReadOnlyModifica
             var column = columns[columnIndex];
             object? originalValue = null;
             object? currentValue = null;
-            RelationalTypeMapping? typeMapping = null;
+            ValueComparer? providerValueComparer = null;
             for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++)
             {
                 var entry = command.Entries[entryIndex];
@@ -634,12 +634,12 @@ private static bool IsModified(IReadOnlyList<IColumn> columns, IReadOnlyModifica
                             break;
                     }
 
-                    typeMapping = columnMapping!.TypeMapping;
+                    providerValueComparer = property.GetProviderValueComparer();
                 }
             }
 
-            if (typeMapping != null
-                && !typeMapping.ProviderValueComparer.Equals(originalValue, currentValue))
+            if (providerValueComparer != null
+                && !providerValueComparer.Equals(originalValue, currentValue))
             {
                 return true;
             }

diff --git a/src/EFCore.Relational/Update/Internal/CompositeRowValueFactory.cs b/src/EFCore.Relational/Update/Internal/CompositeRowValueFactory.cs
@@ -150,7 +150,7 @@ public virtual bool TryCreateDependentKeyValue(IReadOnlyModificationCommand comm
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     protected static IEqualityComparer<object?[]> CreateEqualityComparer(IReadOnlyList<IColumn> columns)
-        => new CompositeCustomComparer(columns.Select(c => c.PropertyMappings.First().TypeMapping.ProviderValueComparer).ToList());
+        => new CompositeCustomComparer(columns.Select(c => c.ProviderValueComparer).ToList());
 
     private sealed class CompositeCustomComparer : IEqualityComparer<object?[]>
     {

diff --git a/src/EFCore.Relational/Update/Internal/RowForeignKeyValueFactory.cs b/src/EFCore.Relational/Update/Internal/RowForeignKeyValueFactory.cs
@@ -96,8 +96,7 @@ public abstract bool TryCreateDependentKeyValue(
     /// </summary>
     protected virtual IEqualityComparer<TKey> CreateKeyEqualityComparer(IColumn column)
 #pragma warning disable EF1001 // Internal EF Core API usage.
-        => NullableComparerAdapter<TKey>.Wrap(
-            column.PropertyMappings.First().TypeMapping.ProviderValueComparer);
+        => NullableComparerAdapter<TKey>.Wrap(column.ProviderValueComparer);
 #pragma warning restore EF1001 // Internal EF Core API usage.
 
     /// <summary>

diff --git a/src/EFCore.Relational/Update/Internal/SimpleRowIndexValueFactory.cs b/src/EFCore.Relational/Update/Internal/SimpleRowIndexValueFactory.cs
@@ -31,7 +31,7 @@ public SimpleRowIndexValueFactory(ITableIndex index)
         _column = index.Columns.Single();
         _columnAccessors = ((Column)_column).Accessors;
 #pragma warning disable EF1001 // Internal EF Core API usage.
-        EqualityComparer = NullableComparerAdapter<TKey>.Wrap(_column.PropertyMappings.First().TypeMapping.ProviderValueComparer);
+        EqualityComparer = NullableComparerAdapter<TKey>.Wrap(_column.ProviderValueComparer);
 #pragma warning restore EF1001 // Internal EF Core API usage.
     }
 

diff --git a/src/EFCore.Relational/Update/Internal/SimpleRowKeyValueFactory.cs b/src/EFCore.Relational/Update/Internal/SimpleRowKeyValueFactory.cs
@@ -31,7 +31,7 @@ public SimpleRowKeyValueFactory(IUniqueConstraint constraint)
         _constraint = constraint;
         _column = constraint.Columns.Single();
         _columnAccessors = ((Column)_column).Accessors;
-        EqualityComparer = new NoNullsCustomEqualityComparer(_column.PropertyMappings.First().TypeMapping.ProviderValueComparer);
+        EqualityComparer = new NoNullsCustomEqualityComparer(_column.ProviderValueComparer);
     }
 
     /// <summary>
@@ -139,14 +139,6 @@ private sealed class NoNullsCustomEqualityComparer : IEqualityComparer<TKey>
 
         public NoNullsCustomEqualityComparer(ValueComparer comparer)
         {
-            if (comparer.Type != typeof(TKey)
-                && comparer.Type == typeof(TKey).UnwrapNullableType())
-            {
-#pragma warning disable EF1001 // Internal EF Core API usage.
-                comparer = comparer.ToNonNullNullableComparer();
-#pragma warning restore EF1001 // Internal EF Core API usage.
-            }
-
             _equals = (Func<TKey?, TKey?, bool>)comparer.EqualsExpression.Compile();
             _hashCode = (Func<TKey, int>)comparer.HashCodeExpression.Compile();
         }

diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs
@@ -480,7 +480,7 @@ public void RecordValue(IColumnMapping mapping, IUpdateEntry entry)
                     break;
                 case EntityState.Added:
                     _currentValue = entry.GetCurrentProviderValue(property);
-                    _write = !mapping.TypeMapping.ProviderValueComparer.Equals(_originalValue, _currentValue);
+                    _write = !mapping.Column.ProviderValueComparer.Equals(_originalValue, _currentValue);
 
                     break;
                 case EntityState.Deleted:
@@ -503,7 +503,7 @@ public bool TryPropagate(IColumnMapping mapping, IUpdateEntry entry)
                 && (entry.EntityState == EntityState.Unchanged
                     || (entry.EntityState == EntityState.Modified && !entry.IsModified(property))
                     || (entry.EntityState == EntityState.Added
-                        && mapping.TypeMapping.ProviderValueComparer.Equals(_originalValue, entry.GetCurrentValue(property)))))
+                        && mapping.Column.ProviderValueComparer.Equals(_originalValue, entry.GetCurrentValue(property)))))
             {
                 if (property.GetAfterSaveBehavior() == PropertySaveBehavior.Save
                     || entry.EntityState == EntityState.Added)

diff --git a/src/EFCore/ChangeTracking/Internal/SimplePrincipalKeyValueFactory.cs b/src/EFCore/ChangeTracking/Internal/SimplePrincipalKeyValueFactory.cs
@@ -122,12 +122,6 @@ private sealed class NoNullsCustomEqualityComparer : IEqualityComparer<TKey>
 
         public NoNullsCustomEqualityComparer(ValueComparer comparer)
         {
-            if (comparer.Type != typeof(TKey)
-                && comparer.Type == typeof(TKey).UnwrapNullableType())
-            {
-                comparer = comparer.ToNonNullNullableComparer();
-            }
-
             _equals = (Func<TKey?, TKey?, bool>)comparer.EqualsExpression.Compile();
             _hashCode = (Func<TKey, int>)comparer.HashCodeExpression.Compile();
         }

diff --git a/src/EFCore/ChangeTracking/Internal/ValueComparerExtensions.cs b/src/EFCore/ChangeTracking/Internal/ValueComparerExtensions.cs
diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs
@@ -868,6 +868,24 @@ protected virtual void ValidateTypeMappings(
                 {
                     _ = property.GetCurrentValueComparer(); // Will throw if there is no way to compare
                 }
+
+                var providerComparer = property.GetProviderValueComparer();
+                if (providerComparer == null)
+                {
+                    continue;
+                }
+
+                var typeMapping = property.GetTypeMapping();
+                var actualProviderClrType = (typeMapping.Converter?.ProviderClrType ?? typeMapping.ClrType).UnwrapNullableType();
+
+                if (providerComparer.Type.UnwrapNullableType() != actualProviderClrType)
+                {
+                    throw new InvalidOperationException(CoreStrings.ComparerPropertyMismatch(
+                        providerComparer.Type.ShortDisplayName(),
+                        property.DeclaringEntityType.DisplayName(),
+                        property.Name,
+                        actualProviderClrType.ShortDisplayName()));
+                }
             }
         }
     }