Skip to content

Commit

Permalink
Handle NaN in value comparers for double/float (#22168)
Browse files Browse the repository at this point in the history
Fixes #22167
  • Loading branch information
roji authored Aug 25, 2020
1 parent c6ef53f commit 9451b9c
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 4 deletions.
6 changes: 5 additions & 1 deletion src/EFCore.Relational/Storage/DoubleTypeMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Data;
using System.Globalization;
using Microsoft.EntityFrameworkCore.ChangeTracking;
using JetBrains.Annotations;

namespace Microsoft.EntityFrameworkCore.Storage
Expand All @@ -27,7 +28,10 @@ public class DoubleTypeMapping : RelationalTypeMapping
public DoubleTypeMapping(
[NotNull] string storeType,
DbType? dbType = null)
: base(storeType, typeof(double), dbType)
: base(
new RelationalTypeMappingParameters(
new CoreTypeMappingParameters(typeof(double), comparer: new DoubleValueComparer()),
storeType, StoreTypePostfix.None, dbType, unicode: false, size: null, fixedLength: false, precision: null, scale: null))
{
}

Expand Down
6 changes: 5 additions & 1 deletion src/EFCore.Relational/Storage/FloatTypeMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Data;
using System.Globalization;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ChangeTracking;

namespace Microsoft.EntityFrameworkCore.Storage
{
Expand All @@ -27,7 +28,10 @@ public class FloatTypeMapping : RelationalTypeMapping
public FloatTypeMapping(
[NotNull] string storeType,
DbType? dbType = null)
: base(storeType, typeof(float), dbType)
: base(
new RelationalTypeMappingParameters(
new CoreTypeMappingParameters(typeof(float), comparer: new FloatValueComparer()),
storeType, StoreTypePostfix.None, dbType, unicode: false, size: null, fixedLength: false, precision: null, scale: null))
{
}

Expand Down
4 changes: 2 additions & 2 deletions src/EFCore/ChangeTracking/ArrayStructuralComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking
{
/// <summary>
/// <para>
/// Specifies value snapshotting and comparison for arrays where each element is compared
/// a new array is constructed when snapshotting.
/// Specifies value comparison for arrays where each element pair is compared.
/// A new array is constructed when snapshotting.
/// </para>
/// </summary>
/// <typeparam name="TElement"> The array element type. </typeparam>
Expand Down
25 changes: 25 additions & 0 deletions src/EFCore/ChangeTracking/DoubleValueComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq.Expressions;
using JetBrains.Annotations;

namespace Microsoft.EntityFrameworkCore.ChangeTracking
{
/// <summary>
/// Defines value comparison for <see cref="double" /> which correctly takes <see cref="double.NaN" />
/// into account.
/// </summary>
public class DoubleValueComparer : ValueComparer<double>
{
/// <summary>
/// Initializes a new instance of the <see cref="DoubleValueComparer" /> class.
/// </summary>
public DoubleValueComparer() : base(
(x, y) => double.IsNaN(x) ? double.IsNaN(y) : x.Equals(y),
d => d.GetHashCode())
{
}
}
}
25 changes: 25 additions & 0 deletions src/EFCore/ChangeTracking/FloatValueComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq.Expressions;
using JetBrains.Annotations;

namespace Microsoft.EntityFrameworkCore.ChangeTracking
{
/// <summary>
/// Defines value comparison for <see cref="float" /> which correctly takes <see cref="float.NaN" />
/// into account.
/// </summary>
public class FloatValueComparer : ValueComparer<float>
{
/// <summary>
/// Initializes a new instance of the <see cref="FloatValueComparer" /> class.
/// </summary>
public FloatValueComparer() : base(
(x, y) => float.IsNaN(x) ? float.IsNaN(y) : x.Equals(y),
d => d.GetHashCode())
{
}
}
}
20 changes: 20 additions & 0 deletions test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,26 @@ public virtual void UShort_literal_generated_correctly()
Test_GenerateSqlLiteral_helper(typeMapping, ushort.MaxValue, "65535");
}

[ConditionalFact]
public virtual void Double_value_comparer_handles_NaN()
{
var typeMapping = new DoubleTypeMapping("double precision", DbType.Double);

Assert.True(typeMapping.Comparer.Equals(3.0, 3.0));
Assert.True(typeMapping.Comparer.Equals(double.NaN, double.NaN));
Assert.False(typeMapping.Comparer.Equals(3.0, double.NaN));
}

[ConditionalFact]
public virtual void Float_value_comparer_handles_NaN()
{
var typeMapping = new FloatTypeMapping("float", DbType.Single);

Assert.True(typeMapping.Comparer.Equals(3.0f, 3.0f));
Assert.True(typeMapping.Comparer.Equals(float.NaN, float.NaN));
Assert.False(typeMapping.Comparer.Equals(3.0f, float.NaN));
}

[ConditionalFact]
public virtual void Primary_key_type_mapping_is_picked_up_by_FK_without_going_through_store_type()
{
Expand Down

0 comments on commit 9451b9c

Please sign in to comment.