Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HasConversion is not used in the provider value comparer #29406

Closed
Searen1984 opened this issue Oct 21, 2022 · 2 comments · Fixed by #29601
Closed

HasConversion is not used in the provider value comparer #29406

Searen1984 opened this issue Oct 21, 2022 · 2 comments · Fixed by #29601
Labels
area-change-tracking closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@Searen1984
Copy link

Searen1984 commented Oct 21, 2022

EF Core version: 7.0.0-rc.2.22472.11
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 6.0
Operating system: Windows 2011
IDE: Visual Studio 2022 17.3.1

When upgrading from EFCore 6 to the release candidate I got the following exception:

---> System.AggregateException: One or more errors occurred. (Unable to cast object of type 'System.Decimal' to type 'System.Int64'.)
---> System.InvalidCastException: Unable to cast object of type 'System.Decimal' to type 'System.Int64'.
   at Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer`1.Equals(Object left, Object right)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetProperty(IPropertyBase propertyBase, Object value, Boolean isMaterialization, Boolean setModified, Boolean isCascadeDelete, CurrentValueType valueType)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.ValueGenerationManager.Generate(InternalEntityEntry entry, Boolean includePrimaryKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Boolean modifyProperties, Nullable`1 forceStateWhenUnknownKey, Nullable`1 fallbackState)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode`1 node)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraph(InternalEntityEntry rootEntry, EntityState targetState, EntityState storeGeneratedWithKeySetTargetState, Boolean forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1.SetEntityState(InternalEntityEntry entry, EntityState entityState)
   at Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1.Add(TEntity entity)

I found already the origin but let me give you some code first. We have the following entity:

public record MyRecord
{
  public long MyKey { get; set; }
  public long OtherData { get; set; }
}

In the SQL database table both values (MyKey and OtherData) are of the type numeric(18,0) which is interpreted in EFCore as the datatype decimal. Because a direct cast between long and decimal is not possible a conversion has always to be done between those data types for comparison and so on. Thus, in the corresponding DbContext I defined the following:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
  modelBuilder.Entity<MyRecord>(entity =>
  {
    entity.HasKey(e => e.MyKey);
    entity.Property(e => e.MyKey)
        .ValueGeneratedOnAdd()
        .HasPrecision(18, 0)
        .HasColumnName("MyKey")
        .HasConversion(v => new decimal(v), v => decimal.ToInt64(v));
  
    entity.Property(e => e.OtherData)
                  .IsRequired()
                  .HasPrecision(18, 0)
                  .HasColumnName("OtherData")
                  .HasConversion(v => new decimal(v), v => decimal.ToInt64(v));
  }
}

However, I found that this conversion is not taken into account anymore for the provider value comparer in EFCore 7. So, I was overwriting the methods Snapshot, GetHashCode and Equals in a derived class and used that instead of the default one. The following code works:

public class MyDbContext : DbContext
{
  ... Some general stuff

  private class MyValueComparer<T> : ValueComparer<T>
  {
    public MyValueComparer(Expression<Func<T, T, bool>> equalsExpression, Expression<Func<T, int>> hashCodeExpression, Expression<Func<T, T>> snapshotExpression) : base(equalsExpression, hashCodeExpression, snapshotExpression)
  {}
  
    public override object Snapshot(object instance)
    {
        if (instance is decimal instance1)
        {
            return Snapshot(decimal.ToInt64(instance1));
        }
        else
        {
            return Snapshot((T)instance);
        }
    }
  
    public override int GetHashCode(object instance)
    {
        if (instance is decimal instance1)
        {
            return GetHashCode(decimal.ToInt64(instance1));
        }
        else
        {
            return GetHashCode((T)instance);
        }
    }
  
    public override bool Equals(object left, object right)
    {
        var v1Null = left == null;
        var v2Null = right == null;
  
        var t1 = left?.GetType();
        var t2 = right?.GetType();
  
        if (right is null)
        {
            return v1Null;
        }
        else if (right is decimal right1)
        {
            return !v1Null && Equals((T)left, decimal.ToInt64(right1));
        }
        else
        {
            return !v1Null && Equals((T)left, (T)right);
        }
    }
  }

  protected override void OnModelCreating(ModelBuilder modelBuilder)
  {
    modelBuilder.Entity<MyRecord>(entity =>
    {
      var keyComparer = new MyValueComparer<long>(
                (l, r) => l == r,
                v => v.GetHashCode(),
                v => v);

            var keyConverter = new ValueConverter<long, decimal>(
                v => new decimal(v),
                v => decimal.ToInt64(v));

      entity.HasKey(e => e.MyKey);
      entity.Property(e => e.MyKey)
          .ValueGeneratedOnAdd()
          .HasPrecision(18, 0)
          .HasColumnName("MyKey")
          .HasConversion(keyConverter, keyComparer);
    
      entity.Property(e => e.OtherData)
                    .IsRequired()
                    .HasPrecision(18, 0)
                    .HasColumnName("OtherData")
                    .HasConversion(v => new decimal(v), v => decimal.ToInt64(v));
    }
  }

So, I guess the bug is that the provider value comparer does not use the defined conversion in the corresponding methods or before those are called.

@ajcvickers
Copy link
Member

@Searen1984 I am not able to reproduce this--see my code below. Please attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

public static class Your
{
    public static string ConnectionString = @"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow";
}

public record MyRecord
{
    public long MyKey { get; set; }
    public long OtherData { get; set; }
}

public class SomeDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(Your.ConnectionString)
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<MyRecord>(
            entity =>
                {
                    var keyComparer = new MyValueComparer<long>(
                        (l, r) => l == r,
                        v => v.GetHashCode(),
                        v => v);

                    var keyConverter = new ValueConverter<long, decimal>(
                        v => new decimal(v),
                        v => decimal.ToInt64(v));

                    entity.HasKey(e => e.MyKey);
                    entity.Property(e => e.MyKey)
                        .ValueGeneratedOnAdd()
                        .HasPrecision(18, 0)
                        .HasColumnName("MyKey")
                        .HasConversion(keyConverter, keyComparer);

                    entity.Property(e => e.OtherData)
                        .IsRequired()
                        .HasPrecision(18, 0)
                        .HasColumnName("OtherData")
                        .HasConversion(v => new decimal(v), v => decimal.ToInt64(v));
                });
    }

    private class MyValueComparer<T> : ValueComparer<T>
    {
        public MyValueComparer(
            Expression<Func<T, T, bool>> equalsExpression,
            Expression<Func<T, int>> hashCodeExpression,
            Expression<Func<T, T>> snapshotExpression)
            : base(equalsExpression, hashCodeExpression, snapshotExpression)
        {
        }

        public override object Snapshot(object instance)
        {
            if (instance is decimal instance1)
            {
                return Snapshot(decimal.ToInt64(instance1));
            }
            else
            {
                return Snapshot((T)instance);
            }
        }

        public override int GetHashCode(object instance)
        {
            if (instance is decimal instance1)
            {
                return GetHashCode(decimal.ToInt64(instance1));
            }
            else
            {
                return GetHashCode((T)instance);
            }
        }

        public override bool Equals(object left, object right)
        {
            var v1Null = left == null;
            var v2Null = right == null;

            var t1 = left?.GetType();
            var t2 = right?.GetType();

            if (right is null)
            {
                return v1Null;
            }
            else if (right is decimal right1)
            {
                return !v1Null && Equals((T)left, decimal.ToInt64(right1));
            }
            else
            {
                return !v1Null && Equals((T)left, (T)right);
            }
        }
    }
}

public class Program
{
    public static void Main()
    {
        using (var context = new SomeDbContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.AddRange(new MyRecord { OtherData = 2 }, new MyRecord { OtherData = 4 });
            context.SaveChanges();
        }
    }
}

@Searen1984
Copy link
Author

I think there is a misunderstanding. The problem only occurs for the key. So, to reproduce the issue you should 1. remove the class MyValueComparer AND 2. remove the keyComparer from the MyKey Property. Then you should run into the exception.

@ajcvickers ajcvickers self-assigned this Oct 26, 2022
@ajcvickers ajcvickers added this to the 8.0.0 milestone Oct 27, 2022
ajcvickers added a commit that referenced this issue Nov 17, 2022
Fixes #29406

The value generator selection was returning a generator for the converted type, when a value for the non-converted type should have been used.
ajcvickers added a commit that referenced this issue Nov 17, 2022
Fixes #29406

The value generator selection was returning a generator for the converted type, when a value for the non-converted type should have been used.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Nov 19, 2022
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-preview1 Jan 29, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview1, 8.0.0 Nov 14, 2023
@ajcvickers ajcvickers removed their assignment Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-change-tracking closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants