Cleanup NaN handling in value comparers (#22242)

Follow-up to #22168
dotnet · Aug 27, 2020 · 87d75d4 · 87d75d4
1 parent a9b0ad7
commit 87d75d4
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 63 deletions.
diff --git a/src/EFCore.Relational/Storage/DoubleTypeMapping.cs b/src/EFCore.Relational/Storage/DoubleTypeMapping.cs
@@ -28,10 +28,7 @@ public class DoubleTypeMapping : RelationalTypeMapping
         public DoubleTypeMapping(
             [NotNull] string storeType,
             DbType? dbType = null)
-            : base(
-                new RelationalTypeMappingParameters(
-                    new CoreTypeMappingParameters(typeof(double), comparer: new DoubleValueComparer()),
-                    storeType, StoreTypePostfix.None, dbType, unicode: false, size: null, fixedLength: false, precision: null, scale: null))
+            : base(storeType, typeof(double), dbType)
         {
         }
 

diff --git a/src/EFCore.Relational/Storage/FloatTypeMapping.cs b/src/EFCore.Relational/Storage/FloatTypeMapping.cs
@@ -28,10 +28,7 @@ public class FloatTypeMapping : RelationalTypeMapping
         public FloatTypeMapping(
             [NotNull] string storeType,
             DbType? dbType = null)
-            : base(
-                new RelationalTypeMappingParameters(
-                    new CoreTypeMappingParameters(typeof(float), comparer: new FloatValueComparer()),
-                    storeType, StoreTypePostfix.None, dbType, unicode: false, size: null, fixedLength: false, precision: null, scale: null))
+            : base(storeType, typeof(float), dbType)
         {
         }
 

diff --git a/src/EFCore/ChangeTracking/DoubleValueComparer.cs b/src/EFCore/ChangeTracking/DoubleValueComparer.cs
diff --git a/src/EFCore/ChangeTracking/FloatValueComparer.cs b/src/EFCore/ChangeTracking/FloatValueComparer.cs
diff --git a/src/EFCore/ChangeTracking/ValueComparer.cs b/src/EFCore/ChangeTracking/ValueComparer.cs
@@ -9,6 +9,7 @@
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Query;
 using Microsoft.EntityFrameworkCore.Utilities;
+using MethodInfoExtensions = Microsoft.EntityFrameworkCore.Internal.MethodInfoExtensions;
 
 namespace Microsoft.EntityFrameworkCore.ChangeTracking
 {
@@ -28,6 +29,12 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking
     /// </summary>
     public abstract class ValueComparer : IEqualityComparer
     {
+        private protected static readonly MethodInfo _doubleEqualsMethodInfo
+            = typeof(double).GetRuntimeMethod(nameof(double.Equals), new[] { typeof(double) });
+
+        private protected static readonly MethodInfo _floatEqualsMethodInfo
+            = typeof(float).GetRuntimeMethod(nameof(float.Equals), new[] { typeof(float) });
+
         internal static readonly MethodInfo ArrayCopyMethod
             = typeof(Array).GetMethods()
                 .Single(
@@ -197,8 +204,19 @@ public static ValueComparer CreateDefault([NotNull] Type type, bool favorStructu
         {
             var nonNullabletype = type.UnwrapNullableType();
 
-            var comparerType =
-                nonNullabletype.IsNumeric()
+            // The equality operator returns false for NaNs, but the Equals methods returns true
+            if (nonNullabletype == typeof(double))
+            {
+                return new DefaultDoubleValueComparer(favorStructuralComparisons);
+            }
+
+            if (nonNullabletype == typeof(float))
+            {
+                return new DefaultFloatValueComparer(favorStructuralComparisons);
+            }
+
+            var comparerType = nonNullabletype.IsInteger()
+                || nonNullabletype == typeof(decimal)
                 || nonNullabletype == typeof(bool)
                 || nonNullabletype == typeof(string)
                 || nonNullabletype == typeof(DateTime)
@@ -213,7 +231,7 @@ public static ValueComparer CreateDefault([NotNull] Type type, bool favorStructu
                 new object[] { favorStructuralComparisons });
         }
 
-        internal sealed class DefaultValueComparer<T> : ValueComparer<T>
+        internal class DefaultValueComparer<T> : ValueComparer<T>
         {
             public DefaultValueComparer(bool favorStructuralComparisons)
                 : base(favorStructuralComparisons)
@@ -232,5 +250,27 @@ public override object Snapshot(object instance)
             public override T Snapshot(T instance)
                 => instance;
         }
+
+        internal sealed class DefaultDoubleValueComparer : DefaultValueComparer<double>
+        {
+            public DefaultDoubleValueComparer(bool favorStructuralComparisons)
+                : base(favorStructuralComparisons)
+            {
+            }
+
+            public override Expression ExtractEqualsBody(Expression leftExpression, Expression rightExpression)
+                => Expression.Call(leftExpression, _doubleEqualsMethodInfo, rightExpression);
+        }
+
+        internal sealed class DefaultFloatValueComparer : DefaultValueComparer<float>
+        {
+            public DefaultFloatValueComparer(bool favorStructuralComparisons)
+                : base(favorStructuralComparisons)
+            {
+            }
+
+            public override Expression ExtractEqualsBody(Expression leftExpression, Expression rightExpression)
+                => Expression.Call(leftExpression, _floatEqualsMethodInfo, rightExpression);
+        }
     }
 }
diff --git a/src/EFCore/ChangeTracking/ValueComparer`.cs b/src/EFCore/ChangeTracking/ValueComparer`.cs
@@ -113,8 +113,6 @@ public ValueComparer(bool favorStructuralComparisons)
                 || unwrappedType == typeof(Guid)
                 || unwrappedType == typeof(bool)
                 || unwrappedType == typeof(decimal)
-                || unwrappedType == typeof(double)
-                || unwrappedType == typeof(float)
                 || unwrappedType == typeof(object)
             )
             {