Support inheritance for owned/property bag/shared-type entity types

[ajcvickers]

For example, this from #9536:

public class Friend
{
    public int Id { get; set; }
    public string Name { get; set; }

    public FullAddress Address { get; set; }
}

public class LessThanFriend
{
    public int Id { get; set; }
    public string Name { get; set; }

    public CityAddress Address { get; set; }
}

public class CityAddress
{
    public string Cap { get; set; }
    public string City { get; set; }
}

public class FullAddress : CityAddress
{
    public string Street { get; set; }
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Friend>().OwnsOne(e => e.Address);
    modelBuilder.Entity<LessThanFriend>().OwnsOne(e => e.Address);
}

This is ambiguous as to whether LessThanFriend.CityAddress should be mapped to allow inheritance such that either a CityAddress or a FullAddress could be persisted. Typically, we only map inheritance when both types are in the model. However, having both types in the model as Owned types on different entities perhaps does not have the same semantics--especially when thinking of them like complex types.

Based on triage discussion from #9536, we think we want to support both the simple mapping (just persisting given concrete type, which is what the request on #9536 is for) and the inheritance mapping, which would require, for TPH, and additional discriminator column in the table. We did not come to a final decision on which should be the default, or what the new API will look like to switch between the two.

[AndriySvyryd]

We would need API on ReferenceOwnershipBuilder to declare the derived types.
Related to #10140

[andre-f-paggi]

It took me so much time to get to this issue.

I am having a problem in which a owned type generates a Discriminator property, even though there is no need for it, since the class that has this property is not derived anywhere else.

Can this be the same issue?

I tried removing it from migration and from the snapshot to test, but it generates a wrong sql statement when acessing the DbContext on that Set.

Is there any way to get around this, without generating the property on the database?

[ajcvickers]

@andre-f-paggi Please file a new issue and include a runnable project/solution or complete code listing that demonstrates the behavior you are seeing.

[chris-stormideas]

@ajcvickers I can look at supplying a runnable solution if it will assist getting it resolved quicker

[ajcvickers]

@chris-stormideas We understand what this issue is about--i.e. inheritance mapping into a shared table is not supported. The feature is currently on the backlog, and not considered very high priority. The best way to influence getting it resolved is probably to make a case for why this specific mapping is so important.

[davidroth]

@ajcvickers

What about collections of owned types, where the owned-collection is polymorphic?
For example:

public class Order
{
    public List<Position> Positions { get; set; } // Owned collection
}

public abstract class Position
{ }

public class FancyPosition : Position
{ }

public class StandardPosition : Position
{ }

public class SampleContext : DbContext
{
    public virtual DbSet<Order> Orders { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        modelBuilder.Entity<Order>().OwnsMany(p => p.Positions);
    }
}

Is this implicitly tracked by this issue or would you recommend creating a new issue for this?

[ajcvickers]

@AndriySvyryd Thoughts on polymorphic owned collections?

[AndriySvyryd]

Polymorphic owned collections will be covered by this, but similar to non-collections all derived types will need to be configured explicitly.

[digitalpowers]

Polymorphic owned collections do provide the ability to compose our object model which is really quite nice.
From my perspective having to define all the derived types explicitly isn't a problem at all.
Do you guys have an idea when this issue might bubble up in the backlog?

I have a project I have been waiting to migrate over to efcore that currently uses Horrible InHouse Awful ORM (tm) that I wrote years ago much to today me's regret. This is the only feature left that I cant really support with efcore.

[AndriySvyryd]

@digitalpowers For now we can only say that it won't happen this year.

[davidroth]

Thanks for the feedback. If polymorphic owned collections are covered by this issue, I would recommend updating the issue title+description so that users can find this. ATM it is not really clear and looks like it only covers inline owned types. Thanks!

[Lobosque]

@AndriySvyryd Does this issue also cover the case where I explicitly want to set a HasDiscriminator under an owned entity?

Example:

            builder.OwnsOne<AlertPattern>("Pattern", b =>
            {
                b.HasDiscriminator<int>("type")
                    .HasValue<PatternOne>(1)
                    .HasValue<PatternTwo>(2)
                    .HasValue<PatternThree>(3)
                    .HasValue<PatternFour>(4)
                    .HasValue<PatternFive>(5);
            });

In this particular example, AlertPattern is an abstract class and PatternOne and friends inherit it.

[AndriySvyryd]

@Lobosque Yes

[reinux]

• (and what I have done) Switch from Owned to Has. This is annoying because I then need to .Inclue it on selects.

Also because it forces you to tightly couple unrelated parts of your code in some common situations.

[AndiHahn]

Any update on this?

Especially with Cosmos Db Provider it's very likely to need inheritance on owned collection types.

[ajcvickers]

@AndiHahn This issue is in the Backlog milestone. This means that it is not planned for the next release (EF Core 7.0). We will re-assess the backlog following the this release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources. Make sure to vote (👍) for this issue if it is important to you.

[darthg8r]

Eeeshhh, this still isn't on the board? It seems like such a basic feature for any kind of non anemic modeling. I constantly bump up against this, search to see if there's an update, and find this issue on GitHub. I've even unvoted and voted for it multiple times. Come on guys, this stinks. Somehow, stored procedure support in EF has gotten more attention, even when it's perfectly viable to drop to ADO.NET to execute.

[ComptonAlvaro]

I would ask in #29252.

In sumary, I wanted to have a value object status for an order. I want to follow the state pattern.

Then to map in EF Core I need to configure the status as owned entity. But I need to configure also the derived classes, but I don't know how to do.

Then my question is. Is it possible to configure EF Core for state pattern when the state is a value object? If not, perhaps the another option would be to use the state as entity, but then the domain is affected by the database, and I would like to avoid this option if it would be possible.

Thanks.

[ajcvickers]

@ComptonAlvaro I don't think it's currently possible to do what you are asking.

[ComptonAlvaro]

@ajcvickers Thanks for you help. Then if actually is not possible, perhaps in the future?

And now, which could be an alternative?

1.- Don't use state pattern in the domain
2.- Use state pattern and create one table for each derived classes. But in this case each table would have only one record.
3.- Use state pattern and use state as entity instead of value object?
4.- Use state pattern in the domain, use state as value object in the domain, in the database to have a table for the states and in the application layer convert the state from database to the value object that will be used by the domain.
5.- Perhaps another option.

Thanks.

[ajcvickers]

@ComptonAlvaro Hard to say, since it's your code and your architecture, but my initial impression is to just keep things simple, which probably means not using the state pattern.

[marchy]

Just wanted to post up and example that I think showcases how much benefit having inheritance specified on owned types could have.

For a two-case discriminated union that allows setting two different types, each of which refers to a different entity (referential relation in this case):

public abstract record Target {
	public record ReferenceTarget( EntityReference Reference ) : Target; // style 1: linked entity only
	public record ContentTarget( string ContentID, Content Content ) : Target; // style 2: linked entity and foreign key
}

that would ideally only have the following property defined in the POCO model:

public class SomeDomainModel {
	...
	public Target Target { get; private set; } // if owned type inheritance was supported
	...
}

we currently have to work around this by mapping all the properties to DAL-only properties and use a property wrapper that switches on the model and nulls out all the non-shared properties/columns between them.

public class SomeDomainModel {
	...
	public Target Target {
		get => TargetType switch {
			nameof(Target.ReferenceTarget) => new Target.ReferenceTarget( Reference! ),
			nameof(Target.ContentTarget) => new Target.ContentTarget( ContentID!, Content! ),
			_ => throw new NotSupportedException( $"Unknown target type '{TargetType}'" ),
		};
		set {
			(string, EntityType?, string?, string?, EntityReference?, string?, Content?) persistedValue = value switch {
				Target.ReferenceTarget referenceTarget => (
					TargetType: nameof(Target.ReferenceTarget),
					ReferenceType: referenceTarget.Reference.Type,
					ReferenceProvider: referenceTarget.Reference.Provider,
					ReferenceID: referenceTarget.Reference.ID,
					Reference: referenceTarget.Reference,
					// NOTE: we null out non-common properties of other sub-classes
					ContentID: null,
					Content: null
				),
				Target.ContentTarget contentTarget => (
					TargetType: nameof(Target.ContentTarget),
					// NOTE: we null out non-common properties of other sub-classes
					ReferenceType: null,
					ReferenceProvider: null,
					ReferenceID: null,
					Reference: null,
					ContentID: contentTarget.ContentID,
					Content: contentTarget.Content
				),
				_ => throw new NotSupportedException( $"Unknown identity type '{value.GetType().Name}'" ),
			};
			(TargetType, ReferenceType, ReferenceProvider, ReferenceID, Reference, ContentID, Content) = persistedValue;
		}
	}
	/*DAL*/internal string TargetType { get; private set; }
	// NOTE: composite foreign key
	/*DAL*/internal EntityType? ReferenceType { get; private set; }
	/*DAL*/internal string? ReferenceProvider { get; private set; }
	/*DAL*/internal string? ReferenceID { get; private set; }
	/*DAL*/internal EntityReference? Reference { get; private set; }
	/*DAL*/internal string? ContentID { get; private set; }
	/*DAL*/internal Content? Content { get; private set; }
	...
}

Note that ReferenceType, ReferenceProvider and ReferenceID are all foreign keys into a composite key of the linked entity, while ContentID is a simple single-column foreign key.

Ideally the EF configuration could be set up so that the underlying columns that get mapped are still a flattened set of the combined sub-class columns similar to what happens for TPH on a non-owned entity:

• TargetType:text (discriminator for union)
• ReferenceType:text (enum)
• ReferenceProvider:text
• ReferenceID:text
• ContentID:text

This would require letting EF know above the known sub-classes (or it discovering them) so that it can "flatten" the hierarchy into nullable columns and add in the discriminator column.

Please note that the code above is required for a single property of our main model (Target), and in practice we have multiple of these in the model alongside other regular properties/columns, and our domain is filled with these patterns where you need a type hierarchy (ie: discriminated union) to accurately model.

For other domain models, we have similar patterns but are okay with them being JSON blobs rather than flattened columns – in which case unfortunately EF doesn't support that either (#27779)

[edwickensrhenus]

@ajcvickers

What about collections of owned types, where the owned-collection is polymorphic? For example:

public class Order
{
    public List<Position> Positions { get; set; } // Owned collection
}

public abstract class Position
{ }

public class FancyPosition : Position
{ }

public class StandardPosition : Position
{ }

public class SampleContext : DbContext
{
    public virtual DbSet<Order> Orders { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        modelBuilder.Entity<Order>().OwnsMany(p => p.Positions);
    }
}

Is this implicitly tracked by this issue or would you recommend creating a new issue for this?

I just wanted to register my interest in support for this exact scenario

[digitalpowers]

is any input needed to help decide if this makes the cut for the current release? It is still the only thing preventing me from migrating to efcore and I very much want to :)

[chrisc-ona]

What's the chance we might see this in EF Core 9?

[ErikEJ]

@chrisc-ona None - 9 is done

[chrisc-ona]

@ErikEJ well hopefully we'll have it in 10 then