Skip to content
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

Relational: TPC inheritance mapping pattern #3170

Open
rowanmiller opened this issue Sep 18, 2015 · 31 comments

Comments

Projects
None yet
@rowanmiller
Copy link
Member

commented Sep 18, 2015

No description provided.

@wangkanai

This comment has been minimized.

Copy link

commented Oct 21, 2015

Can I make assumption that TPC is now going to be in RC? judged by the following work as expected

public class Program
{
    public void Main(string[] args)
    {
        var context = new InheritanceContext();
        var sarin = new Manager() {Name = "Sarin"};
        var siriphan = new Employee() {Name = "Siriphan", Manager = sarin};
        context.Managers.Add(sarin);
        context.Employees.Add(siriphan);
        context.SaveChanges();
    }
}

public class InheritanceContext : DbContext
{
    public DbSet<Manager> Managers { get; set; }
    public DbSet<Employee> Employees { get; set; }
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"Server=(localdb)\MSSQLLocalDB;Database=Ef7Inheritance;Trusted_Connection=True;");
    }
}

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

public class Manager : Person
{
    public Collection<Employee> Staffs { get; set; } 
}

public class Employee : Person
{
    public Manager Manager { get; set; }
}

tpc
This code is tested EntityFramework 7.0.0-beta8

@jimmymain

This comment has been minimized.

Copy link

commented Oct 21, 2015

awesome!

@rowanmiller

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2015

@wangkanai you are really just getting two separate entity types with an unmapped base type. They each have a separate key space and you won't be able to run a LINQ query on Person. So it's not really inheritance in EF, it's two separate entity types that happen to share a base type in the CLR for implementation.

@wangkanai

This comment has been minimized.

Copy link

commented Oct 21, 2015

If their are in same key space would that work well with sql server auto incremental primary key?

@rowanmiller

This comment has been minimized.

Copy link
Member Author

commented Oct 21, 2015

@wangkanai not with Identity (since that is per table) but you could use a sequence to generate values. What I really meant is that with true TPC you can't have Manager with Id = 1 and an Employee with Id = 1 since then you would have two instances of Person with Id = 1 (and EF would throw if you every tried to have this). In you setup this is possible since they aren't really in an inheritance hierarchy as far as EF is concerned.

@wangkanai

This comment has been minimized.

Copy link

commented Oct 22, 2015

@rowanmiller so if in theory we make a convention to use Id as identity key as Guid (in theory it unique), this should work right?

@weitzhandler

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2015

I'd wanna see TPT before TPC. As @rowanmiller said, @wangkanai example will be used in rare cases when you explicitly wanna keep the entities unmapped or whatever other reason I can't think of now.
But TPT is even more important than TPH, because it can save a bunch of columns generated in each table completely DRYed and less-productive for some purposes.
Lack of TPC made me start a new project in MVC5 and give up the new beta.
Will this be implemented for the RC?

@rowanmiller

This comment has been minimized.

Copy link
Member Author

commented Oct 23, 2015

@wangkanai yep that would work. Just to be clear though... with the code you listed back at the start there is no need to ensure that the keys are unique between types since EF is treating them as completely separate types. But for true TPC, yes GUID keys is one option, or a sequence for numeric values, etc.

@jimmymain

This comment has been minimized.

Copy link

commented Dec 4, 2015

@weitzhandler I agree. Our database teams design with high normal forms in mind. TPT is what's required. I am of the opinion that TPC is a weak compromise compared to TPT. I often need to query base classes, and TPT makes this easy. A classic example of this is Martin Fowlers paper on Roles. Our clients will not be moving to EF until TPT is implemented.

@weitzhandler

This comment has been minimized.

Copy link
Contributor

commented Oct 20, 2016

@jimmymain
Here is the TPT discussion arena.

The funny part is that there are many things that are more convenient in ASP.NET Core (for example Identity), but in the big picture some really crucial deal-breaking features are missing (TPT).

@frogec

This comment has been minimized.

Copy link

commented Jun 17, 2017

What is the status on this feature? This turns out to be a really important feature in our case.

@AndriySvyryd

This comment has been minimized.

Copy link
Member

commented Sep 19, 2017

The implementation could be simplified if we used a database model #8258

@sthiakos

This comment has been minimized.

Copy link

commented Nov 2, 2017

+1 For Me - Note to all that you should "thumbs up" the initial post by @rowanmiller to vote up this issue. This and TPT is how DB's are/should be modeled.

@ite-klass

This comment has been minimized.

Copy link

commented Nov 2, 2017

The roadmap lists both TPC and TPT as high priority. Sad to not see either of these tickets in the 2.1 or 3.0 milestone.

@ajcvickers

This comment has been minimized.

Copy link
Member

commented Jan 22, 2018

Note: see also #10739

@J4S0Nc

This comment has been minimized.

Copy link

commented Feb 19, 2018

Will this ever make it in? 884 days and counting...

https://days.to/18-september/2015

@VocaTinityl

This comment has been minimized.

Copy link

commented Apr 16, 2018

Any news about TPC ?

@ajcvickers

This comment has been minimized.

Copy link
Member

commented Apr 16, 2018

This issue is in the Backlog milestone. This means that it is not going to happen for the 2.1 release. We will re-assess the backlog following the 2.1 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.

@alexdrl

This comment has been minimized.

Copy link

commented May 28, 2018

@rowanmiller We're using TPC with something like #3170 (comment) but we're getting stuck trying to get one to one relationships to work. We get the following exception:

System.InvalidOperationException: A key cannot be configured on 'BloodPressure' because it is a derived type. The key must be configured on the root type 'Observation'. If you did not intend for 'Observation' to be included in the model, ensure that it is not included in a DbSet property on your context, referenced in a configuration call to ModelBuilder, or referenced from a navigation property on a type that is included in the model.

One to many relationships Works correctly, even considering that TPC is not supported in EF Core. Any idea for that?

@ajcvickers

This comment has been minimized.

Copy link
Member

commented May 29, 2018

@alexdrl If you think you are hitting a bug, then can you please open a new issue including a runnable project/solution/repo or complete code listing that exhibits the behavior?

@markusschaber

This comment has been minimized.

Copy link

commented Jun 4, 2018

PostgreSQL as an object-relational database supports inheritance on the table level (https://www.postgresql.org/docs/current/static/tutorial-inheritance.html). I don't know whether other databases also support this kind of inheritance, but it could map very well to C# as it's also single-inheritance, and thus could be exposed in EF Core. It should deliver both optimal speed and space usage in most cases, thus performing better on average than TPC, TPH, TPT or the hackish "Use a single table for the base class, and stash all additional fields from subclasses in a JSON column" way.

@roji

This comment has been minimized.

Copy link
Member

commented Jun 4, 2018

@markusschaber PostgreSQL table inheritance is already tracked by #10739.

@markusschaber

This comment has been minimized.

Copy link

commented Jun 4, 2018

@roji Thanks for the hint! Subscribing immediately :-)

@alexdrl

This comment has been minimized.

Copy link

commented Jun 5, 2018

@ajcvickers The problem was a non-mapped reference to the abstract class that was not mapped to a table, this caused EF Core to acknowledge that table, and throwing an exception because the base class was being tracked in the context.

Based in the example by @wangkanai, this would be something like:

public class Program
{
    public void Main(string[] args)
    {
        var context = new InheritanceContext();
        var sarin = new Manager() {Name = "Sarin"};
        var siriphan = new Employee() {Name = "Siriphan", Manager = sarin};
        context.Managers.Add(sarin);
        context.Employees.Add(siriphan);
        context.SaveChanges();
    }
}

public class InheritanceContext : DbContext
{
    public DbSet<Manager> Managers { get; set; }
    public DbSet<Employee> Employees { get; set; }
    public DbSet<Computer> Computers { get; set; }
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"Server=(localdb)\MSSQLLocalDB;Database=Ef7Inheritance;Trusted_Connection=True;");
    }
}

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

public class Manager : Person
{
    public Collection<Employee> Staffs { get; set; } 
}

public class Employee : Person
{
    public Manager Manager { get; set; }
}

public class Computer
{
    public string Name { get; set; }
    public Person User { get; set; }
}
@ConX-Ryan

This comment has been minimized.

Copy link

commented Jul 19, 2018

What is the timeline on this ? kind of a blocking Item for us

@AndriySvyryd

This comment has been minimized.

@Unitarian

This comment has been minimized.

Copy link

commented Jul 30, 2018

I am having "duplicate primary key values" problem and I have read somewhere that there are two ways to resolve this

  1. Use [DatabaseGenerated(DatabaseGenerationOption.None)] on base class primary key and not map it on child classes at all and manage Id values manually (which I don't want to do)

  2. Use the different initial seed for different types. Now I have 15 classes inherited from abstract BaseEntity class and I couldn't find any example of using the different initial seed for code first approach.

Can anyone please explain it to me how "different initial seed" works and is there any alternative to fix this without having to manage Id manually

Cheers

@brianjlowry

This comment has been minimized.

Copy link

commented Jul 30, 2018

It continues to sit in the backlog. We had to rewrite everything in 4.7 as this was too much, and we had concerns around other things that may not be finished yet that we would find out later in our development process.

@generik0

This comment has been minimized.

Copy link

commented Nov 5, 2018

I forgot to add the code for the IDbContextOptions implemation. Maybe something needs to be activily dispoosed here?

public abstract class DbContextOptionsBase : IDbContextOptions
    {
        protected readonly DbContextOptionsBuilder Builder = new DbContextOptionsBuilder();

        public virtual DbContextOptions GetOptions(string connectionString)
        {
            Configuring(Builder, connectionString);
            var conventionSet = GetConventionSet();
            return conventionSet == null ? Builder.Options : AddModelBuilder(conventionSet);
        }

        protected virtual void Configuring(DbContextOptionsBuilder optionsBuilder, string connectionString)
        {
            optionsBuilder.UseQueryTrackingBehavior(QueryTrackingBehavior.TrackAll);
            if (string.IsNullOrWhiteSpace(connectionString)) throw new ArgumentNullException("Connection string cannot be null");
            Configuration(optionsBuilder, connectionString.ResolveConnectionString());
        }

        // e.g. optionsBuilder.UseSqlServer(ConnectionString);
        // e.g. optionsBuilder.UseInMemoryDatabase(ConnectionString); 
        protected abstract void Configuration(DbContextOptionsBuilder optionsBuilder, string connectionString);

        protected virtual DbContextOptions AddModelBuilder(ConventionSet conventionSet)
        {
            ModelBuilder = new ModelBuilder(conventionSet);
            ModelBuilder = ModelCreating(ModelBuilder);
            if (ModelBuilder != null)
            {
                Builder.UseModel(ModelBuilder.Model);
            }
            return Builder.Options;
        }

        public ModelBuilder ModelBuilder { get; protected set; }

        protected virtual ConventionSet GetConventionSet()
        {
            return GetConventionSet<DbContext>();
        }

        protected virtual ModelBuilder ModelCreating(ModelBuilder modelBuilder)
        {
            return null;
        }

        protected virtual IEnumerable<IMutableEntityType> GetModelBuilderTypes()
        {
           return ModelBuilder.Model?.GetEntityTypes();
        }

        protected virtual void RemoveModelBuilderTypes(IEnumerable<IMutableEntityType> entityTypes)
        {
            foreach (var entityType in entityTypes)
            {
                ModelBuilder.Model?.RemoveEntityType(entityType);    
            }
        }

        protected virtual ConventionSet GetConventionSet<TDbContext>() where TDbContext : DbContext
        {
            var serviceProvider = GetServiceProviderForConventionSet().BuildServiceProvider();
            using (var serviceScope = serviceProvider.GetRequiredService<IServiceScopeFactory>().CreateScope())
            {
                using (var context = serviceScope.ServiceProvider.GetService<TDbContext>())
                {
                    return ConventionSet.CreateConventionSet(context);
                }
            }
        }

        protected abstract IServiceCollection GetServiceProviderForConventionSet();
    }
@MSiffert

This comment has been minimized.

Copy link

commented Mar 10, 2019

In the docs about breaking changes of EF Core 3.0 it says that

Starting with EF Core 3.0 and in preparation for adding TPT and TPC support in a later release

I assume that means TPT / TPC will be supported in 3.1 maybe 3.2?

@ajcvickers

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

@MSiffert It does not imply that. It just means we know it's a break we need to make, and making the break sooner is preferable to leaving it like this for longer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.