Efcore: TPH: ValueConverters for properties with shared backing columns are overridden

Created on 2 Feb 2019  路  5Comments  路  Source: dotnet/efcore

In a TBH scenario, if two derived classes use the same backing column for a property, value conversion won't work properly. That is only one of the converters are called for every derived type. It seems like the converter on the first derived type alphabetically is used always.

Steps to reproduce

Here's a repro on github.

Data models:
```c#
public abstract class Base
{
public int Id { get; set; }
public abstract int Type { get; set; }
}

public class Derived1 : Base
{
    public override int Type
    {
        get => 1;
        set { }
    }

    public int PropertyWithSharedBackingColumn { get; set; }

    public override string ToString()
    {
        return $"Id={Id} Type={Type} SharedColumnValue={PropertyWithSharedBackingColumn}";
    }
}

public class Derived2 : Base
{

    public override int Type
    {
        get => 2;
        set { }
    }

    public int PropertyWithSharedBackingColumn { get; set; }

    public override string ToString()
    {
        return $"Id={Id} Type={Type} SharedColumnValue={PropertyWithSharedBackingColumn}";
    }
}
public class Derived3 : Base
{

    public override int Type
    {
        get => 3;
        set { }
    }

    public string PropertyWithSharedBackingColumn { get; set; }

    public override string ToString()
    {
        return $"Id={Id} Type={Type} SharedColumnValue={PropertyWithSharedBackingColumn}";
    }
}
Context:
```c#
    public class TestDbContext : DbContext
    {
        public TestDbContext(DbContextOptions options)
            : base(options)
        {

        }

        protected override void OnModelCreating(ModelBuilder builder)
        {
            builder.Entity<Base>()
                .HasDiscriminator(e => e.Type)
                .HasValue<Derived1>(1)
                .HasValue<Derived2>(2)
                .HasValue<Derived3>(3);


            builder.Entity<Derived1>()
                .HasBaseType<Base>()
                .Property(e => e.PropertyWithSharedBackingColumn)
                .HasColumnName("SharedColumn")
                .HasConversion(
                    i => "1",
                    stored => 1);

            builder.Entity<Derived2>()
                .HasBaseType<Base>()
                .Property(e => e.PropertyWithSharedBackingColumn)
                .HasColumnName("SharedColumn")
                .HasConversion(
                    i => "2",
                    stored => 2);

            builder.Entity<Derived3>()
                .HasBaseType<Base>()
                .Property(e => e.PropertyWithSharedBackingColumn)
                .HasColumnName("SharedColumn")
                .HasConversion(
                    i => "3",
                    stored => "3");
        }

        public DbSet<Base> Entities { get; set; }
    }

And the code to test the converters which simply writes the two saved objects to the output
```c#
var dbConnection = new SqliteConnection("DataSource=:memory:");
dbConnection.Open();

        var options = new DbContextOptionsBuilder<TestDbContext>()
            .UseSqlite(dbConnection)
            .Options;

        var context = new TestDbContext(options);

        context.Database.EnsureCreated();

        context.Add(new Derived1() { PropertyWithSharedBackingColumn = 1 });
        context.Add(new Derived2() { PropertyWithSharedBackingColumn = 2 });
        context.Add(new Derived3() { PropertyWithSharedBackingColumn = "3" });
        context.SaveChanges();

        List<Base> results = new TestDbContext(options).Entities.ToList();

        Console.WriteLine(string.Join(Environment.NewLine, results));
### Expected behavior

Id=1 Type=1 SharedColumnValue=1
Id=2 Type=2 SharedColumnValue=2
Id=3 Type=3 SharedColumnValue=3

### Actual behavior

Id=1 Type=1 SharedColumnValue=1
Id=2 Type=2 SharedColumnValue=1 <--- this should be 2
Id=3 Type=3 SharedColumnValue=3 <--- note that this one is not affected
```

It seems the converters with the same signatures are only overridden here. But In the project I was originally encountered this issue my properties were of different types (both dictionaries but with different type params) being serialized into a nvarchar column.
I have also tested this against SQL server which yields the same behavior.

Further technical details

EF Core version: 2.2.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer 2.2.1, Microsoft.EntityFrameworkCore.Sqlite 2.2.1
Operating system: Windows 10 1803, Ubuntu 16.04
Runtime: dotnet core 2.1

area-c-mapping area-relational-mapping area-type-mapping closed-fixed customer-reported punted-for-3.0 punted-for-3.1 punted-for-5.0 type-enhancement

Most helpful comment

Notes from triage: This is non-trivial with the current materializer since value conversion happens when reading from the data reader into the value buffer, which is not conditional on the final property. However, the 3.0 query changes _may_ make this easy if the actual entity type instance and property is known when reading from the data reader. /cc @smitpatel

For 3.0, we will either throw to explicitly disallow this kind of converter configuration, or make it work if the new materializer makes that trivial.

All 5 comments

Notes from triage: This is non-trivial with the current materializer since value conversion happens when reading from the data reader into the value buffer, which is not conditional on the final property. However, the 3.0 query changes _may_ make this easy if the actual entity type instance and property is known when reading from the data reader. /cc @smitpatel

For 3.0, we will either throw to explicitly disallow this kind of converter configuration, or make it work if the new materializer makes that trivial.

This should be able to work in new query pipeline materializer.

@AndriySvyryd - Do all of the above properties have different IProperty in the model?

@smitpatel Yes

Verified that is is fixed in 3.1 and 5.0. (In 3.1, the string property needs to be made non-nullable; this is not required for 5.0.)

Was this page helpful?
0 / 5 - 0 ratings