dotnet
Identity Annotation is not updated by migration
I had created a typical ID field with identity. Then I realized I needed to set a seed value of 20000 so I updated the Configuration:
public void Configure(EntityTypeBuilder<Ticket> builder)
{
builder.HasKey(x => x.Id);
builder.Property(x => x.Id)
.UseIdentityColumn(20_000, 1);
}
This was discovered correctly by the migration:
migrationBuilder.AlterColumn<int>(
name: "Id",
schema: "ArpWork",
table: "Ticket",
type: "int",
nullable: false,
oldClrType: typeof(int),
oldType: "int")
.Annotation("SqlServer:Identity", "20000, 1")
.OldAnnotation("SqlServer:Identity", "1, 1");
}
but the SQL script generated as
DECLARE @var0 sysname;
SELECT @var0 = [d].[name]
FROM [sys].[default_constraints] [d]
INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id]
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[ArpWork].[Ticket]') AND [c].[name] = N'Id');
IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [ArpWork].[Ticket] DROP CONSTRAINT [' + @var0 + '];');
ALTER TABLE [ArpWork].[Ticket] ALTER COLUMN [Id] int NOT NULL;
GO
This does not have any affect as the table definition is still
CREATE TABLE [ArpWork].[Ticket](
[Id] [INT] IDENTITY(1,1) NOT NULL,
If I remove migrations and create the table with the new identity value in place, it works fine.
However, even with the correct value in place, there is nothing in the default_constraints table.
DECLARE @var0 sysname;
SELECT @var0 = [d].[name]
FROM [sys].[default_constraints] [d]
INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id]
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[ArpWork].[Ticket]') AND [c].[name] = N'Id');
That returns an empty set.
It could be modified to use:
IF EXISTS (
SELECT *
FROM sys.identity_columns AS i
INNER JOIN sys.columns AS c ON c.column_id = i.column_id AND c.object_id = i.object_id
WHERE i.object_id = OBJECT_ID(N'ArpWork.Ticket') AND c.name = N'Id')
BEGIN
DBCC CHECKIDENT(N'ArpWork.Ticket', RESEED,20000)
END
but that only changes the current Seed value. If the table were truncated, the seed would restart at the original value (1) again.
The only way to ensure that the column identity seed definition is changed would be creating a new table, copying the data and dropping the old table.
In my opinion, changing the seed is rare, and it is only reset by a truncate. I propose that changing the current seed value using CHECKIDENT is sufficient for this type of change. I think this would be preferable rather than the concerns of changing the PK and related FK constraints throughout the database.
If a truncate happens and existing data is being brought over, it would be the responsibility of the DBA to ensure the keys are set properly. If the table is dropped and recreated by EF, the new identity definition would be used at table creation.
Include provider and version information
EF Core version: 6.0.6 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 6.0 Operating system: Windows 10 IDE: Visual Studio 2022 17.2.1
@ajcvickers Can someone take a look at this. I believe this is a bug in EFC 6.
@wdhenrik it sometimes takes us a couple days to look at issues, please be patient.
@wdhenrik I overall agree with what you're saying.
Dropping and recreating the identity column (rebuild) is something we're already tracking in #2100, for situations where an IDENTITY column is changed to no longer be IDENTITY (or vice versa); this is generally pretty risky and we don't plan on implementing it at the moment.
I'm personally not convinced that it's a good idea to reset the existing current value when the starting value changes; but that would be consistent with how we currently manage sequences (changing the start value scaffolds a RestartSequece migration which changes the current value).
We'll discuss in triage whether the limitation on CHECKIDENT(RESEED) is problematic (i.e. the fact that it's lost when doing TRUNCATE TABLE.
There are two aspects that need consideration. The first is to change the migration so it doesn't generate the bad sql. I don't think a default constraint can exist on an identity column, but generating an unneeded drop statement is a bit scary. It would be better for it to do nothing, or raise an exception in the migration code that this action cannot be done through a migration. I would prefer to see this portion fixed in EFC 6.
The second being the correct implementation to change an identity seed. Since migrations successfully detected a change to the Identity value, would it be reasonable to use CHECKIDENT(RESEED) and surround it with some generated SQL comments, calling out the limitations of RESEED and TRUNCATE? When the dev reviews the migrations, they should be able to see it and change strategy if needed. This would be far less complicated and risky than the table swapping alternative.
It's true that we shouldn't be generating the current SQL, but it doesn't have any negative effects as far as I can tell. Our bar for patching stable releases is generally quite high, I don't think this would make it unless there's an actual consequence here.
Re what to actually do here... The team is generally not a fan of the table/column rebuild option, as in #2100; we're unlikely to implement it. Assuming that option is off the table, then it's a question of whether it's better to implement a "partial" solution which is reverted upon table truncation, or do nothing at all (with a possible warning). I can definitely an argument for not doing anything by default, and pointing users to do CHECKIDENT(RESEED) manually if that's what they want. While I agree that truncation is rare, if someone does do it than the unexpected reverting of the seed would be very surprising and hard to track down.
Since EFC6 is an LTS, I figured the patch suggestion would be worth it. Currently it does nothing, but pretends to do something. I think It would be better if it did nothing correctly, or maybe just add a minor SQL RAISERROR to draw attention that nothing is happening.
Going forward, a compile time warning or error would be sufficient for me. Something to draw my attention that EF Core cannot implement the desired change and custom migration steps are required. I would definitely be opposed to the ‘do nothing’ approach for EF7. That just seems like building in a false positive.
I do see this as an issue for the SQL Server provider, not EFCore directly. How is this handled in the other providers?
Wes
I think It would be better if it did nothing correctly, or maybe just add a minor SQL RAISERROR to draw attention that nothing is happening.
I definitely agree, I just don't think it meets the bar for a patch release.
I do see this as an issue for the SQL Server provider, not EFCore directly.
It's definitely a provider-specific issue, but since the SQL Server provider is part of the EF Core project (maintained by the team), that doesn't make much of a difference.
How is this handled in the other providers?
I'm not aware of a similar problem in other providers. IDENTITY columns are a SQL Server-specific thing; similar concepts exist in other databases, but I've not seen this particular problem appear elsewhere yet.
Note from triage: we should not worry too much about table truncations, so doing the thing that works here make sense.
Note from triage: we should not worry too much about table truncations, so doing the thing that works here make sense.
I agree. If someone is manually truncating the table, they should own the responsibility for the keys being reset and how that would affect any data being imported into the table.
For my situation, I'm only starting at 20_000 because I have legacy records I need to keep. If I truncated the table, there would not be any legacy data and no concerns about identity key start. CHECKIDENT would be sufficient to address this in a migration.
Implemented this as discussed, with a change in identity seed triggering a CHECKIDENT RESEED in migrations. TRUNCATE reverts the table to its original seed value (ignoring the the seed change), but as discussed, TRUNCATE is a manual operation and special care must be take in any case.