Proposed fix for Optional RestartSequenceOperation.StartValue #26560 (#…

…29346) --------- Co-authored-by: Daniel Mihaita <[email protected]>
dotnet · Mar 30, 2023 · 50b55d3 · 50b55d3
1 parent 72ab932
commit 50b55d3
Show file tree

Hide file tree

Showing 11 changed files with 88 additions and 13 deletions.
diff --git a/src/EFCore.Design/Migrations/Design/CSharpMigrationOperationGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpMigrationOperationGenerator.cs
@@ -1805,11 +1805,15 @@ protected virtual void Generate(RestartSequenceOperation operation, IndentedStri
                     .Append(Code.Literal(operation.Schema));
             }
 
-            builder
-                .AppendLine(",")
-                .Append("startValue: ")
-                .Append(Code.Literal(operation.StartValue))
-                .Append(")");
+            if (operation.StartValue.HasValue)
+            {
+                builder
+                    .AppendLine(",")
+                    .Append("startValue: ")
+                    .Append(Code.Literal(operation.StartValue.Value));
+            }
+
+            builder.Append(")");
 
             Annotations(operation.GetAnnotations(), builder);
         }

diff --git a/src/EFCore.Relational/Migrations/MigrationBuilder.cs b/src/EFCore.Relational/Migrations/MigrationBuilder.cs
@@ -1233,12 +1233,12 @@ public virtual OperationBuilder<RenameTableOperation> RenameTable(
     ///     See <see href="https://aka.ms/efcore-docs-migrations">Database migrations</see> for more information and examples.
     /// </remarks>
     /// <param name="name">The name of the sequence.</param>
-    /// <param name="startValue">The value at which the sequence will start, defaulting to 1.</param>
+    /// <param name="startValue">The value at which the sequence will start. If <see langword="null" /> (the default), the sequence restarts based on the configuration used during creation.</param>
     /// <param name="schema">The schema that contains the sequence, or <see langword="null" /> to use the default schema.</param>
     /// <returns>A builder to allow annotations to be added to the operation.</returns>
     public virtual OperationBuilder<RestartSequenceOperation> RestartSequence(
         string name,
-        long startValue = 1L,
+        long? startValue = null,
         string? schema = null)
     {
         Check.NotEmpty(name, nameof(name));

diff --git a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs
@@ -794,8 +794,16 @@ protected virtual void Generate(
         builder
             .Append("ALTER SEQUENCE ")
             .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name, operation.Schema))
-            .Append(" RESTART WITH ")
-            .Append(longTypeMapping.GenerateSqlLiteral(operation.StartValue))
+            .Append(" RESTART");
+
+        if (operation.StartValue.HasValue)
+        {
+            builder
+                .Append(" WITH ")
+                .Append(longTypeMapping.GenerateSqlLiteral(operation.StartValue.Value));
+        }
+
+        builder
             .AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
 
         EndStatement(builder);

diff --git a/src/EFCore.Relational/Migrations/Operations/RestartSequenceOperation.cs b/src/EFCore.Relational/Migrations/Operations/RestartSequenceOperation.cs
@@ -23,7 +23,7 @@ public class RestartSequenceOperation : MigrationOperation
     public virtual string? Schema { get; set; }
 
     /// <summary>
-    ///     The value at which the sequence should re-start, defaulting to 1.
+    ///     The value at which the sequence should restart. If <see langword="null" /> (the default), the sequence restarts based on the configuration used during creation.
     /// </summary>
-    public virtual long StartValue { get; set; } = 1L;
+    public virtual long? StartValue { get; set; }
 }
diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
@@ -553,8 +553,16 @@ protected override void Generate(
         builder
             .Append("ALTER SEQUENCE ")
             .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name, operation.Schema))
-            .Append(" RESTART WITH ")
-            .Append(IntegerConstant(operation.StartValue))
+            .Append(" RESTART");
+
+        if (operation.StartValue.HasValue)
+        {
+            builder
+                .Append(" WITH ")
+                .Append(IntegerConstant(operation.StartValue.Value));
+        }
+
+        builder
             .AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
 
         EndStatement(builder);

diff --git a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsSqlGeneratorTestBase.cs b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsSqlGeneratorTestBase.cs
@@ -671,6 +671,7 @@ public void UpdateDataOperation_throws_for_types_count_mismatch()
                             Values = new object[,] { { "Targaryen" } }
                         })).Message);
 
+
     [ConditionalTheory]
     [InlineData(false)]
     [InlineData(true)]
@@ -723,6 +724,20 @@ public virtual void DefaultValue_with_line_breaks_2(bool isUnicode)
             });
     }
 
+    [ConditionalTheory]
+    [InlineData(3L)]
+    [InlineData(null)]
+    public virtual void Sequence_restart_operation(long? startsAt)
+    {
+        Generate(
+            new RestartSequenceOperation
+            {
+                Name = "TestRestartSequenceOperation",
+                Schema = "dbo",
+                StartValue = startsAt
+            });
+    }
+
     private static void CreateGotModel(ModelBuilder b)
         => b.HasDefaultSchema("dbo").Entity(
             "Person", pb =>

diff --git a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs
@@ -1666,6 +1666,18 @@ public virtual Task Alter_sequence_increment_by()
                 Assert.Equal(2, sequence.IncrementBy);
             });
 
+    [ConditionalFact]
+    public virtual Task Alter_sequence_restart_with()
+        => Test(
+            builder => builder.HasSequence<int>("foo"),
+            builder => { },
+            builder => builder.HasSequence<int>("foo").StartsAt(3),
+            model =>
+            {
+                var sequence = Assert.Single(model.Sequences);
+                Assert.Equal(3, sequence.StartValue);
+            });
+
     [ConditionalFact]
     public virtual Task Drop_sequence()
         => Test(

diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs
@@ -2701,6 +2701,14 @@ public override async Task Alter_sequence_increment_by()
 """);
     }
 
+    public override async Task Alter_sequence_restart_with()
+    {
+        await base.Alter_sequence_restart_with();
+
+        AssertSql(
+            @"ALTER SEQUENCE [foo] RESTART WITH 3;");
+    }
+
     public override async Task Drop_sequence()
     {
         await base.Drop_sequence();

diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs
@@ -1157,6 +1157,17 @@ public override void DefaultValue_with_line_breaks_2(bool isUnicode)
         AssertSql(expectedSql);
     }
 
+    public override void Sequence_restart_operation(long? startsAt)
+    {
+        base.Sequence_restart_operation(startsAt);
+
+        var expectedSql = startsAt.HasValue ?
+            @$"ALTER SEQUENCE [dbo].[TestRestartSequenceOperation] RESTART WITH {startsAt};" :
+            @$"ALTER SEQUENCE [dbo].[TestRestartSequenceOperation] RESTART;";
+        AssertSql(expectedSql);
+    }
+
+
     [ConditionalFact]
     public virtual void CreateIndex_generates_exec_when_legacy_filter_and_idempotent()
     {

diff --git a/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs
@@ -1720,6 +1720,9 @@ public override Task Alter_sequence_all_settings()
     public override Task Alter_sequence_increment_by()
         => AssertNotSupportedAsync(base.Alter_sequence_increment_by, SqliteStrings.SequencesNotSupported);
 
+    public override Task Alter_sequence_restart_with()
+        => AssertNotSupportedAsync(base.Alter_sequence_restart_with, SqliteStrings.SequencesNotSupported);
+
     public override Task Drop_sequence()
         => AssertNotSupportedAsync(base.Drop_sequence, SqliteStrings.SequencesNotSupported);
 

diff --git a/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs b/test/EFCore.Sqlite.FunctionalTests/Migrations/SqliteMigrationsSqlGeneratorTest.cs
@@ -680,6 +680,12 @@ public override void UpdateDataOperation_required_args_multiple_rows()
 """);
     }
 
+    public override void Sequence_restart_operation(long? startsAt)
+    {
+        var ex = Assert.Throws<NotSupportedException>(() => base.Sequence_restart_operation(startsAt));
+        Assert.Equal(SqliteStrings.SequencesNotSupported, ex.Message);      
+    }
+
     [ConditionalFact]
     public virtual void AddPrimaryKey_throws_when_no_model()
     {