Skip to content

Commit dd9da57

Browse files
Strip identity annotations from temporal history table column operations (#38019)
Fixes #36025 Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
1 parent 83afe7c commit dd9da57

3 files changed

Lines changed: 222 additions & 0 deletions

File tree

src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2586,6 +2586,17 @@ private static bool IsIdentity(ColumnOperation operation)
25862586
|| operation[SqlServerAnnotationNames.ValueGenerationStrategy] as SqlServerValueGenerationStrategy?
25872587
== SqlServerValueGenerationStrategy.IdentityColumn;
25882588

2589+
private static void RemoveIdentityAnnotations(ColumnOperation operation)
2590+
{
2591+
operation.RemoveAnnotation(SqlServerAnnotationNames.Identity);
2592+
2593+
if (operation[SqlServerAnnotationNames.ValueGenerationStrategy] as SqlServerValueGenerationStrategy?
2594+
== SqlServerValueGenerationStrategy.IdentityColumn)
2595+
{
2596+
operation.RemoveAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy);
2597+
}
2598+
}
2599+
25892600
private static bool TryParseIdentitySeedIncrement(ColumnOperation operation, out int seed, out int increment)
25902601
{
25912602
if (operation[SqlServerAnnotationNames.Identity] is string seedIncrement
@@ -2929,6 +2940,25 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
29292940
}
29302941
}
29312942

2943+
var historyTables = new HashSet<(string Name, string? Schema)>(
2944+
temporalTableInformationMap.Values
2945+
.Where(t => t.IsTemporalTable && t.HistoryTableName != null)
2946+
.Select(t => (t.HistoryTableName!, t.HistoryTableSchema)));
2947+
2948+
if (model != null)
2949+
{
2950+
foreach (var table in model.GetRelationalModel().Tables)
2951+
{
2952+
if (table[SqlServerAnnotationNames.IsTemporal] as bool? == true
2953+
&& table[SqlServerAnnotationNames.TemporalHistoryTableName] is string modelHistoryTableName)
2954+
{
2955+
var modelHistoryTableSchema =
2956+
table[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string;
2957+
historyTables.Add((modelHistoryTableName, modelHistoryTableSchema));
2958+
}
2959+
}
2960+
}
2961+
29322962
// now we do proper processing - for table operations we look at the annotations on them
29332963
// and continuously update the stored temporal info as the table is being modified
29342964
// for column (and other) operations we don't have annotations on them, so we look into the
@@ -3150,6 +3180,11 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema
31503180
temporalInformation.PeriodStartColumnName = periodStartColumnName;
31513181
temporalInformation.PeriodEndColumnName = periodEndColumnName;
31523182

3183+
if (isTemporalTable && historyTableName != null)
3184+
{
3185+
historyTables.Add((historyTableName, historyTableSchema));
3186+
}
3187+
31533188
operations.Add(operation);
31543189
break;
31553190
}
@@ -3228,11 +3263,20 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema
32283263
addHistoryTableColumnOperation.ComputedColumnSql = null;
32293264
}
32303265

3266+
// identity columns are not allowed inside HistoryTables
3267+
RemoveIdentityAnnotations(addHistoryTableColumnOperation);
3268+
32313269
operations.Add(addHistoryTableColumnOperation);
32323270
}
32333271
}
32343272
else
32353273
{
3274+
// identity columns are not allowed inside HistoryTables
3275+
if (historyTables.Contains((tableName, schema)))
3276+
{
3277+
RemoveIdentityAnnotations(addColumnOperation);
3278+
}
3279+
32363280
operations.Add(addColumnOperation);
32373281
}
32383282

@@ -3379,11 +3423,22 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema
33793423
alterHistoryTableColumn.OldColumn.Table = temporalInformation.HistoryTableName!;
33803424
alterHistoryTableColumn.OldColumn.Schema = temporalInformation.HistoryTableSchema;
33813425

3426+
// identity columns are not allowed inside HistoryTables
3427+
RemoveIdentityAnnotations(alterHistoryTableColumn);
3428+
RemoveIdentityAnnotations(alterHistoryTableColumn.OldColumn);
3429+
33823430
operations.Add(alterHistoryTableColumn);
33833431
}
33843432
}
33853433
else
33863434
{
3435+
// identity columns are not allowed inside HistoryTables
3436+
if (historyTables.Contains((tableName, schema)))
3437+
{
3438+
RemoveIdentityAnnotations(alterColumnOperation);
3439+
RemoveIdentityAnnotations(alterColumnOperation.OldColumn);
3440+
}
3441+
33873442
operations.Add(alterColumnOperation);
33883443
}
33893444

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

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10442,6 +10442,93 @@ FROM [sys].[default_constraints] [d]
1044210442
"""
1044310443
DECLARE @historyTableSchema1 nvarchar(max) = QUOTENAME(SCHEMA_NAME())
1044410444
EXEC(N'ALTER TABLE [Customer] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = ' + @historyTableSchema1 + '.[CustomerHistory]))')
10445+
""");
10446+
}
10447+
10448+
[ConditionalFact]
10449+
public virtual async Task Add_identity_column_to_temporal_table_when_versioning_is_disabled()
10450+
{
10451+
await Test(
10452+
builder => builder.Entity(
10453+
"Customer", e =>
10454+
{
10455+
e.Property<int>("Id").ValueGeneratedNever();
10456+
e.Property<DateTime>("Start").ValueGeneratedOnAddOrUpdate();
10457+
e.Property<DateTime>("End").ValueGeneratedOnAddOrUpdate();
10458+
e.HasKey("Id");
10459+
10460+
e.ToTable(
10461+
"Customers", tb => tb.IsTemporal(ttb =>
10462+
{
10463+
ttb.UseHistoryTable("HistoryTable");
10464+
ttb.HasPeriodStart("Start");
10465+
ttb.HasPeriodEnd("End");
10466+
}));
10467+
}),
10468+
builder => builder.Entity(
10469+
"Customer", e =>
10470+
{
10471+
e.Property<string>("Name");
10472+
}),
10473+
builder => builder.Entity(
10474+
"Customer", e =>
10475+
{
10476+
e.Property<int>("Number").UseIdentityColumn();
10477+
}),
10478+
model =>
10479+
{
10480+
var table = Assert.Single(model.Tables);
10481+
Assert.Equal("Customers", table.Name);
10482+
Assert.Equal(true, table[SqlServerAnnotationNames.IsTemporal]);
10483+
Assert.Equal("Start", table[SqlServerAnnotationNames.TemporalPeriodStartPropertyName]);
10484+
Assert.Equal("End", table[SqlServerAnnotationNames.TemporalPeriodEndPropertyName]);
10485+
Assert.Equal("HistoryTable", table[SqlServerAnnotationNames.TemporalHistoryTableName]);
10486+
10487+
Assert.Collection(
10488+
table.Columns,
10489+
c => Assert.Equal("Id", c.Name),
10490+
c => Assert.Equal("Number", c.Name));
10491+
Assert.Same(
10492+
table.Columns.Single(c => c.Name == "Id"),
10493+
Assert.Single(table.PrimaryKey!.Columns));
10494+
});
10495+
10496+
AssertSql(
10497+
"""
10498+
ALTER TABLE [Customers] SET (SYSTEM_VERSIONING = OFF)
10499+
""",
10500+
//
10501+
"""
10502+
DECLARE @var2 nvarchar(max);
10503+
SELECT @var2 = QUOTENAME([d].[name])
10504+
FROM [sys].[default_constraints] [d]
10505+
INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id]
10506+
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[Customers]') AND [c].[name] = N'Name');
10507+
IF @var2 IS NOT NULL EXEC(N'ALTER TABLE [Customers] DROP CONSTRAINT ' + @var2 + ';');
10508+
ALTER TABLE [Customers] DROP COLUMN [Name];
10509+
""",
10510+
//
10511+
"""
10512+
DECLARE @var3 nvarchar(max);
10513+
SELECT @var3 = QUOTENAME([d].[name])
10514+
FROM [sys].[default_constraints] [d]
10515+
INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id]
10516+
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[HistoryTable]') AND [c].[name] = N'Name');
10517+
IF @var3 IS NOT NULL EXEC(N'ALTER TABLE [HistoryTable] DROP CONSTRAINT ' + @var3 + ';');
10518+
ALTER TABLE [HistoryTable] DROP COLUMN [Name];
10519+
""",
10520+
//
10521+
"""
10522+
ALTER TABLE [Customers] ADD [Number] int NOT NULL IDENTITY;
10523+
""",
10524+
//
10525+
"""
10526+
ALTER TABLE [HistoryTable] ADD [Number] int NOT NULL DEFAULT 0;
10527+
""",
10528+
//
10529+
"""
10530+
DECLARE @historyTableSchema1 nvarchar(max) = QUOTENAME(SCHEMA_NAME())
10531+
EXEC(N'ALTER TABLE [Customers] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = ' + @historyTableSchema1 + '.[HistoryTable]))')
1044510532
""");
1044610533
}
1044710534
}

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,86 @@ public virtual void AddColumnOperation_identity_legacy()
101101
""");
102102
}
103103

104+
[ConditionalFact]
105+
public virtual void AddColumnOperation_identity_not_propagated_to_history_table()
106+
{
107+
Generate(
108+
modelBuilder => modelBuilder.Entity(
109+
"Customer", e =>
110+
{
111+
e.Property<int>("Id").ValueGeneratedOnAdd();
112+
e.Property<DateTime>("PeriodStart").ValueGeneratedOnAddOrUpdate();
113+
e.Property<DateTime>("PeriodEnd").ValueGeneratedOnAddOrUpdate();
114+
e.HasKey("Id");
115+
e.ToTable(
116+
"Customers", tb => tb.IsTemporal(ttb =>
117+
{
118+
ttb.UseHistoryTable("CustomersHistory");
119+
ttb.HasPeriodStart("PeriodStart");
120+
ttb.HasPeriodEnd("PeriodEnd");
121+
}));
122+
}),
123+
new AddColumnOperation
124+
{
125+
Table = "CustomersHistory",
126+
Name = "Number",
127+
ClrType = typeof(int),
128+
ColumnType = "int",
129+
DefaultValue = 0,
130+
IsNullable = false,
131+
[SqlServerAnnotationNames.ValueGenerationStrategy] =
132+
SqlServerValueGenerationStrategy.IdentityColumn
133+
});
134+
135+
AssertSql(
136+
"""
137+
ALTER TABLE [CustomersHistory] ADD [Number] int NOT NULL DEFAULT 0;
138+
""");
139+
}
140+
141+
[ConditionalFact]
142+
public virtual void AddColumnOperation_identity_legacy_not_propagated_to_history_table()
143+
{
144+
var migrationBuilder = new MigrationBuilder("Microsoft.EntityFrameworkCore.SqlServer");
145+
146+
migrationBuilder.AddColumn<int>(
147+
name: "Number",
148+
table: "CustomersHistory",
149+
type: "int",
150+
nullable: false,
151+
defaultValue: 0)
152+
.Annotation("SqlServer:Identity", "1, 1")
153+
.Annotation("SqlServer:IsTemporal", true)
154+
.Annotation("SqlServer:TemporalHistoryTableName", "CustomersHistory")
155+
.Annotation("SqlServer:TemporalHistoryTableSchema", null)
156+
.Annotation("SqlServer:TemporalPeriodEndColumnName", "PeriodEnd")
157+
.Annotation("SqlServer:TemporalPeriodStartColumnName", "PeriodStart");
158+
159+
Generate(
160+
modelBuilder => modelBuilder.Entity(
161+
"Customer", e =>
162+
{
163+
e.Property<int>("Id").ValueGeneratedOnAdd();
164+
e.Property<DateTime>("PeriodStart").ValueGeneratedOnAddOrUpdate();
165+
e.Property<DateTime>("PeriodEnd").ValueGeneratedOnAddOrUpdate();
166+
e.HasKey("Id");
167+
e.Property<string>("Name");
168+
e.ToTable(
169+
"Customers", tb => tb.IsTemporal(ttb =>
170+
{
171+
ttb.UseHistoryTable("CustomersHistory");
172+
ttb.HasPeriodStart("PeriodStart");
173+
ttb.HasPeriodEnd("PeriodEnd");
174+
}));
175+
}),
176+
migrationBuilder.Operations.ToArray());
177+
178+
AssertSql(
179+
"""
180+
ALTER TABLE [CustomersHistory] ADD [Number] int NOT NULL DEFAULT 0;
181+
""");
182+
}
183+
104184
public override void AddColumnOperation_without_column_type()
105185
{
106186
base.AddColumnOperation_without_column_type();

0 commit comments

Comments
 (0)