Skip to content

Commit

Permalink
Apply fill factor when creating tables (#33407)
Browse files Browse the repository at this point in the history 
* Apply fill factor when creating tables

Fixes #33269

* Updated to remove KeyTraits and use IndexOptions
  • Loading branch information
@ajcvickers
ajcvickers committed Mar 27, 2024
1 parent 15b129b commit ce152e3
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 64 deletions.
25 changes: 8 additions & 17 deletions src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,6 @@ protected virtual void Generate(

PrimaryKeyConstraint(operation, model, builder);

KeyWithOptions(operation, builder);

if (terminate)
{
builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
Expand All @@ -269,8 +267,6 @@ protected virtual void Generate(

UniqueConstraint(operation, model, builder);

KeyWithOptions(operation, builder);

builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
EndStatement(builder);
}
Expand Down Expand Up @@ -1622,6 +1618,8 @@ protected virtual void PrimaryKeyConstraint(
builder.Append("(")
.Append(ColumnList(operation.Columns))
.Append(")");

IndexOptions(operation, model, builder);
}

/// <summary>
Expand Down Expand Up @@ -1669,6 +1667,8 @@ protected virtual void UniqueConstraint(
builder.Append("(")
.Append(ColumnList(operation.Columns))
.Append(")");

IndexOptions(operation, model, builder);
}

/// <summary>
Expand Down Expand Up @@ -1716,16 +1716,6 @@ protected virtual void CheckConstraint(
.Append(")");
}

/// <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 virtual void KeyWithOptions(MigrationOperation operation, MigrationCommandListBuilder builder)
{
}

/// <summary>
/// Generates a SQL fragment for traits of an index from a <see cref="CreateIndexOperation" />,
/// <see cref="AddPrimaryKeyOperation" />, or <see cref="AddUniqueConstraintOperation" />.
Expand Down Expand Up @@ -1767,13 +1757,14 @@ protected virtual void GenerateIndexColumnList(CreateIndexOperation operation, I
/// <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 IndexOptions(CreateIndexOperation operation, IModel? model, MigrationCommandListBuilder builder)
protected virtual void IndexOptions(MigrationOperation operation, IModel? model, MigrationCommandListBuilder builder)
{
if (!string.IsNullOrEmpty(operation.Filter))
if (operation is CreateIndexOperation createIndexOperation
&& !string.IsNullOrEmpty(createIndexOperation.Filter))
{
builder
.Append(" WHERE ")
.Append(operation.Filter);
.Append(createIndexOperation.Filter);
}
}

Expand Down
72 changes: 25 additions & 47 deletions src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Globalization;
using System.Text;
using Microsoft.EntityFrameworkCore.SqlServer.Internal;
Expand Down Expand Up @@ -1775,30 +1774,6 @@ 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)
{
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(")");
}
}

/// <summary>
/// Generates a SQL fragment for traits of an index from a <see cref="CreateIndexOperation" />,
/// <see cref="AddPrimaryKeyOperation" />, or <see cref="AddUniqueConstraintOperation" />.
Expand All @@ -1820,7 +1795,7 @@ protected override void IndexTraits(MigrationOperation operation, IModel? model,
/// <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 override void IndexOptions(CreateIndexOperation operation, IModel? model, MigrationCommandListBuilder builder)
protected override void IndexOptions(MigrationOperation operation, IModel? model, MigrationCommandListBuilder builder)
{
if (operation[SqlServerAnnotationNames.Include] is IReadOnlyList<string> includeColumns
&& includeColumns.Count > 0)
Expand All @@ -1839,37 +1814,40 @@ protected override void IndexOptions(CreateIndexOperation operation, IModel? mod
builder.Append(")");
}

if (!string.IsNullOrEmpty(operation.Filter))
if (operation is CreateIndexOperation createIndexOperation)
{
builder
.Append(" WHERE ")
.Append(operation.Filter);
}
else if (UseLegacyIndexFilters(operation, model))
{
var table = model?.GetRelationalModel().FindTable(operation.Table, operation.Schema);
var nullableColumns = operation.Columns
.Where(c => table?.FindColumn(c)?.IsNullable != false)
.ToList();

builder.Append(" WHERE ");
for (var i = 0; i < nullableColumns.Count; i++)
if (!string.IsNullOrEmpty(createIndexOperation.Filter))
{
builder
.Append(" WHERE ")
.Append(createIndexOperation.Filter);
}
else if (UseLegacyIndexFilters(createIndexOperation, model))
{
if (i != 0)
var table = model?.GetRelationalModel().FindTable(createIndexOperation.Table, createIndexOperation.Schema);
var nullableColumns = createIndexOperation.Columns
.Where(c => table?.FindColumn(c)?.IsNullable != false)
.ToList();

builder.Append(" WHERE ");
for (var i = 0; i < nullableColumns.Count; i++)
{
builder.Append(" AND ");
}
if (i != 0)
{
builder.Append(" AND ");
}

builder
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(nullableColumns[i]))
.Append(" IS NOT NULL");
builder
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(nullableColumns[i]))
.Append(" IS NOT NULL");
}
}
}

IndexWithOptions(operation, builder);
}

private static void IndexWithOptions(CreateIndexOperation operation, MigrationCommandListBuilder builder)
private static void IndexWithOptions(MigrationOperation operation, MigrationCommandListBuilder builder)
{
var options = new List<string>();

Expand Down
36 changes: 36 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit ce152e3

Please sign in to comment.