-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Add inheritance support for complex types #31250
Comments
Please, please prioritize doing this. We use type hierarchies all over our domain, specifically associative enums which in C# are (currently) best modelled via abstract records (or Kotlin's data classes equivalent). Currently we have to add "access wrapper" properties around these objects and hide away the field-mapping implementation details in partial classes to try to keep the domain clean. For example, if some entity can be associated to either a user or an anonymous user (ie: modelled in the following example through an Installation entity), we have to add discriminator columns and hide away the 'XOR'-like logic of referencing the respective columns ourselves. public abstract record Identity {
public record Anonymous( string InstallationID, Platform Platform, Installation? Installation ) : Identity;
public record Authenticated( long AccountID, Account? Account ) : Identity;
}
public partial class SomeEntity {
// ... other state of the entity
// NOTE: complex type hierarchy
public Identity Identity {
get => IdentityType switch {
nameof(Identity.Anonymous) => new Identity.Anonymous( InstallationID.Value, Platform.Value, Installation ),
nameof(Identity.Authenticated) => new Identity.Authenticated( AccountID.Value, Account ),
_ => throw new ArgumentOutOfRangeException( $"Unknown identity type '{IdentityType}' ),
};
init => {
(string IdentityType, string? InstallationID, Installation? Installation, long? AccountID) persistedValue = value switch {
Identity.Anonymous anonymousIdentity => {
IdentityType: nameof(Identity.Anonymous),
InstallationID: anonymousIdentity.InstallationID,
Platform: anonymousIdentity.Platform,
Installation: anonymousIdentity.Installation,
AccountID: null,
Account: null,
},
Identity.Authenticated authenticatedIdentity => {
IdentityType: nameof(Identity.Authenticated),
InstallationID: null,
Platform: null,
Installation: null,
AccountID: authenticatedIdentity.AccountID,
Account: authenticatedIdentity.Account,
}
};
(IdentityType, InstallationID, Platform, Installation, AccountID, Account) = persisted value
}
}
}
// HACK: Hide DAL fields away from the domain model
/*DAL*/partial class SomeEntity {
internal string IdentityType { get; private set; }
public Platform? Platform { get; private set; } // compound foreign key to Installation
public string? InstallationID { get; private set; } // compound foreign key to Installation
public Installation? Installation { get; private set; } // NOTE: could be lazy-loaded
public long? AccountID { get; private set; } // foreign key to Account
public Account? Account { get; private set; } // NOTE: could be lazy-loaded
} This SHOULD be exactly what complex types can handle, by simply mapping ie: in future EF version: modelBuilder.Entity<SomeEntity>()
.ComplexProperty( _ => _.Identity )
.HasDiscriminator<string>( "IdentityType" ) public abstract record Identity {
public record Anonymous( string InstallationID, Platform Platform, Installation? Installation ) : Identity;
public record Authenticated( long AccountID, Account? Account ) : Identity;
}
public class SomeEntity {
// ... other state of the entity
// NOTE: complex type hierarchy
public Identity Identity { get; private set; }
} (presumably with some extra specifiers in the DB config if you don't want to prepend 'Identity_' in front of all the 3 complex type fields above, as would be the default: POTENTIALLY DEPENDENT ISSUE: #31376 |
@roji -- I don't know if this is helpful at all, but I'll just put this here: https://gist.github.com/atrauzzi/dde4f5e92fb783cb6847ff2b1c2d6710 Seems relevant to this convo? (Thoughts welcome, especially if EF8 makes any part of it obsolete.) |
Actually, I was just trying out the Very seriously, I'm totally open to EF adapting it into a new feature if you want... |
@atrauzzi I haven't gone into your solution in-depth, but it seems like what you've done above handles polymorphism/inheritance completely outside of EF's model; as such, it's more like what EFCore.PG did to support JSON before the new ToJson() support in 8.0. Not going through EF's model means that a lot of things are impossible, since EF has no idea what information is actually being modelled inside your document. In fact, if this is the kind of thing you're looking for, why build your own thing rather than just using System.Text.Json's built-in polymorphism feature? You should be able to easily set up a value converter that just does this; of course, at that point your can't query into the document, but that would be the case with you solution as well, it seems. BTW we refer to these kinds of schemes - where EF isn't aware of the data inside the document - as "weakly-typed" JSON mapping (see #28871 which tracks these). These make sense mainly for scenarios where there really isn't any fixed data schema (i.e. the same JSON column can sometimes hold an array of Blogs, sometimes a dictionary with Customers...). We believe that actual fully dynamic scenarios are rare, difficult to work with, and EF may not be the right tool for the job. But that doesn't seem to be what you're looking for in this case - only the ability to model inheritance/polymorphism in a strongly-typed model. In any case, we do intend to support discriminator-based inheritance for complex types mapped to JSON - but there's lots of competing priorities. |
@roji -- I actually am using edit: I think it's because the parent has to define all of its children. So I couldn't cross assemblies without creating circular references. another edit: This is probably a good time to mention again how features don't always release fully baked and it forces devs into what look like self-inflicted workarounds -- but are actually necessary to make a feature useful... And yeah, you got it. I don't necessarily want completely unpredictable data. But if I have an Is there a "medium-typed" or discriminator based JSON feature being tracked? |
I get what you're saying; but this is where the "half-baked" idea sometimes goes a bit too far for me - various people have various needs, and everyone obviously considers the thing they're missing as vital, making the feature "half-baked". Specifically, saying that complex types as a feature "isn't useful" is because it doesn't yet support inheritance is a bit much; many people can use it who don't require inheritance (just as they've been using owned types for many years without inheritance being supported). EF is a big, complicated project, with many intersecting features; it's simply not feasible to release something that supports the entire matrix of possibilities in day one. Nor do I think would that be desirable: if we were bent on making everything always work with everything, we'd end up wasting a lot of time on various intersections which nobody needs or cares about. So releasing a feature without the entire "matrix" is also a way to get it out there, and gather feedback on what people actually need (and how badly), rather than doing everything up-front always. Just for fun, we discussed the theoretical possibility of supporting discriminator-based inheritance for complex types without JSON, i.e. when the complex type is just mapped to columns in the principal's table. That's of course technically possible, and not dissimilar to regular TPH mapping (some columns would just be unused depending on the type being represented). Would it be useful for us to spend on time on it? Probably not. Is there someone out there that needs it and would consider complex types incomplete without it? For sure.
What do you mean by this? Supporting mapping complex types to JSON is tracked by #31252, and allowing discriminator-based inheritance for that is tracked by this issue. |
Yup, very fair, and I know. Although when it comes to things like polymorphic navigations and polymorphic JSON, there might be some blind spots. Waiting for people to tell you what they want isn't always the best because oftentimes, you just don't hear back -- or worse... people hack it up worse than what I showed you above and suffer in silence. 🥀
And I think this is where I feel like things tend to get too defensive to service the plea "how can we possibly know anything?" 🤷♂️ . ....Some applied intuition on how certain features will be used however would go a really long way. With release cycles that are so wide, missing what looks like a "corner", but is actually "a whole room" 😅 could mean something goes effectively unused for an entire release. Happy to be part of the solution though, give me MVP and I'll start offering early design suggestions. I always manage to find the "most popular corner cases"! 😆
Ah thank you, upvoted and subscribed! I might post a couple of questions or notes in there as well. And to my earlier point, I wouldn't have even known what to search for to find that issue to upvote. Further reinforcing the inconsistency of "waiting for people to tell us with upvotes" for prioritization. Yes, you likely have other signals, but without the link you provided, I'd have fallen into neither demographic. I would not be surprised if the vast majority of EF Core users are in a similar situation. |
I don't think we ever really do that.
To be clear, I don't think anyone doubted we'll do this - at least for me it's really a matter of when; implementing complex types was already enough work that we could deliver both JSON mappings and inheritance in the same release. That doesn't make something half-baked: lots of people are using complex types successfully; if you require inheritance, don't use it yet. Having said all that, I do believe this is one of our higher priorities in general and I really hope we get it into 9! |
I mean, I've facilitated it myself today. But hopefully it will be easy to switch over to any official flavour. 🙏
I'll be there!! 😄 |
@roji -- You mentioned that you don't follow the approach above. When you define a parent type, The problem with that approach is that this inadvertently implies that all the types have to be in the same assembly. If I define a type that is to "be a polymorph" in another assembly, configuring it would create a circular reference. By doing what I've done above with the |
Cross-assembly hierarchies are explicitly addressed as a use-case for the contract model (docs) - that would seem to be the recommended way to do that (in any case, I'd go that way instead of rolling my own thing with a registry etc.). But in any case, this issue isn't about proper use of System.Text.Json deserialization - and in any case, if it works for you and you're happy, by all means... |
I think that's effectively what I did, although I have the registry to optimize it a bit up front and so that I can manage the discriminators separately. I think it's just worth calling out this use case for any future EF Core implementation. Be aware that people are going to want to be able to define parent and child types separately. Minimizing the amount of customization to accomplish this would be super nice. |
Ooooo, care to share your solution? Just started considering use of JsonColumns and the ability to do polymorphic deserialization/querying is all that's holding me back. |
Only TPH would be supported
Related to #9630
The text was updated successfully, but these errors were encountered: