Apply fill factor when creating tables

Fixes #33269
dotnet · Mar 26, 2024 · 01d4ea9 · 01d4ea9
1 parent f99afbb
commit 01d4ea9
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 23 deletions.
diff --git a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs
@@ -241,8 +241,6 @@ protected virtual void Generate(
 
         PrimaryKeyConstraint(operation, model, builder);
 
-        KeyWithOptions(operation, builder);
-
         if (terminate)
         {
             builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
@@ -269,8 +267,6 @@ protected virtual void Generate(
 
         UniqueConstraint(operation, model, builder);
 
-        KeyWithOptions(operation, builder);
-
         builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
         EndStatement(builder);
     }
@@ -1622,6 +1618,8 @@ protected virtual void PrimaryKeyConstraint(
         builder.Append("(")
             .Append(ColumnList(operation.Columns))
             .Append(")");
+
+        KeyTraits(operation, model, builder);
     }
 
     /// <summary>
@@ -1669,6 +1667,8 @@ protected virtual void UniqueConstraint(
         builder.Append("(")
             .Append(ColumnList(operation.Columns))
             .Append(")");
+
+        KeyTraits(operation, model, builder);
     }
 
     /// <summary>
@@ -1717,12 +1717,13 @@ protected virtual void CheckConstraint(
     }
 
     /// <summary>
-    ///     Generates a SQL fragment for extra with options of a key from a
-    ///     <see cref="AddPrimaryKeyOperation" /> or <see cref="AddUniqueConstraintOperation" />.
+    ///     Generates a SQL fragment for traits of an primary key or alternate key from a <see cref="CreateTableOperation" />,
+    ///     <see cref="AddPrimaryKeyOperation" />, or <see cref="AddUniqueConstraintOperation" />.
     /// </summary>
     /// <param name="operation">The operation.</param>
+    /// <param name="model">The target model which may be <see langword="null" /> if the operations exist without a model.</param>
     /// <param name="builder">The command builder to use to add the SQL fragment.</param>
-    protected virtual void KeyWithOptions(MigrationOperation operation, MigrationCommandListBuilder builder)
+    protected virtual void KeyTraits(MigrationOperation operation, IModel? model, MigrationCommandListBuilder builder)
     {
     }
 

diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
@@ -1775,26 +1775,14 @@ protected virtual void Transfer(
         }
     }
 
-    /// <summary>
-    ///     Generates a SQL fragment for extra with options of a key from a
-    ///     <see cref="AddPrimaryKeyOperation" />, or <see cref="AddUniqueConstraintOperation" />.
-    /// </summary>
-    /// <param name="operation">The operation.</param>
-    /// <param name="builder">The command builder to use to add the SQL fragment.</param>
-    protected override void KeyWithOptions(MigrationOperation operation, MigrationCommandListBuilder builder)
+    /// <inheritdoc/>
+    protected override void KeyTraits(MigrationOperation operation, IModel? model, MigrationCommandListBuilder builder)
     {
-        var options = new List<string>();
-
         if (operation[SqlServerAnnotationNames.FillFactor] is int fillFactor)
-        {
-            options.Add("FILLFACTOR = " + fillFactor);
-        }
-
-        if (options.Count > 0)
         {
             builder
-                .Append(" WITH (")
-                .Append(string.Join(", ", options))
+                .Append(" WITH (FILLFACTOR = ")
+                .Append(fillFactor.ToString())
                 .Append(")");
         }
     }

diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs
@@ -354,6 +354,42 @@ PERIOD FOR SYSTEM_TIME([PeriodStart], [PeriodEnd])
 """);
     }
 
+    [ConditionalFact]
+    public virtual async Task Create_table_with_fill_factor()
+    {
+        await Test(
+            _ => { },
+            builder =>
+            {
+                builder.Entity("People").Property<int>("TheKey");
+                builder.Entity("People").Property<Guid>("TheAlternateKey");
+                builder.Entity("People").HasKey("TheKey").HasFillFactor(81);
+                builder.Entity("People").HasAlternateKey("TheAlternateKey").HasFillFactor(82);
+            },
+            model =>
+            {
+                var table = Assert.Single(model.Tables);
+
+                var primaryKey = table.PrimaryKey;
+                Assert.NotNull(primaryKey);
+                Assert.Equal(81, primaryKey[SqlServerAnnotationNames.FillFactor]);
+
+                var uniqueConstraint = table.UniqueConstraints.FirstOrDefault();
+                Assert.NotNull(uniqueConstraint);
+                Assert.Equal(82, uniqueConstraint[SqlServerAnnotationNames.FillFactor]);
+            });
+
+        AssertSql(
+            """
+CREATE TABLE [People] (
+    [TheKey] int NOT NULL IDENTITY,
+    [TheAlternateKey] uniqueidentifier NOT NULL,
+    CONSTRAINT [PK_People] PRIMARY KEY ([TheKey]) WITH (FILLFACTOR = 81),
+    CONSTRAINT [AK_People_TheAlternateKey] UNIQUE ([TheAlternateKey]) WITH (FILLFACTOR = 82)
+);
+""");
+    }
+
     public override async Task Drop_table()
     {
         await base.Drop_table();