Skip to content

Commit c756434

Browse files
maumarmaumar
authored
Targeted fix for #35162 - Regression from EF Core 8 to 9: MigrationBuilder.DropTable Causes Issues with Subsequent Table Recreation (#35764)
Problem was that in 9.0 we changed how we process temporal tables, tracking the temporal table information as we process migrations. However, we didn't take into account the cases where users would manually modify their migrations, or squash multiple migrations together, resulting in corrupted dictionary access for cases where a table was deleted and created in the same migration. This fix just makes the temporal information map handling more robust, so those errors don't happen anymore. There is a broader issue with temporal tables but those will be addressed in a different PR. Fixes #35162
1 parent ba7e7bc commit c756434

File tree

4 files changed

+615
-5
lines changed

4 files changed

+615
-5
lines changed

Diff for: ‎src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs

+20-5
Original file line numberDiff line numberDiff line change
@@ -2577,10 +2577,16 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
25772577
var newRawSchema = renameTableOperation.NewSchema;
25782578
var newSchema = newRawSchema ?? model?.GetDefaultSchema();
25792579

2580+
var temporalTableInformation = BuildTemporalInformationFromMigrationOperation(schema, renameTableOperation);
25802581
if (!temporalTableInformationMap.ContainsKey((tableName, rawSchema)))
25812582
{
2582-
var temporalTableInformation = BuildTemporalInformationFromMigrationOperation(schema, renameTableOperation);
25832583
temporalTableInformationMap[(tableName, rawSchema)] = temporalTableInformation;
2584+
}
2585+
2586+
// we still need to check here - table with the new name could have existed before and have been deleted
2587+
// we want to preserve the original temporal info of that deleted table
2588+
if (!temporalTableInformationMap.ContainsKey((newTableName, newRawSchema)))
2589+
{
25842590
temporalTableInformationMap[(newTableName, newRawSchema)] = temporalTableInformation;
25852591
}
25862592

@@ -2675,10 +2681,19 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
26752681

26762682
var schema = rawSchema ?? model?.GetDefaultSchema();
26772683

2678-
// we are guaranteed to find entry here - we looped through all the operations earlier,
2679-
// info missing from operations we got from the model
2680-
// and in case of no/incomplete model we created dummy (non-temporal) entries
2681-
var temporalInformation = temporalTableInformationMap[(tableName, rawSchema)];
2684+
TemporalOperationInformation temporalInformation;
2685+
if (operation is CreateTableOperation)
2686+
{
2687+
// for create table we always generate new temporal information from the operation itself
2688+
// just in case there was a table with that name before that got deleted/renamed
2689+
// also, temporal state (disabled versioning etc.) should always reset when creating a table
2690+
temporalInformation = BuildTemporalInformationFromMigrationOperation(schema, operation);
2691+
temporalTableInformationMap[(tableName, rawSchema)] = temporalInformation;
2692+
}
2693+
else
2694+
{
2695+
temporalInformation = temporalTableInformationMap[(tableName, rawSchema)];
2696+
}
26822697

26832698
switch (operation)
26842699
{

Diff for: ‎test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs

+149
Original file line numberDiff line numberDiff line change
@@ -3256,6 +3256,118 @@ public virtual Task Add_required_primitve_collection_with_custom_converter_and_c
32563256
Assert.Single(customersTable.PrimaryKey!.Columns));
32573257
});
32583258

3259+
[ConditionalFact]
3260+
public virtual Task Multiop_drop_table_and_create_the_same_table_in_one_migration()
3261+
=> TestComposite(
3262+
[
3263+
builder => builder.Entity(
3264+
"Customer", e =>
3265+
{
3266+
e.Property<int>("Id").ValueGeneratedOnAdd();
3267+
e.Property<string>("Name");
3268+
e.HasKey("Id");
3269+
e.ToTable("Customers");
3270+
}),
3271+
builder => { },
3272+
builder => builder.Entity(
3273+
"Customer", e =>
3274+
{
3275+
e.Property<int>("Id").ValueGeneratedOnAdd();
3276+
e.Property<string>("Name");
3277+
e.HasKey("Id");
3278+
3279+
e.ToTable("Customers");
3280+
})
3281+
]);
3282+
3283+
[ConditionalFact]
3284+
public virtual Task Multiop_create_table_and_drop_it_in_one_migration()
3285+
=> TestComposite(
3286+
[
3287+
builder => { },
3288+
builder => builder.Entity(
3289+
"Customer", e =>
3290+
{
3291+
e.Property<int>("Id").ValueGeneratedOnAdd();
3292+
e.Property<string>("Name");
3293+
e.HasKey("Id");
3294+
3295+
e.ToTable("Customers");
3296+
}),
3297+
builder => { },
3298+
]);
3299+
3300+
[ConditionalFact]
3301+
public virtual Task Multiop_rename_table_and_drop()
3302+
=> TestComposite(
3303+
[
3304+
builder => builder.Entity(
3305+
"Customer", e =>
3306+
{
3307+
e.Property<int>("Id").ValueGeneratedOnAdd();
3308+
e.Property<string>("Name");
3309+
e.HasKey("Id");
3310+
3311+
e.ToTable("Customers");
3312+
}),
3313+
builder => builder.Entity(
3314+
"Customer", e =>
3315+
{
3316+
e.Property<int>("Id").ValueGeneratedOnAdd();
3317+
e.Property<string>("Name");
3318+
e.HasKey("Id");
3319+
3320+
e.ToTable("NewCustomers");
3321+
}),
3322+
builder => { },
3323+
]);
3324+
3325+
[ConditionalFact]
3326+
public virtual Task Multiop_rename_table_and_create_new_table_with_the_old_name()
3327+
=> TestComposite(
3328+
[
3329+
builder => builder.Entity(
3330+
"Customer", e =>
3331+
{
3332+
e.Property<int>("Id").ValueGeneratedOnAdd();
3333+
e.Property<string>("Name");
3334+
e.HasKey("Id");
3335+
3336+
e.ToTable("Customers");
3337+
}),
3338+
builder => builder.Entity(
3339+
"Customer", e =>
3340+
{
3341+
e.Property<int>("Id").ValueGeneratedOnAdd();
3342+
e.Property<string>("Name");
3343+
e.HasKey("Id");
3344+
3345+
e.ToTable("NewCustomers");
3346+
}),
3347+
builder =>
3348+
{
3349+
builder.Entity(
3350+
"Customer", e =>
3351+
{
3352+
e.Property<int>("Id").ValueGeneratedOnAdd();
3353+
e.Property<string>("Name");
3354+
e.HasKey("Id");
3355+
3356+
e.ToTable("NewCustomers");
3357+
});
3358+
3359+
builder.Entity(
3360+
"AnotherCustomer", e =>
3361+
{
3362+
e.Property<int>("Id").ValueGeneratedOnAdd();
3363+
e.Property<string>("Name");
3364+
e.HasKey("Id");
3365+
3366+
e.ToTable("Customers");
3367+
});
3368+
},
3369+
]);
3370+
32593371
protected class Person
32603372
{
32613373
public int Id { get; set; }
@@ -3305,6 +3417,43 @@ protected virtual Task Test(
33053417
MigrationsSqlGenerationOptions migrationsSqlGenerationOptions = MigrationsSqlGenerationOptions.Default)
33063418
=> Test(_ => { }, buildSourceAction, buildTargetAction, asserter, withConventions, migrationsSqlGenerationOptions);
33073419

3420+
protected virtual Task TestComposite(
3421+
List<Action<ModelBuilder>> buildActions,
3422+
bool withConventions = true,
3423+
MigrationsSqlGenerationOptions migrationsSqlGenerationOptions = MigrationsSqlGenerationOptions.Default)
3424+
{
3425+
if (buildActions.Count < 3)
3426+
{
3427+
throw new InvalidOperationException("You need at least 3 build actions for the composite case.");
3428+
}
3429+
3430+
var context = CreateContext();
3431+
var modelDiffer = context.GetService<IMigrationsModelDiffer>();
3432+
var modelRuntimeInitializer = context.GetService<IModelRuntimeInitializer>();
3433+
3434+
var models = new List<IModel>();
3435+
for (var i = 0; i < buildActions.Count; i++)
3436+
{
3437+
var modelBuilder = CreateModelBuilder(withConventions);
3438+
buildActions[i](modelBuilder);
3439+
3440+
var preSnapshotModel = modelRuntimeInitializer.Initialize(
3441+
(IModel)modelBuilder.Model, designTime: true, validationLogger: null);
3442+
3443+
models.Add(preSnapshotModel);
3444+
}
3445+
3446+
// build all migration operations going through each intermediate state of the model
3447+
var operations = new List<MigrationOperation>();
3448+
for (var i = 0; i < models.Count - 1; i++)
3449+
{
3450+
operations.AddRange(
3451+
modelDiffer.GetDifferences(models[i].GetRelationalModel(), models[i + 1].GetRelationalModel()));
3452+
}
3453+
3454+
return Test(models.First(), models.Last(), operations, null, migrationsSqlGenerationOptions);
3455+
}
3456+
33083457
protected virtual Task Test(
33093458
Action<ModelBuilder> buildCommonAction,
33103459
Action<ModelBuilder> buildSourceAction,

0 commit comments

Comments
 (0)