Fix to #26166 - Idempotent migration script generates problematic tem…

…poral table alter statement

Problem was that for idempotent migration scripts that convert non-temporal table to temporal we need to wrap the period switching statement in exec. Otherwise sql parser(?) complains that the command is invalid because period already exists. This is a false negative - the statement will not actually execute if this migration has already been applied. Exec fixes the problem since the statement inside it can't be parsed. This is how we deal with similar issues in other places in the code.

Fixes #26166

src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs

@@ -71,7 +71,7 @@ public override IReadOnlyList<MigrationCommand> Generate(
             _operations = operations;
             try
             {
-                return base.Generate(RewriteOperations(operations, model), model, options);
+                return base.Generate(RewriteOperations(operations, model, options), model, options);
             }
             finally
             {
@@ -2357,7 +2357,8 @@ private static bool HasDifferences(IEnumerable<IAnnotation> source, IEnumerable<
 
         private IReadOnlyList<MigrationOperation> RewriteOperations(
             IReadOnlyList<MigrationOperation> migrationOperations,
-            IModel? model)
+            IModel? model,
+            MigrationsSqlGenerationOptions options)
         {
             var operations = new List<MigrationOperation>();
 
@@ -2697,18 +2698,29 @@ void DisablePeriod(string table, string? schema, string periodStartColumnName, s
 
             void EnablePeriod(string table, string? schema, string periodStartColumnName, string periodEndColumnName)
             {
+                var addPeriodSql = new StringBuilder()
+                    .Append("ALTER TABLE ")
+                    .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(table, schema))
+                    .Append(" ADD PERIOD FOR SYSTEM_TIME (")
+                    .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(periodStartColumnName))
+                    .Append(", ")
+                    .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(periodEndColumnName))
+                    .Append(")")
+                    .ToString();
+
+                if (options.HasFlag(MigrationsSqlGenerationOptions.Idempotent))
+                {
+                    addPeriodSql = new StringBuilder()
+                    .Append("EXEC(N'")
+                    .Append(addPeriodSql.Replace("'", "''"))
+                    .Append("')")
+                    .ToString();
+                }
+
                 operations.Add(
                     new SqlOperation
                     {
-                        Sql = new StringBuilder()
-                            .Append("ALTER TABLE ")
-                            .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(table, schema))
-                            .Append(" ADD PERIOD FOR SYSTEM_TIME (")
-                            .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(periodStartColumnName))
-                            .Append(", ")
-                            .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(periodEndColumnName))
-                            .Append(")")
-                            .ToString()
+                        Sql = addPeriodSql
                     });
 
                 operations.Add(

test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs

@@ -1163,6 +1163,43 @@ public virtual void UpdateData_generates_exec_when_idempotent()
                 @$"EXEC(N'UPDATE [Table1] SET [Column1] = 2
 WHERE [Id] = 1;
 SELECT @@ROWCOUNT');
+");
+        }
+
+        [ConditionalFact]
+        public virtual void Converting_table_to_temporal_idempotent()
+        {
+            Generate(
+                builder => builder.Entity(
+                    "Customer", e =>
+                    {
+                        e.Property<int>("Id").ValueGeneratedOnAdd();
+                        e.Property<string>("Name");
+                        e.Property<DateTime>("PeriodStart").ValueGeneratedOnAddOrUpdate();
+                        e.Property<DateTime>("PeriodEnd").ValueGeneratedOnAddOrUpdate();
+                        e.HasKey("Id");
+                    }).HasAnnotation(RelationalAnnotationNames.Schema, "dbo"),
+            migrationBuilder => migrationBuilder
+                    .AlterTable("Customer")
+                    .Annotation(SqlServerAnnotationNames.IsTemporal, true)
+                    .Annotation(SqlServerAnnotationNames.TemporalHistoryTableName, "CustomerHistory")
+                    .Annotation(SqlServerAnnotationNames.TemporalPeriodStartColumnName, "PeriodStart")
+                    .Annotation(SqlServerAnnotationNames.TemporalPeriodEndColumnName, "PeriodEnd"),
+                MigrationsSqlGenerationOptions.Idempotent);
+
+            AssertSql(
+                @"EXEC(N'ALTER TABLE [Customer] ADD PERIOD FOR SYSTEM_TIME ([PeriodStart], [PeriodEnd])')
+GO
+
+ALTER TABLE [Customer] ALTER COLUMN [PeriodStart] ADD HIDDEN
+GO
+
+ALTER TABLE [Customer] ALTER COLUMN [PeriodEnd] ADD HIDDEN
+GO
+
+DECLARE @historyTableSchema sysname = SCHEMA_NAME()
+EXEC(N'ALTER TABLE [Customer] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + '].[CustomerHistory]))')
+
 ");
         }