Allow SQLServer Sequence/HiLo on non-key properties

Closes #29758

Author: roji

src/EFCore.SqlServer/Infrastructure/Internal/SqlServerModelValidator.cs

@@ -44,7 +44,6 @@ public override void Validate(IModel model, IDiagnosticsLogger<DbLoggerCategory.
 
         ValidateDecimalColumns(model, logger);
         ValidateByteIdentityMapping(model, logger);
-        ValidateNonKeyValueGeneration(model, logger);
         ValidateTemporalTables(model, logger);
     }
 
@@ -113,34 +112,6 @@ protected virtual void ValidateByteIdentityMapping(
         }
     }
 
-    /// <summary>
-    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
-    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
-    ///     any release. You should only use it directly in your code with extreme caution and knowing that
-    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
-    /// </summary>
-    protected virtual void ValidateNonKeyValueGeneration(
-        IModel model,
-        IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
-    {
-        foreach (var entityType in model.GetEntityTypes())
-        {
-            foreach (var property in entityType.GetDeclaredProperties()
-                         .Where(
-                             p => (p.GetValueGenerationStrategy() == SqlServerValueGenerationStrategy.SequenceHiLo
-                                     || p.GetValueGenerationStrategy() == SqlServerValueGenerationStrategy.Sequence)
-                                 && ((IConventionProperty)p).GetValueGenerationStrategyConfigurationSource() != null
-                                 && !p.IsKey()
-                                 && p.ValueGenerated != ValueGenerated.Never
-                                 && (!(p.FindAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy) is IConventionAnnotation strategy)
-                                     || !ConfigurationSource.Convention.Overrides(strategy.GetConfigurationSource()))))
-            {
-                throw new InvalidOperationException(
-                    SqlServerStrings.NonKeyValueGeneration(property.Name, property.DeclaringEntityType.DisplayName()));
-            }
-        }
-    }
-
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs

@@ -278,9 +278,6 @@
   <data name="NoInitialCatalog" xml:space="preserve">
     <value>The database name could not be determined. To use 'EnsureDeleted', the connection string must specify 'Initial Catalog'.</value>
   </data>
-  <data name="NonKeyValueGeneration" xml:space="preserve">
-    <value>The property '{property}' on entity type '{entityType}' is configured to use 'SequenceHiLo' value generator, which is only intended for keys. If this was intentional, configure an alternate key on the property, otherwise call 'ValueGeneratedNever' or configure store generation for this property.</value>
-  </data>
   <data name="NoSavepointRelease" xml:space="preserve">
     <value>SQL Server does not support releasing a savepoint.</value>
   </data>

src/EFCore.SqlServer/Properties/SqlServerStrings.resx

@@ -55,8 +55,7 @@ public virtual IModel Initialize(
         bool designTime = true,
         IDiagnosticsLogger<DbLoggerCategory.Model.Validation>? validationLogger = null)
     {
-        if (model is Model mutableModel
-            && !mutableModel.IsReadOnly)
+        if (model is Model { IsReadOnly: false } mutableModel)
         {
             lock (SyncObject)
             {

src/EFCore/Infrastructure/ModelRuntimeInitializer.cs

@@ -956,6 +956,54 @@ await Test(
 """);
     }
 
+    [ConditionalFact]
+    public virtual async Task Add_column_sequence()
+    {
+        await Test(
+            builder => builder.Entity("People").Property<string>("Id"),
+            builder => { },
+            builder => builder.Entity("People").Property<int>("SequenceColumn").UseSequence(),
+            model =>
+            {
+                var table = Assert.Single(model.Tables);
+                var column = Assert.Single(table.Columns, c => c.Name == "SequenceColumn");
+
+                // Note: #29863 tracks recognizing sequence columns as such
+                Assert.Equal("(NEXT VALUE FOR [PeopleSequence])", column.DefaultValueSql);
+            });
+
+        AssertSql(
+"""
+CREATE SEQUENCE [PeopleSequence] START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE NO CYCLE;
+""",
+            //
+"""
+ALTER TABLE [People] ADD [SequenceColumn] int NOT NULL DEFAULT (NEXT VALUE FOR [PeopleSequence]);
+""");
+    }
+
+    [ConditionalFact]
+    public virtual async Task Add_column_hilo()
+    {
+        await Test(
+            builder => builder.Entity("People").Property<string>("Id"),
+            builder => { },
+            builder => builder.Entity("People").Property<int>("SequenceColumn").UseHiLo(),
+            _ =>
+            {
+                // Reverse-engineering of hilo columns isn't supported
+            });
+
+        AssertSql(
+"""
+CREATE SEQUENCE [EntityFrameworkHiLoSequence] START WITH 1 INCREMENT BY 10 NO MINVALUE NO MAXVALUE NO CYCLE;
+""",
+            //
+"""
+ALTER TABLE [People] ADD [SequenceColumn] int NOT NULL DEFAULT 0;
+""");
+    }
+
     public override async Task Alter_column_change_type()
     {
         await base.Alter_column_change_type();

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

@@ -143,6 +143,51 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
         }
     }
 
+    [ConditionalFact]
+    public void Insert_with_non_key_sequence()
+    {
+        using var testStore = SqlServerTestStore.CreateInitialized(DatabaseName);
+        using (var context = new BlogContextNonKeySequence(testStore.Name))
+        {
+            context.Database.EnsureCreatedResiliently();
+
+            context.AddRange(
+                new Blog { Name = "One Unicorn" }, new Blog { Name = "Two Unicorns" });
+
+            context.SaveChanges();
+        }
+
+        using (var context = new BlogContextNonKeySequence(testStore.Name))
+        {
+            var blogs = context.Blogs.OrderBy(e => e.Id).ToList();
+
+            Assert.Equal(1, blogs[0].Id);
+            Assert.Equal(1, blogs[0].OtherId);
+            Assert.Equal(2, blogs[1].Id);
+            Assert.Equal(2, blogs[1].OtherId);
+        }
+    }
+
+    public class BlogContextNonKeySequence : ContextBase
+    {
+        public BlogContextNonKeySequence(string databaseName)
+            : base(databaseName)
+        {
+        }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            base.OnModelCreating(modelBuilder);
+
+            modelBuilder.Entity<Blog>(
+                eb =>
+                {
+                    eb.Property(b => b.OtherId).UseSequence();
+                    eb.Property(b => b.OtherId).ValueGeneratedOnAdd();
+                });
+        }
+    }
+
     [ConditionalFact]
     public void Insert_with_default_value_from_sequence()
     {

test/EFCore.SqlServer.FunctionalTests/SqlServerValueGenerationScenariosTest.cs

@@ -614,15 +614,6 @@ public void Passes_for_non_key_identity_on_model()
         Validate(modelBuilder);
     }
 
-    [ConditionalFact]
-    public void Detects_non_key_SequenceHiLo()
-    {
-        var modelBuilder = CreateConventionModelBuilder();
-        modelBuilder.Entity<Dog>().Property(c => c.Type).UseHiLo();
-
-        VerifyError(SqlServerStrings.NonKeyValueGeneration(nameof(Dog.Type), nameof(Dog)), modelBuilder);
-    }
-
     [ConditionalFact]
     public void Passes_for_non_key_SequenceHiLo_on_model()
     {
@@ -635,15 +626,6 @@ public void Passes_for_non_key_SequenceHiLo_on_model()
         Validate(modelBuilder);
     }
 
-    [ConditionalFact]
-    public void Detects_non_key_KeySequence()
-    {
-        var modelBuilder = CreateConventionModelBuilder();
-        modelBuilder.Entity<Dog>().Property(c => c.Type).UseSequence();
-
-        VerifyError(SqlServerStrings.NonKeyValueGeneration(nameof(Dog.Type), nameof(Dog)), modelBuilder);
-    }
-
     [ConditionalFact]
     public void Passes_for_non_key_KeySequence_on_model()
     {