Efcore: Use base class for ad hoc discriminators in TPH

[ajcvickers]

This issue was moved from the discussions tab.

Question

@snappyfoo
In 3.1, and 5.0 where you mark the discriminator as incomplete, reading the base class will generate an IN for all the known values provided in the EF setup. In 5.0 you can mark as complete to exclude the IN, but it will fail to read unknown discriminator values as it won’t know which class to instantiate.

Can I ask, is there a reason why the base class can’t be used for all ad hoc values it comes across that aren’t defined, including null?

It would be quite handy just to be able to get all types in cases where there may be customer defined discriminator values. We provide APIs that deal with certain known types but also want to be able to get all types without errors.

I would request it as a feature but I suspect there are some fundamental reasons why what I’m proposing don’t make sense in the discriminator pattern- but I’d like to discuss.

Replies

@smitpatel

Can I ask, is there a reason why the base class can’t be used for all ad hoc values it comes across that aren’t defined, including null?

Querying data from server is just half work done for a query. After data is returned from the server, we also need to materialize it to generate client side objects. So if you have a discriminator value which is not known to EF then we don't know what object type to materialize and it will throw error when executing the query.

It would be quite handy just to be able to get all types in cases where there may be customer defined discriminator values.

Does EF Core know about customer defined discriminator values? Even if we get that data from server, how are you planning to access it if we cannot create instances of it?

In the end, discriminator mapping is complete i.e. all possible values and their associated object types are part of EF Core model and EF core can create instance of any of them. or EF Core does not know all possible values and need to generate IN clause to avoid throwing an error when hitting an unknown value which it does not know how to materialize.

@snappyfoo

So if you have a discriminator value which is not known to EF then we don't know what object type to materialize and it will throw error when executing the query.

As I have suggested in the question, you could materialise it into the base class. Why is this not an option? You don’t know what specific type it is (it’s unknown to EF) but you know it has at least has all the base class properties. You know what the table looks like.

@smitpatel

10140 allows to map multiple discriminator values to same entity type. Once that is implemented you can map all other discriminator to base type.

We cannot materialize in the base class if the discriminator value does not match any value. It could potentially cause a data loss because additional data from server was not loaded.

[snappyfoo]

Thanks @smitpatel, writing back and potential data loss was the gap in my thinking. Out of interest, what is the feasibility around having this as read-only behaviour? i.e. base class materialisations due to unknown discriminator values cannot be saved?

I’ll admit, we have a peculiar set up where the database is actually managed by two systems, only one being EF (DB first), and for this particular table EF only needs to read. We are probably not using the discriminator feature as intended either, as all types have the same properties (ie all are defined on the base) but being able to have navigation properties to particular types as defined via the discriminator is very useful.

[smitpatel]

While we say it is read-only behavior but it is not truly read-only in EF context because user could actually attach the instance after it is created. Perhaps a different way to model your scenario may help.
Is mapping base class required? Could it be a base type which is not mapped in EF? Derived types will still take properties from base type. How are you defining navigations in derived type? Do they have same target or different target? Can you share more details about model specially with some code snippets.

[snappyfoo]

The table holds addresses, which can be owned by different types of entity. It does this like so:

Id | OwnerType | OwnerId | UpdatedByPersonId | [OtherColumns]
1    PERSON      1                2
2    BUSINESS    1               2

The OwnerId is a 'soft' foreign key to another table, depending on the type of entity.

Firstly, let me cover the database design. This is a legacy table but I've looked at other ways to represent this relationship, and I don't think there is a good alternative that uses foreign keys. Creating linking tables, e.g. PersonAddress and BusinessAddress, cannot enforce that the address can only have one owner - a link could be created in both linking tables. Or having nullable columns on the address table, each being an FK to other entities, gets out of hand when there are lots of other entities, plus check constraints or triggers to check not more than one is set. So, given the design...

I try to represent these links as navigation properties in EF. But a relationship can't include hard coded values. So I tried the discriminator approach.

In the Address configuration...

    .HasDiscriminator<string>(nameof(Address.OwnerType))
    .HasValue<PersonAddress>("PERSON")
    .HasValue<BusinessAddress>("BUSINESS");

Then Person can have an Addresses property of type ICollection<PersonAddress>, with a configuration like so...

    .HasMany(x => x.Addresses)
    .WithOne()
    .HasPrincipalKey(x => x.Id)
    .HasForeignKey(x => x.OwnerId);

Both bits of config end up producing what essentially looks like a join ON a.OwnerType = 'PERSON' AND a.OwnerId = p.Id

Having that produced by EF so I can use navigation properties instead of awkward joins in query syntax makes it so much easier to use.

Person also has an AddressesUpdated property of type ICollection<Address> which works off the UpdatedByPersonId column. This lists all the addresses regardless of type.

Now, as the product is old, many different customers, legacy data, lack of proper foreign key constraints, it's possible for unknown types to appear in that table. As there are actually many different known types (more than the 2 here), having it as incomplete adds a very large IN statement which I don't like. But marking as complete opens it up to it failing. Now, I realise the onus is probably on us to sort out the data, but thinking about all this just made me wonder why it couldn't just materialize unknown types into the base class. You mentioned one of the complications, i.e. around saving, which I take on board. However, I would appreciate any ideas about how better to model, if at all. Thanks

[smitpatel]

@AndriySvyryd - Is above scenario supported?

[AndriySvyryd]

Duplicate of #10140

Flexible discriminators will allow to have this type of condition