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

EF Core 6.0 temporal tables - Add-Migration - Period property 'Comment.PeriodStart' must be a shadow property #26960

Closed
Ogglas opened this issue Dec 10, 2021 · 4 comments

Comments

@Ogglas
Copy link

Ogglas commented Dec 10, 2021

We have recently upgraded our project to Microsoft.EntityFrameworkCore 6.0.0 as this release enables SQL Server temporal tables out of the box.

https://devblogs.microsoft.com/dotnet/prime-your-flux-capacitor-sql-server-temporal-tables-in-ef-core-6-0/

https://stackoverflow.com/a/70017768/3850405

We have used temporal tables since Entity Framework Core 3.1 using custom migrations as described here:

https://stackoverflow.com/a/64776658/3850405

https://stackoverflow.com/a/64244548/3850405

Simply following the guide from devblog will of course not work since default column names are PeriodStart and PeriodEnd instead of our SysStartTime and SysEndTime. History table name does not match either.

modelBuilder
    .Entity<Comment>()
    .ToTable("Comments", b => b.IsTemporal());

Migration created:

protected override void Up(MigrationBuilder migrationBuilder)
{
    migrationBuilder.AlterTable(
        name: "Comments")
        .Annotation("SqlServer:IsTemporal", true)
        .Annotation("SqlServer:TemporalHistoryTableName", "CommentsHistory")
        .Annotation("SqlServer:TemporalHistoryTableSchema", null)
        .Annotation("SqlServer:TemporalPeriodEndColumnName", "PeriodEnd")
        .Annotation("SqlServer:TemporalPeriodStartColumnName", "PeriodStart");

    migrationBuilder.AddColumn<DateTime>(
        name: "PeriodEnd",
        table: "Comments",
        type: "datetime2",
        nullable: false,
        defaultValue: new DateTime(1, 1, 1, 0, 0, 0, 0, DateTimeKind.Unspecified))
        .Annotation("SqlServer:IsTemporal", true)
        .Annotation("SqlServer:TemporalPeriodEndColumnName", "PeriodEnd")
        .Annotation("SqlServer:TemporalPeriodStartColumnName", "PeriodStart");

    migrationBuilder.AddColumn<DateTime>(
        name: "PeriodStart",
        table: "Comments",
        type: "datetime2",
        nullable: false,
        defaultValue: new DateTime(1, 1, 1, 0, 0, 0, 0, DateTimeKind.Unspecified))
        .Annotation("SqlServer:IsTemporal", true)
        .Annotation("SqlServer:TemporalPeriodEndColumnName", "PeriodEnd")
        .Annotation("SqlServer:TemporalPeriodStartColumnName", "PeriodStart");
}

Creating a custom conversion should fix this as described below:

modelBuilder
    .Entity<Comment>()
    .ToTable("Comments", tb => tb.IsTemporal(temp =>
    {
        temp.UseHistoryTable("Comments", "History");
        temp.HasPeriodStart("SysStartTime");
        temp.HasPeriodEnd("SysEndTime");
    }));

https://coderedirect.com/questions/540979/how-can-i-use-system-versioned-temporal-table-with-entity-framework

However when doing it like this I get the following error on Add-Migration command:

Period property 'Comment.SysStartTime' must be a shadow property.

To verify there was nothing wrong with any other code I had I reverted to:

modelBuilder
    .Entity<Comment>()
    .ToTable("Comments", b => b.IsTemporal());

And then added public DateTime PeriodStart { get; set; } to Comment.

I then received the error:

Period property 'Comment.PeriodStart' must be a shadow property.

Is there any way to get around this? We use our SysStartTime as a last modified/last updated value and it works really well. Having to include it via EF.Property<DateTime>(comment, "SysStartTime")) seems very unnecessary since the column is present both in temporal table and the original table.

@jamesfera
Copy link

#26463

@Ogglas
Copy link
Author

Ogglas commented Dec 10, 2021

@jamesfera @ajcvickers Yes but is there anyway to get around this or do we have to wait for 7.0? Since we use temporal tables at the moment and can access the property SysStartTime we do not want to implement a new property for Updated/Modified just because EF Core for some reason enforces shadow property at the moment.

@ajcvickers
Copy link
Member

@Ogglas Unfortunately, there isn't any workaround for this that allows both the use of the new temporal table features, and mapping the period columns to non-shadow properties. Of course, you can continue to map the columns to non-shadow properties as you have been doing, without declaring the types as temporal. We plan to enable this in EF7, as tracked by #26463. Please make sure to vote (👍) for that issue.

@dragos-durlut
Copy link

dragos-durlut commented Jan 12, 2022

@Ogglas

Yes but is there anyway to get around this or do we have to wait for 7.0? Since we use temporal tables at the moment and can access the property SysStartTime we do not want to implement a new property for Updated/Modified just because EF Core for some reason enforces shadow property at the moment.

I've had the same issue.
I've posted a workaround here

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants