dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.5k stars 3.13k forks source link

Relational: TPT inheritance mapping pattern #2266

Closed satyajit-behera closed 4 years ago

satyajit-behera commented 9 years ago

Even if TPT is considered slow, it is a boon for many real world business scenarios. If implemented properly and well thought of, TPT is not slow for a given need in most of the cases.

If possible, optimization can be done to TPT. But its a very important feature for EF to be accepted for developing DDD applications.

The other method, Composition over Inheritance does not look viable since we cannot use interface property to create our model. Binding to a concrete type takes away the flexibility to customize the models. TPT makes the customization very easy and flexible.

rowanmiller commented 9 years ago

Update: we are planning to implement TPT. This comment is from 2015. Please read the rest of the thread, not just this comment.

~The feeling from our team is that TPT is generally an anti-pattern and results in significant performance issues later on. While enabling it may make some folks "happier" to start with it ultimately just leads to issues. We are willing to consider it though, so we're leaving this open and will consider it based on the feedback we get.~

satyajit-behera commented 9 years ago

Thanks. Issue is the absence of any alternative to build an OO application. Even Composition using interfaces does not look viable for now.

rowanmiller commented 9 years ago

We are in the process of implementing TPH at the moment, so you will be able to persist an inheritance hierarchy to a single table in the near future.

satyajit-behera commented 9 years ago

But it does not solve the main issue of extending a table. TPT becomes a necessity in practical business scenario. Otherwise, it may become a table with 100s of fields in it.

anpete commented 9 years ago

@satyajit-behera TPT is an inheritance mapping pattern. Are you referring to "Entity Splitting". i.e. Having a single entity mapped to more than one table?

satyajit-behera commented 9 years ago

@anpete I am referring to One table per inherited entity type.

GSPP commented 9 years ago

TPT can be important for performance. Think of huge tables. Usually, you want only the absolutely required fields in such a table. TPH has column bloat, TPT is lean. Also, with TPT database statistics can be specialized for each type. This is important because different types can have vastly different data distributions.

TPT is quite an important pattern and I don't see why it would be considered an anti-pattern. An anti-pattern is a thing that is almost always the wrong choice. This does not seem to hold here.

santanaguy commented 9 years ago

@GSPP i agree with you. Having TPT is a must for an ORM and i think it was recurrent solution for a lot of problems in EF6 in the real world. I used it in my models a lot. One should be careful with the tools he uses, but that doesn't mean he shouldn't have those tools available. Both types of inheritance should be present in EF Core. Please include this scenario in the 7.0 release

satyajit-behera commented 9 years ago

@rowanmiller I hope you will prioritize and include the TPT inheritance mapping for EF 7. All of us can discuss if some specific feature make it slow and get a workaround for the same. But the feature itself is very useful in business applications. Please include this in EF Core release. Thanks a lot.

ToddThomson commented 8 years ago

:+1: TPT inheritance may have some performance issues, but the design clarity it provides for real world scenarios outweighs them. I also do not feel that TPT inheritance is an "anti-pattern".

konstantin999 commented 8 years ago

@rowanmiller TPT inheritance is the reason why we can not use EF in our projects :( It would be nice if it will included in EF Core. http://data.uservoice.com/forums/72025-entity-framework-feature-suggestions/suggestions/1015337-tpt-table-per-type-inheritance-performance https://entityframework.codeplex.com/workitem/2332

konstantin999 commented 8 years ago

For example, in DO this problem was solved by including a TypeId field in all types. http://help.x-tensive.com/Default.aspx##DataObjects.Net%20v3.9/html/P_DataObjects_NET_DataObject_TypeID.htm

AndyNewland commented 8 years ago

Agree 100% with @ToddThomson. This is another strong vote to add this feature ASAP. I think that TPH can be viewed as a SQL Server anti-pattern once the number of columns becomes unmanageable from a statistics and (more importantly) indexing viewpoint.

satyajit-behera commented 8 years ago

@rowanmiller Hi, now you can consider including this feature asap. There is really no alternative to this to model real world objects in business application.

jimmymain commented 8 years ago

I agree with andy, if the EF team honestly believe that they cannot do this because it's too slow, we would have to look to alternate products. Entity framework becomes pretty useless for our real world business scenarios. Our SQL team will never endorse de-normalised tables because of EF.

I think some evidence is necessary before labeling this an anti pattern.

ToddThomson commented 8 years ago

Preface: This is not a knock against the EF Core team and their design goals. I really just want to know what I'm going to be working with if I choose to go with EF Core over EF6 for the next 12 months or so.

I think that there are real world scenarios that benefit from either TPH, TPC or TPT inheritance. I feel that the EF Core team needs to state clearly that TPC and/or TPT are going to be part of the future EF Core implementation or perhaps only to make some people happier down the road given enough community votes and when time permits. If TPC or TPT inheritance is not a high priority ( implementation if any, most likely later in 2016 ), then composition is the most likely alternative for those who want a normalized database schema.

For me personally, I believe moving to ASP.NET Core and EF Core is a port of my MVC 5, EF 6 application and I am willing to try different approaches. If I can't get EF Core to work then EF6 is still an option ( although the ASP.NET 5 Identity package requires EF Core and EF 6 code first migrations are issues ).

jimmymain commented 8 years ago

A clear statement of intent would assist us in making sound decisions on behalf of our clients.

rowanmiller commented 8 years ago

TPT is definitely out for our November RC release (purely due to time constraints). Clearing up milestone so that we can re-discuss in triage and see if we can make a clear yes/no on our intent to support the feature in the future (my feeling is that we have enough feedback that folks want it).

ToddThomson commented 8 years ago

Thank-you for the feedback @rowanmiller .

satyajit-behera commented 8 years ago

Thanks @rowanmiller . Hope you will take this item with priority, since an alternative is not available to implement the concepts possible with TPT. Critical and Important. I am sure many enterprise level applications will be using this.

thomas-darling commented 8 years ago

Another vote for TPC and TPH - we absolutely need it and will look for another ORM if it's not supported. I agree that query performance is not great, but it is very important if we want a nicely normalized domain model. We can always create denormalized tables or views optimized for specific queries, but our single source of truth must be nicely structured.

The fact that inexperienced developers tend to abuse a feature, that does not mean the feature is not a criitical requirement for those of us who know what we are doing. It just means that people should learn about their tools before using them and that documentation should be improved to clearly state performance characteristics.

rowanmiller commented 8 years ago

@thomas-darling TPH is already being implemented for our RC release in November (it mostly works now) and TPC is on our backlog (https://github.com/aspnet/EntityFramework/issues/3170). This issue is specifically about TPT and whether that pattern needs to be supported.

satyajit-behera commented 8 years ago

@rowanmiller Difficult to get rid of TPT without any alternative implementation for the same. One common "Id" shared across all inherited classes. Base class should allow typecasting to inherited class. And all reside in different tables.

rowanmiller commented 8 years ago

Triage: We do plan to do this based on feedback from the community. Purely due to time constraints it won't be in the initial release (just TPH will be in).

AndyNewland commented 8 years ago

Thanks @rowanmiller, good news

nteague22 commented 8 years ago

I know as any release draws near, things can get hectic... But I at least want to take a moment and give you guys a round of applause. You and your team have stepped up in most every manner, continue to impress on response time and feedback, and I feel personally have shown a lot of poise. To many of my peers, the pre existing entity framework was one of the old and sluggish libs still around from Microsoft less nimble days, and your team has not only made it better and community first, but has undergone the scrutiny of developing in the open here, which increases pressure across the board. There's s reason many restaurants don't do open kitchens and similar businesses follow suit -- scrutiny is tough, especially from non empathetic view point. I will shut it down now, but I just wanted to say thanks, I have followed you guys since beta-2 on this, and am psyched at what new levels of uml branching to rapid ddl design I can achieve from just what I've used already!

satyajit-behera commented 8 years ago

Thanks @rowanmiller for putting this again as a backlog item. I hope, this will get a place in the final release.

sanalbakis commented 8 years ago

I see on the most comments, everyone would like to have the TPT, we also. I hope, it comes soon.

satyajit-behera commented 8 years ago

@rowanmiller Congratulations for the first release of EF Core as a Release Candidate. Thank you all to build such a wonderful product. I hope that the TPT implementation will also start soon. Not able to think of moving my existing projects to the newer version of asp.net waiting for this feature. Thanks a lot.

vincedan commented 8 years ago

We don't think that TPT inheritance is an "anti-pattern", on the countrary, it is the elegant way for "Conceptual Modeling" of datameta. without TPT, we can not move system to EF Core and only discard it. to refer performance issue, I think that developers should consider banlance and compromising. but as new product, you should provide this very important function.

rowanmiller commented 8 years ago

@vincedan - you may have already seen this from the comments... but we have agreed, based on feedback, that TPT should be added to EF Core.

satyajit-behera commented 8 years ago

@rowanmiller Thanks a lot :+1:

Sbiligudy commented 8 years ago

@rowanmiller Thanks a lot, TPT is very usefull!

vincedan commented 8 years ago

@rowanmiller very glad to hear that your guys have already added it in backlog. Thanks a lot.

sthiakos commented 8 years ago

I'm okay with Composition for TPT for now. @ToddThomson makes a very good recommendation to use composition, rather than inheritance, for TPT scenarios. The one drawback is that our object model now requires what would have been a base class to be instantiable rather than abstract, making the relational model influence our object model design.

tomqwertysdsad commented 8 years ago

Glad to see TPT will be implemented into EF7. I understand that it won't be in the first release but is there any guess to when we can expect to see it (6 months, this year)? Is there a workaround or crude implementation I could use in the mean time?

rowanmiller commented 8 years ago

@tomsharratt we'll be doing planning/prioritization for the post-7.0.0 releases around the time we release 7.0.0... so I don't have specific plans to share yet, but we will share them publically as soon as we have them.

sthiakos commented 8 years ago

Given

CREATE TABLE Base (
    BaseId INT NOT NULL Identity,
    BaseProperty INT NOT NULL,
    CONSTRAINT PK_Base PRIMARY KEY (BaseId)
)

CREATE TABLE Sub (
    BaseId INT NOT NULL,
    SubProperty BIGINT NOT NULL,
    CONSTRAINT PK_Sub PRIMARY KEY (BaseId),
    CONSTRAINT FK_Sub_Base FOREIGN KEY (BaseId) REFERENCES Base(BaseId)
)

TPT representation would be:

public abstract class Base
{
    [Key, DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public int BaseId { get; set; }

    public int BaseProperty { get; set; }

    protected Base()
    {
    }
}

public class Sub : Base
{
    public long SubProperty { get; set; }
}

The Composition (alternative) representation is:

public /*abstract*/ class Base
{
    [Key, DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public int BaseId { get; set; }

    public int BaseProperty { get; set; }
}

public class Sub
{
    [Key]
    public int BaseId { get; set; }
    public Base Base { get; set; }

    public long SubProperty { get; set; }
}
crozone commented 8 years ago

@sthiakos does this work at all with polymorphism? To do a query on this, it appears that one needs to perform a separate query for every subclass that inherits Base

Eg. Context.SubAs.Where(s => s.Base.BaseProperty == "Something"); Context.SubBs.Where(s => s.Base.BaseProperty == "Something"); Context.SubCs.Where(s => s.Base.BaseProperty == "Something");

And each result set would need to be dealt with separately.

sthiakos commented 8 years ago

I think you're right @crozone.

satyajit-behera commented 8 years ago

@rowanmiller Hi, hope the team will start with this feature now. Thanks.

taspeotis commented 8 years ago

Hi,

Thanks for all your hard work on EF Core. I realise it's pre-release software but the omission of TPT is disappointing.

I also find it somewhat disappointing that it's considered an anti-pattern. In the application I look after we have people operating in various contexts (users, contacts, employees). At the end of the day they're all people so I have a Person table that contains GivenName, Surname, DateOfBirth etc.

We never actually care about a person without their context (be it a user, contact or employee). So the bad queries that TPT can generate like:

SELECT ...
  FROM Person
       LEFT JOIN ( User ...
                   UNION ALL Contact ...
                   UNION ALL Employee )

Never happen.

Instead, if I wanted to do something like report on users who have recently failed to sign in (think _unitOfWork.Users.Where(u => u.LastFailedAttempt > ...)) there would be a query dispatched like:

SELECT ...
  FROM Person
       INNER JOIN User
 WHERE ...

Which is likely to be an efficient merge join between Person and User.

Our database is a bit more complicated than what EF Migrations would be ideal for. In among the tables that EF deals with there are things like user defined table types, materialized views and filtered indexes. To this end, we use SQL Server Data Tools to manage the schema.

TPT is nice because there's one place in the database for each attribute and no redundancy. (TPH has this too but at the expense of infecting my schema with its god tables and discriminator columns. TPC does not have this.)

To me TPH is an anti-pattern because of its wide tables and TPC is an anti-pattern because there's three places for me to maintain for any one attribute.

For now we use EF6. Please consider implementing TPT as soon as possible.

Again, thanks for all the hard work.

GSPP commented 8 years ago

To me TPH is an anti-pattern because of its wide tables and TPC is an anti-pattern because there's three places for me to maintain for any one attribute.

Be careful with such statements, all three forms have their uses. It depends on the requirements which is, in fact, a point you made earlier. Access patterns and storage concerns vary widely.

@taspeotis

taspeotis commented 8 years ago

Hi, thanks for reading.

Please don't read too much into that sentence: what I intended to convey that was in the application I maintain TPH and TPC appear as anti-patterns for the particular data we store.

TPH would result in wide tables. TPC would result in maintain some columns in tables in triplicate. In contrast, TPT appears as a desirable pattern. This is not necessarily true of TPH, TPC and TPT generally.

I probably should have started it with "For me..."

jimmymain commented 8 years ago

Do third normal form database design principles not contraindicate both TPC and TPH.

I agree that trade offs are made for many reasons, including expediency and efficiency, but I cannot see a scenario in which my clients database teams would sign of on either approach. They would insist on a database design consistent with TPT to comply with the principles of at least third normal form - some take it further.

GSPP commented 8 years ago

@jimmymain It does contraindicate that, yes. But the normal forms are just guidelines.

If your client is unwilling to denormalize in specific, well-reasoned cases they are causing themselves problems. This is not an issue with the three patterns. It's a lack of understanding on their part.

This insistence on following rules to the letter is a typical and understandable step in the path of most developers. As they become even more experienced they learn when to relax the rules.

sthiakos commented 8 years ago

I'd say TPC is not an anti-pattern nor contrary to 3NF. In TPC there is no repeating data within the table. "fixing" TPC would imply having one table for the entire database that is for the attribute that is to be maintained (i.e. a master lookup table)

It's tough to accept relaxing the rules (even as an experienced developer). I'm not a fan at all of TPH because it swings to much to the OOD mindset. It seems TPH is the all-encompassing solution and the alternative is a TPC-TPT combination.

mhosman commented 8 years ago

Any news about TPT in EF7? I think this is CRITICAL. I started a new project with EF7 and when I discovered the omission of TPT I had to change and back to EF6. This is dissapointing.

rowanmiller commented 8 years ago

We have committed to implementing TPT (based mostly on feedback from the community), but it will be a post v1.0.0 feature. You can see a list of the features we consider most important post v1.0.0 as part of our Roadmap https://github.com/aspnet/EntityFramework/wiki/Roadmap.

jimmymain commented 8 years ago

Hi,

That's great news - but it's a show stopper for all our clients we will all eagerly await version 2.

On Mon, 23 May 2016 at 10:10 PM Rowan Miller notifications@github.com wrote:

We have committed to implementing TPT (based mostly on feedback from the community), but it will be a post v1.0.0 feature. You can see a list of the features we consider most important post v1.0.0 as part of our Roadmap https://github.com/aspnet/EntityFramework/wiki/Roadmap.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/aspnet/EntityFramework/issues/2266#issuecomment-221081340