Support change tracking of non-primitive types

[davidkvc]

ChangeTracker does not detect changes to properties of type Dictionary<,>. In provider for PostgreSQL database database column type hstore is mapped to .net type Dictionary<string,string> but this feature can not be used properly if we do not have change tracking for those properties. One solution would be to use ImmutableDictionary but we want to avoid that.

Steps to reproduce

public class HStoreContext : DbContext
{
   public DbSet<SomeEntity> SomeEntities { get; set; }
}

public class SomeEntity
{
   public int Id { get; set; }
   public Dictionary<string, string> Tags { get; set; }
}

using (HStoreContext ctx = CreateContext())
{
   var entity = new SomeEntity()
   {
       Tags = new Dictionary<string, string>()
   };
   ctx.SomeEntities.Add(entity);
   var num = ctx.SaveChanges();

   Assert.Equal(1, num);

   var entry = ctx.Entry(entity);
   Assert.Equal(EntityState.Unchanged, entry.State);

   entity.Tags.Add("kind", "new");
   ctx.ChangeTracker.DetectChanges();

   Assert.Equal(EntityState.Modified, entry.State); // This fails
}

Further technical details

EF Core version: 2.0.0
Database Provider: Microsoft.EntityFrameworkCore.PostgreSQL
Operating system:
IDE: Visual Studio 2017

[ajcvickers]

@dejvid-smth The ability to extend equality checks beyond ".Equals" is something that is not implemented yet. We intended to do it as part of type conversions (#242), since it is often needed then, although it is really a different feature. It's also being tracked as part of type mapping here: #8537

[roji]

Hmm... Unless I'm mistaken Dictionary's .Equals() should compare the items, so EF Core should properly detect changes in a dictionary's contents... @dejvid-smth I haven't had time yet to properly review and test your PR which requires this, but could you double-check what's happening?

[davidkvc]

I don't know how exactly ChangeTracker works but we are just updating the same instance of Dictionary and EntityEntry from ChangeTracker only tracks if we actually change instance so both OriginalValue and CurrentValue are pointing to the same instance of Dictionary and therefore are the same.

We would have to somehow make a copy of original Dictionary when EntityEntry's state is set to Unmodified and save that as OriginalValue in EntityEntry so we could later compare OriginalValue with CurrentValue.

[ajcvickers]

@roji Pretty sure Dictionary just does reference equality on the dictionary instance.

@dejvid-smth Correct, the snapshot would need to have a copy of the dictionary.

[ajcvickers]

@roji You will need to generate mappings with the appropriate ValueComparers for the types that Postgres supports. It's not clear to be that a deep snapshot/comparison is always the best choice for this kind of property depending on how it is used, but I think you would know better how people typically use these kinds of properties with Postgres and can therefore make a decision as to which is the appropriate behavior. Let's discuss if I'm missing anything.

[roji]

Thanks @ajcvickers! I'll take a look at the relevant types and modify their mappings accordingly for 2.1. I think that for arrays and for hstore (a native Dictionary<string,string> PostgreSQL datatype) a deep value comparison is the right thing to do, but we'll see what users say.

For anyone following, I've opened npgsql/efcore.pg#305 for array comprison behavior and npgsql/efcore.pg#306 for hstore.

[roji]

Just to report that I've value comparison for hstore seems to work perfectly... I'll be adding comparers to array and other relevant types for 2.1 as well.

Thanks!

[roji]

@ajcvickers if you're interesting in seeing a complex/interesting comparer implementation, take a look at the array comparer, it will also illustrate some of the issues detailed in #11072.