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.78k stars 3.19k forks source link

System.InvalidOperationException: Only root entity type should be marked as temporal. Entity type: #33119

Open rburk69-wgu-edu opened 8 months ago

rburk69-wgu-edu commented 8 months ago

I wanted to make use of the Temporal table feature to track changes for my entities but it appears to the validator is too strict for even basic inheritance scenarios. The SqlServerModelValidator ValidateTemporalTables method doesn't allow temporal tables on entities with a BaseType. That doesn't really make a lot of sense. With the code that migrations generates to change a table to Temporal I was easily able to add the Temporal feature with PeriodStart and PeriodEnd respectively. I even tried to replace the IModelValidator service with a inherited version of the SqlServerModelValidator class, overrode the method ValidateTemporalTables, and removed the if statement but somewhere deep down in the dependency chain someone hard coded the built in SqlServerModelValidator and the class that serves it is sealed so I could not replace it with my own validator. Here is a very simple sample along with the tables that are built and the code I used to add the Temporal feature after the database was created. So I believe that the validator should allow temporal tables to be built for the entity even if it has a BaseType, and also I think that the dependency chain issue should be resolved so that EF Core is as customizable as it appears.

public class Person
{
    public long Id { get; set; }
    public Guid Guid { get; set; } = Guid.NewGuid();
    public string FirstName { get; set; } = string.Empty;
    public string LastName { get; set; } = string.Empty;
}

public class Educator : Person
{
    public string Title { get; set; } = string.Empty;
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Person>(builder =>
    {
        builder
            .HasKey(b => b.Id);
        builder
            .Property(b => b.Id)
            .UseIdentityColumn();
        builder
            .Property(b => b.Guid)
            .ValueGeneratedOnAdd();
        builder
            .Property(b => b.FirstName)
            .IsRequired();
        builder
            .Property(b => b.LastName)
            .IsRequired();

        builder.ToTable(
            $"{builder.Metadata.ClrType.Name}",
            schema: "core",
            tableBuiler =>
        {
            tableBuiler.IsTemporal();
        });
    });

    modelBuilder.Entity<Educator>(builder =>
    {
        builder
            .HasBaseType<Person>();

        builder
            .Property(b => b.Title)
            .IsRequired();

        builder.ToTable(
            $"{builder.Metadata.ClrType.Name}",
            schema: "core",
            tableBuiler =>
            {
                tableBuiler.IsTemporal(); //this is the offending validation error
            });
    });
}

ALTER TABLE [core].[Educator] ADD [PeriodEnd] datetime2 NOT NULL DEFAULT '9999-12-31T23:59:59.9999999'; ALTER TABLE [core].[Educator] ADD [PeriodStart] datetime2 NOT NULL DEFAULT '0001-01-01T00:00:00.0000000'; ALTER TABLE [core].[Educator] ALTER COLUMN [PeriodStart] ADD HIDDEN ALTER TABLE [core].[Educator] ALTER COLUMN [PeriodEnd] ADD HIDDEN ALTER TABLE [core].[Educator] ADD PERIOD FOR SYSTEM_TIME ([PeriodStart], [PeriodEnd]) ALTER TABLE [core].[Educator] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [core].[EducatorHistory]))

image

maumar commented 8 months ago

temporal tables feature is currently only supported for TPH (see https://github.com/dotnet/efcore/issues/26457 for more context). We should block this in migrations until support for TPT/TPC is added.