Skip to content

Commit

Permalink
Check offset when comparing DateTimeOffset (#24606)
Browse files Browse the repository at this point in the history
Fixes #24567
  • Loading branch information
roji authored Apr 8, 2021
1 parent 784624e commit ac2bb48
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
38 changes: 35 additions & 3 deletions src/EFCore/ChangeTracking/ValueComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Microsoft.EntityFrameworkCore.Query;
Expand Down Expand Up @@ -205,13 +206,17 @@ public static ValueComparer CreateDefault(Type type, bool favorStructuralCompari
return new DefaultFloatValueComparer(favorStructuralComparisons);
}

if (nonNullabletype == typeof(DateTimeOffset))
{
return new DefaultDateTimeOffsetValueComparer(favorStructuralComparisons);
}

var comparerType = nonNullabletype.IsInteger()
|| nonNullabletype == typeof(decimal)
|| nonNullabletype == typeof(bool)
|| nonNullabletype == typeof(string)
|| nonNullabletype == typeof(DateTime)
|| nonNullabletype == typeof(Guid)
|| nonNullabletype == typeof(DateTimeOffset)
|| nonNullabletype == typeof(TimeSpan)
? typeof(DefaultValueComparer<>)
: typeof(ValueComparer<>);
Expand All @@ -228,6 +233,14 @@ public DefaultValueComparer(bool favorStructuralComparisons)
{
}

public DefaultValueComparer(Expression<Func<T?, T?, bool>> equalsExpression, bool favorStructuralComparisons)
: base(
equalsExpression,
CreateDefaultHashCodeExpression(favorStructuralComparisons),
CreateDefaultSnapshotExpression(favorStructuralComparisons))
{
}

public override Expression ExtractEqualsBody(Expression leftExpression, Expression rightExpression)
=> Expression.Equal(leftExpression, rightExpression);

Expand All @@ -244,7 +257,7 @@ public override Expression ExtractSnapshotBody(Expression expression)
internal sealed class DefaultDoubleValueComparer : DefaultValueComparer<double>
{
public DefaultDoubleValueComparer(bool favorStructuralComparisons)
: base(favorStructuralComparisons)
: base((v1, v2) => v1.Equals(v2), favorStructuralComparisons)
{
}

Expand All @@ -255,12 +268,31 @@ public override Expression ExtractEqualsBody(Expression leftExpression, Expressi
internal sealed class DefaultFloatValueComparer : DefaultValueComparer<float>
{
public DefaultFloatValueComparer(bool favorStructuralComparisons)
: base(favorStructuralComparisons)
: base((v1, v2) => v1.Equals(v2), favorStructuralComparisons)
{
}

public override Expression ExtractEqualsBody(Expression leftExpression, Expression rightExpression)
=> Expression.Call(leftExpression, _floatEqualsMethodInfo, rightExpression);
}

internal sealed class DefaultDateTimeOffsetValueComparer : DefaultValueComparer<DateTimeOffset>
{
private static readonly PropertyInfo _offsetPropertyInfo = typeof(DateTimeOffset).GetProperty(nameof(DateTimeOffset.Offset))!;

// In .NET, two DateTimeOffset instances are considered equal if they represent the same point in time but with different
// time zone offsets. This comparer considers such DateTimeOffset as non-equal.
public DefaultDateTimeOffsetValueComparer(bool favorStructuralComparisons)
: base((v1, v2) => v1 == v2 && v1.Offset == v2.Offset, favorStructuralComparisons)
{
}

public override Expression ExtractEqualsBody(Expression leftExpression, Expression rightExpression)
=> Expression.And(
Expression.Equal(leftExpression, rightExpression),
Expression.Equal(
Expression.Property(leftExpression, _offsetPropertyInfo),
Expression.Property(rightExpression, _offsetPropertyInfo)));
}
}
}
10 changes: 10 additions & 0 deletions test/EFCore.Relational.Tests/Storage/RelationalTypeMappingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,16 @@ public virtual void Float_value_comparer_handles_NaN()
Assert.False(typeMapping.Comparer.Equals(3.0f, float.NaN));
}

[ConditionalFact]
public virtual void DateTimeOffset_value_comparer_handles_different_offsets()
{
var typeMapping = new DateTimeOffsetTypeMapping("datetimeoffset", DbType.DateTimeOffset);

Assert.False(typeMapping.Comparer.Equals(
new DateTimeOffset(2000, 1, 1, 12, 0, 0, TimeSpan.FromHours(0)),
new DateTimeOffset(2000, 1, 1, 13, 0, 0, TimeSpan.FromHours(1))));
}

[ConditionalFact]
public virtual void Primary_key_type_mapping_is_picked_up_by_FK_without_going_through_store_type()
{
Expand Down
4 changes: 2 additions & 2 deletions test/EFCore.Specification.Tests/BuiltInDataTypesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ private void QueryBuiltInDataTypesTest<TEntity>(EntityEntry<TEntity> source)

if (entityType.FindProperty(nameof(BuiltInDataTypes.TestDateTimeOffset)) != null)
{
var param7 = new DateTimeOffset(new DateTime(), TimeSpan.FromHours(-8.0));
var param7 = new DateTimeOffset(new DateTime(), TimeSpan.Zero);
Assert.Same(
entity,
set.Where(e => e.Id == 11 && EF.Property<DateTimeOffset>(e, nameof(BuiltInDataTypes.TestDateTimeOffset)) == param7)
Expand Down Expand Up @@ -495,7 +495,7 @@ protected EntityEntry<TEntity> AddTestBuiltInDataTypes<TEntity>(DbSet<TEntity> s
TestDouble = -1.23456789,
TestDecimal = -1234567890.01M,
TestDateTime = Fixture.DefaultDateTime,
TestDateTimeOffset = new DateTimeOffset(new DateTime(), TimeSpan.FromHours(-8.0)),
TestDateTimeOffset = new DateTimeOffset(new DateTime(), TimeSpan.Zero),
TestTimeSpan = new TimeSpan(0, 10, 9, 8, 7),
TestSingle = -1.234F,
TestBoolean = true,
Expand Down

0 comments on commit ac2bb48

Please sign in to comment.