SYSTEM_VERSIONING = OFF in EF8 migration causes SYSTEM_VERSIONING is not turned ON in EF9

[SimonCropp]

We currently target EF8.

We have an existing migration

public partial class VirtualProgramEvent : Migration
{
    /// <inheritdoc />
    protected override void Up(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.Sql("""
                             ALTER TABLE dbo.ProgramEvents
                             SET(SYSTEM_VERSIONING = OFF)
                             """);

        migrationBuilder.AddColumn<bool>(name: "Virtual",
            table: "_ProgramEvents_History",
            type: "bit",
            defaultValue: false,
            nullable: true);

        migrationBuilder.AddColumn<bool>(
                name: "Virtual",
                table: "ProgramEvents",
                type: "bit",
                nullable: false,
                computedColumnSql: "CAST(CASE WHEN (Location IS NULL AND OfficialLocationId IS NULL) THEN 1 ELSE 0 END AS BIT)",
                stored: true)
            .Annotation("SqlServer:IsTemporal", true)
            .Annotation("SqlServer:TemporalHistoryTableName", "_ProgramEvents_History")
            .Annotation("SqlServer:TemporalHistoryTableSchema", null)
            .Annotation("SqlServer:TemporalPeriodEndColumnName", "PeriodEnd")
            .Annotation("SqlServer:TemporalPeriodStartColumnName", "PeriodStart");

        migrationBuilder.Sql("""
                             ALTER TABLE dbo.ProgramEvents
                             SET(SYSTEM_VERSIONING = ON (HISTORY_TABLE = dbo._ProgramEvents_History))
                             """);
    }

    /// <inheritdoc />
    protected override void Down(MigrationBuilder migrationBuilder) =>
        migrationBuilder.DropColumn(
                name: "Virtual",
                table: "ProgramEvents")
            .Annotation("SqlServer:IsTemporal", true)
            .Annotation("SqlServer:TemporalHistoryTableName", "_ProgramEvents_History")
            .Annotation("SqlServer:TemporalHistoryTableSchema", null)
            .Annotation("SqlServer:TemporalPeriodEndColumnName", "PeriodEnd")
            .Annotation("SqlServer:TemporalPeriodStartColumnName", "PeriodStart");
}

note that it manipulates SYSTEM_VERSIONING.

This migration works fine on EF8.

once i update to EF9 i get

Microsoft.Data.SqlClient.SqlException : SYSTEM_VERSIONING is not turned ON for table 'template.dbo.ProgramEvents'.
Data:
  HelpLink.ProdName: Microsoft SQL Server
  HelpLink.ProdVer: 16.00.4155
  HelpLink.EvtSrc: MSSQLServer
  HelpLink.EvtID: 13591
  HelpLink.BaseHelpUrl: https://go.microsoft.com/fwlink
  HelpLink.LinkId: 20476
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, SqlCommand command, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlCommand.InternalEndExecuteNonQuery(IAsyncResult asyncResult, Boolean isInternal, String endMethod)
   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryInternal(IAsyncResult asyncResult)
   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryAsync(IAsyncResult asyncResult)
   at Microsoft.Data.SqlClient.SqlCommand.<>c.<InternalExecuteNonQueryAsync>b__193_1(IAsyncResult asyncResult)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
--- End of stack trace from previous location ---
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteNonQueryAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteNonQueryAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteNonQueryAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteAsync(IReadOnlyList`1 migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, Boolean beginTransaction, Boolean commitTransaction, Nullable`1 isolationLevel, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteAsync(IReadOnlyList`1 migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, Boolean beginTransaction, Boolean commitTransaction, Nullable`1 isolationLevel, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteNonQueryAsync(IReadOnlyList`1 migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, Boolean commitTransaction, Nullable`1 isolationLevel, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.MigrateImplementationAsync(DbContext context, String targetMigration, MigrationExecutionState state, Boolean useTransaction, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.MigrateImplementationAsync(DbContext context, String targetMigration, MigrationExecutionState state, Boolean useTransaction, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.<>c.<<MigrateAsync>b__22_1>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.MigrateAsync(String targetMigration, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.MigrateAsync(String targetMigration, CancellationToken cancellationToken)
   at MinisterialProgram.Data.DbInitializer.Initialize(MinProgramDb context) in C:\Code\MinisterialProgramDataModel\src\DataModel\DbInitializer.cs:line 11

if i debug unto it i can see in MigrationCommandExecutor.ExecuteAsync that there are duplicate

note the newline differences. so the one from our migration is first with the command split over two lines.

i looked through the history and found this change e3e7a5b
it might be the cause

I dont have the time to create a minimal repro, and my plan is to squash the migrations to make the problem go away.
But i figured i would raise an issue and see if you want any more context while i can still debug into at and give u more information

[SimonCropp]

sigh. this SET(SYSTEM_VERSIONING = OFF) pattern is in 27 of our migrations across multiple projects. looks like we will not be updating to EF9 until this is fixed

[maumar]

This is by design. EF8 had a bug, where for some temporal migration operations we were not switching off versioning (and then switching it back on again at the end). e3e7a5b addressed those issues and now EF properly generates these calls, however in this case, manual

        migrationBuilder.Sql("""
                             ALTER TABLE dbo.ProgramEvents
                             SET(SYSTEM_VERSIONING = OFF)
                             """);

was added by the user to work around the bug in EF8,

Now that EF9 manages the versioning correctly, it tries to disable versioning, but it already has been disabled by this manual step. We can't reliably parse user's custom migration code to see if they happen to disable versioning, period or do some other workarounds.

Only thing we could reliably do it to make the versioning ON/OFF operation that we generate idempotent, but that incurs cost on everyone to mitigate this edge case. If we get more reports of this issue we might consider doing that, but for now we are ok with the behavior as is, and suggest fixing the migrations manually (which should just be deleting the custom versioning ON and OFF statements)

[gbiellem]

@maumar The above approach that @SimonCropp outlined allowed adding a computed column to an existing temporal table, Since the move to EF9 we are finding it impossible to so in a migration. Could you please should how this can be done now?

[SimonCropp]

@maumar or better yet. add the "how this can be done" to the release notes given it is a breaking change

[SimonCropp]

@ajcvickers any thoughts on the last two comments?

[ajcvickers]

@SimonCropp @gbiellem If this still doesn't work after removing your previous workarounds, then please attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

[maumar]

@SimonCropp @gbiellem we do have tests adding computed column to the temporal table, so those should be working once the manual versioning manipulation is removed. Is that not what you see in your scenario?

basically this migration should execute just fine of EF9:

    protected override void Up(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.AddColumn<bool>(name: "Virtual",
            table: "_ProgramEvents_History",
            type: "bit",
            defaultValue: false,
            nullable: true);

        migrationBuilder.AddColumn<bool>(
                name: "Virtual",
                table: "ProgramEvents",
                type: "bit",
                nullable: false,
                computedColumnSql: "CAST(CASE WHEN (Location IS NULL AND OfficialLocationId IS NULL) THEN 1 ELSE 0 END AS BIT)",
                stored: true)
            .Annotation("SqlServer:IsTemporal", true)
            .Annotation("SqlServer:TemporalHistoryTableName", "_ProgramEvents_History")
            .Annotation("SqlServer:TemporalHistoryTableSchema", null)
            .Annotation("SqlServer:TemporalPeriodEndColumnName", "PeriodEnd")
            .Annotation("SqlServer:TemporalPeriodStartColumnName", "PeriodStart");
    }

you can also remove all the temporal annotations from AddColumn - we don't use them anymore, we only use annotations on table-modifying operations. But things should work either with or without them.

Existing tests:

efcore/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs

Line 7141 in ac5bd7b

public virtual async Task Add_nullable_computed_column_to_temporal_table()

efcore/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs

Line 7205 in ac5bd7b

public virtual async Task Add_non_nullable_computed_column_to_temporal_table()

Here is the issue #27844 and here is the fix b96c768 (fix comment provides a bunch of context)