SqlServer FillFactor for PK is missing

[bryanhunwardsen]

"FillFactor" was added several versions ago but it is incomplete/broken.
.HasFillFactor() is only settable on explicit/defined Indexes.
In SqlServer, FillFactor can be set on the PK without additional complexity to the Schema.
This is not available with the FillFactor implementation of EFC 8.
This is an incomplete implementation of the feature which is why I am filing as a bug and not as a feature request.

To advertise FillFactor is supported in EF, code like this:

builder.HasKey(x => x.Id).HasFillFactor(90);

Should produce Sql Schema as:

CREATE TABLE [dbo].[MyTable](
	[Id] [int] IDENTITY(1,1) NOT NULL
 CONSTRAINT [PK_MyTable] PRIMARY KEY CLUSTERED 
(
	[Id] ASC
)WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, FILLFACTOR = 90, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY]
) ON [PRIMARY]
GO

or at a minimum, provide a fluent option that ultimately executes the following sql as it will cover the PK:

ALTER TABLE MyTable REBUILD WITH (FILLFACTOR = 90);
GO

EF Core version: 8
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 6.0
Operating system: Win10
IDE: Visual Studio 2022 17.4

[roji]

Yeah, this makes sense. For now, you can insert the ALTER TABLE SQL directly in your migrations to work around this.

[bryanhunwardsen]

Yeah, this makes sense. For now, you can insert the ALTER TABLE SQL directly in your migrations to work around this.

Unfortunately this is not the case, if the migration epoch is reset, or just rolled back/deleted and regenerated, these changes are lost, would have to be manually reapplied, and are therefore not sufficiently durable. (This is exactly the use case I am addressing currently)

I was able to implement a custom CSharpMigrationOperationGenerator that injects the FillFactor sql script into into the migration Up for each TableCreate and TableAlter Up operations, it is oversimplified, heavy handed, and fragile. Still would prefer a proper fluent implementation from EFCore.

[roji]

Unfortunately this is not the case, if the migration epoch is reset, or just rolled back/deleted and regenerated, these changes are lost, would have to be manually reapplied, and are therefore not sufficiently durable. (This is exactly the use case I am addressing currently)

That's true, but this amounts to saying that SQL can never be used in migrations; we're unfortunately unable to provide built-in migration support for each and every supported feature of all databases out there - there's far too many, and modeling all the possibilities of all DDL variants is simply unfeasible.

I do agree we should implement this (after all we do have it for indexes) - I'm just saying that it's unlikely to be prioritized. It's also not an very difficult thing to do, if you're interested in contributing it.

[deano-hunter]

I would be interested in picking this up as a first issue.

It looks like adding the WITH options and including FILLFACTOR for primary keys to the SQL Generators is what is needed so that HasFillFactor can be used with key properties.

Looking through the code, this looks relatively straight forward. While there will be a few changes to handle adding the FillFactor constant for primary keys, the main code will be in the SqlServerMigrationsSqlGenerator, SqlServerKeyExtensions, SqlServerKeyBuilderExtensions classes.

[bryanhunwardsen]

I would be interested in picking this up as a first issue.

It looks like adding the WITH options and including FILLFACTOR for primary keys to the SQL Generators is what is needed so that HasFillFactor can be used with key properties.

Looking through the code, this looks relatively straight forward. While there will be a few changes to handle adding the FillFactor constant for primary keys, the main code will be in the SqlServerMigrationsSqlGenerator, SqlServerKeyExtensions, SqlServerKeyBuilderExtensions classes.

Please note, .HasAlternateKey() should be included with this work to fill the gap to a complete feature.

[ajcvickers]

@deano-hunter Sounds good; go for it!