diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index 40573c91264..613f391200a 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -2549,10 +2549,16 @@ private IReadOnlyList RewriteOperations( var newRawSchema = renameTableOperation.NewSchema; var newSchema = newRawSchema ?? model?.GetDefaultSchema(); + var temporalTableInformation = BuildTemporalInformationFromMigrationOperation(schema, renameTableOperation); if (!temporalTableInformationMap.ContainsKey((tableName, rawSchema))) { - var temporalTableInformation = BuildTemporalInformationFromMigrationOperation(schema, renameTableOperation); temporalTableInformationMap[(tableName, rawSchema)] = temporalTableInformation; + } + + // we still need to check here - table with the new name could have existed before and have been deleted + // we want to preserve the original temporal info of that deleted table + if (!temporalTableInformationMap.ContainsKey((newTableName, newRawSchema))) + { temporalTableInformationMap[(newTableName, newRawSchema)] = temporalTableInformation; } @@ -2647,10 +2653,19 @@ private IReadOnlyList RewriteOperations( var schema = rawSchema ?? model?.GetDefaultSchema(); - // we are guaranteed to find entry here - we looped through all the operations earlier, - // info missing from operations we got from the model - // and in case of no/incomplete model we created dummy (non-temporal) entries - var temporalInformation = temporalTableInformationMap[(tableName, rawSchema)]; + TemporalOperationInformation temporalInformation; + if (operation is CreateTableOperation) + { + // for create table we always generate new temporal information from the operation itself + // just in case there was a table with that name before that got deleted/renamed + // also, temporal state (disabled versioning etc.) should always reset when creating a table + temporalInformation = BuildTemporalInformationFromMigrationOperation(schema, operation); + temporalTableInformationMap[(tableName, rawSchema)] = temporalInformation; + } + else + { + temporalInformation = temporalTableInformationMap[(tableName, rawSchema)]; + } switch (operation) { diff --git a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs index f83e2575b6b..d91dafb6d63 100644 --- a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs @@ -3213,6 +3213,118 @@ public virtual Task Add_required_primitve_collection_with_custom_converter_and_c Assert.Single(customersTable.PrimaryKey!.Columns)); }); + [ConditionalFact] + public virtual Task Multiop_drop_table_and_create_the_same_table_in_one_migration() + => TestComposite( + [ + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + e.ToTable("Customers"); + }), + builder => { }, + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("Customers"); + }) + ]); + + [ConditionalFact] + public virtual Task Multiop_create_table_and_drop_it_in_one_migration() + => TestComposite( + [ + builder => { }, + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("Customers"); + }), + builder => { }, + ]); + + [ConditionalFact] + public virtual Task Multiop_rename_table_and_drop() + => TestComposite( + [ + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("Customers"); + }), + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("NewCustomers"); + }), + builder => { }, + ]); + + [ConditionalFact] + public virtual Task Multiop_rename_table_and_create_new_table_with_the_old_name() + => TestComposite( + [ + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("Customers"); + }), + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("NewCustomers"); + }), + builder => + { + builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("NewCustomers"); + }); + + builder.Entity( + "AnotherCustomer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("Customers"); + }); + }, + ]); + protected class Person { public int Id { get; set; } @@ -3262,6 +3374,43 @@ protected virtual Task Test( MigrationsSqlGenerationOptions migrationsSqlGenerationOptions = MigrationsSqlGenerationOptions.Default) => Test(_ => { }, buildSourceAction, buildTargetAction, asserter, withConventions, migrationsSqlGenerationOptions); + protected virtual Task TestComposite( + List> buildActions, + bool withConventions = true, + MigrationsSqlGenerationOptions migrationsSqlGenerationOptions = MigrationsSqlGenerationOptions.Default) + { + if (buildActions.Count < 3) + { + throw new InvalidOperationException("You need at least 3 build actions for the composite case."); + } + + var context = CreateContext(); + var modelDiffer = context.GetService(); + var modelRuntimeInitializer = context.GetService(); + + var models = new List(); + for (var i = 0; i < buildActions.Count; i++) + { + var modelBuilder = CreateModelBuilder(withConventions); + buildActions[i](modelBuilder); + + var preSnapshotModel = modelRuntimeInitializer.Initialize( + (IModel)modelBuilder.Model, designTime: true, validationLogger: null); + + models.Add(preSnapshotModel); + } + + // build all migration operations going through each intermediate state of the model + var operations = new List(); + for (var i = 0; i < models.Count - 1; i++) + { + operations.AddRange( + modelDiffer.GetDifferences(models[i].GetRelationalModel(), models[i + 1].GetRelationalModel())); + } + + return Test(models.First(), models.Last(), operations, null, migrationsSqlGenerationOptions); + } + protected virtual Task Test( Action buildCommonAction, Action buildSourceAction, diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs index b7998252e12..b1931dad046 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs @@ -3440,6 +3440,90 @@ await Test( """); } + public override async Task Multiop_drop_table_and_create_the_same_table_in_one_migration() + { + await base.Multiop_drop_table_and_create_the_same_table_in_one_migration(); + + AssertSql( + """ +DROP TABLE [Customers]; +""", + // + """ +CREATE TABLE [Customers] ( + [Id] int NOT NULL IDENTITY, + [Name] nvarchar(max) NULL, + CONSTRAINT [PK_Customers] PRIMARY KEY ([Id]) +); +"""); + } + + public override async Task Multiop_create_table_and_drop_it_in_one_migration() + { + await base.Multiop_create_table_and_drop_it_in_one_migration(); + + AssertSql( + """ +CREATE TABLE [Customers] ( + [Id] int NOT NULL IDENTITY, + [Name] nvarchar(max) NULL, + CONSTRAINT [PK_Customers] PRIMARY KEY ([Id]) +); +""", + // + """ +DROP TABLE [Customers]; +"""); + } + + public override async Task Multiop_rename_table_and_drop() + { + await base.Multiop_rename_table_and_drop(); + + AssertSql( + """ +ALTER TABLE [Customers] DROP CONSTRAINT [PK_Customers]; +""", + // + """ +EXEC sp_rename N'[Customers]', N'NewCustomers', 'OBJECT'; +""", + // + """ +ALTER TABLE [NewCustomers] ADD CONSTRAINT [PK_NewCustomers] PRIMARY KEY ([Id]); +""", + // + """ +DROP TABLE [NewCustomers]; +"""); + } + + public override async Task Multiop_rename_table_and_create_new_table_with_the_old_name() + { + await base.Multiop_rename_table_and_create_new_table_with_the_old_name(); + + AssertSql( + """ +ALTER TABLE [Customers] DROP CONSTRAINT [PK_Customers]; +""", + // + """ +EXEC sp_rename N'[Customers]', N'NewCustomers', 'OBJECT'; +""", + // + """ +ALTER TABLE [NewCustomers] ADD CONSTRAINT [PK_NewCustomers] PRIMARY KEY ([Id]); +""", + // + """ +CREATE TABLE [Customers] ( + [Id] int NOT NULL IDENTITY, + [Name] nvarchar(max) NULL, + CONSTRAINT [PK_Customers] PRIMARY KEY ([Id]) +); +"""); + } + [ConditionalFact] public virtual async Task Create_temporal_table_default_column_mappings_and_default_history_table() { @@ -10757,6 +10841,363 @@ CONSTRAINT [PK_HistoryTable] PRIMARY KEY ([Id]) """); } + [ConditionalFact] + public virtual async Task Temporal_multiop_drop_temporal_table_and_add_the_same_table_in_one_migration() + { + await TestComposite( + [ + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("SystemTimeStart").ValueGeneratedOnAddOrUpdate(); + e.Property("SystemTimeEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + "Customers", tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "historySchema"); + ttb.HasPeriodStart("SystemTimeStart"); + ttb.HasPeriodEnd("SystemTimeEnd"); + })); + }), + builder => { }, + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("SystemTimeStart").ValueGeneratedOnAddOrUpdate(); + e.Property("SystemTimeEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + "Customers", tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "historySchema"); + ttb.HasPeriodStart("SystemTimeStart"); + ttb.HasPeriodEnd("SystemTimeEnd"); + })); + }) + ]); + + AssertSql( +""" +ALTER TABLE [Customers] SET (SYSTEM_VERSIONING = OFF) +""", + // + """ +DROP TABLE [Customers]; +""", + // + """ +DROP TABLE [historySchema].[HistoryTable]; +""", + // + """ +IF SCHEMA_ID(N'historySchema') IS NULL EXEC(N'CREATE SCHEMA [historySchema];'); +""", + // + """ +CREATE TABLE [Customers] ( + [Id] int NOT NULL IDENTITY, + [Name] nvarchar(max) NULL, + [SystemTimeEnd] datetime2 GENERATED ALWAYS AS ROW END HIDDEN NOT NULL, + [SystemTimeStart] datetime2 GENERATED ALWAYS AS ROW START HIDDEN NOT NULL, + CONSTRAINT [PK_Customers] PRIMARY KEY ([Id]), + PERIOD FOR SYSTEM_TIME([SystemTimeStart], [SystemTimeEnd]) +) WITH (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [historySchema].[HistoryTable])); +"""); + } + + [ConditionalFact] + public virtual async Task Temporal_multiop_create_temporal_and_drop() + { + await TestComposite( + [ + builder => { }, + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("SystemTimeStart").ValueGeneratedOnAddOrUpdate(); + e.Property("SystemTimeEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + "Customers", tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "historySchema"); + ttb.HasPeriodStart("SystemTimeStart"); + ttb.HasPeriodEnd("SystemTimeEnd"); + })); + }), + builder => { }, + ]); + + AssertSql( +""" +IF SCHEMA_ID(N'historySchema') IS NULL EXEC(N'CREATE SCHEMA [historySchema];'); +""", + // + """ +CREATE TABLE [Customers] ( + [Id] int NOT NULL IDENTITY, + [Name] nvarchar(max) NULL, + [SystemTimeEnd] datetime2 GENERATED ALWAYS AS ROW END HIDDEN NOT NULL, + [SystemTimeStart] datetime2 GENERATED ALWAYS AS ROW START HIDDEN NOT NULL, + CONSTRAINT [PK_Customers] PRIMARY KEY ([Id]), + PERIOD FOR SYSTEM_TIME([SystemTimeStart], [SystemTimeEnd]) +) WITH (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [historySchema].[HistoryTable])); +""", + // + """ +ALTER TABLE [Customers] SET (SYSTEM_VERSIONING = OFF) +""", + // + """ +DROP TABLE [Customers]; +""", + // + """ +DROP TABLE [historySchema].[HistoryTable]; +"""); + } + + [ConditionalFact] + public virtual async Task Temporal_multiop_rename_temporal_and_drop() + { + await TestComposite( + [ + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("SystemTimeStart").ValueGeneratedOnAddOrUpdate(); + e.Property("SystemTimeEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + "Customers", tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "historySchema"); + ttb.HasPeriodStart("SystemTimeStart"); + ttb.HasPeriodEnd("SystemTimeEnd"); + })); + }), + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("SystemTimeStart").ValueGeneratedOnAddOrUpdate(); + e.Property("SystemTimeEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + "NewCustomers", tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "historySchema"); + ttb.HasPeriodStart("SystemTimeStart"); + ttb.HasPeriodEnd("SystemTimeEnd"); + })); + }), + builder => { }, + ]); + + AssertSql( +""" +ALTER TABLE [Customers] SET (SYSTEM_VERSIONING = OFF) +""", + // + """ +ALTER TABLE [Customers] DROP CONSTRAINT [PK_Customers]; +""", + // + """ +EXEC sp_rename N'[Customers]', N'NewCustomers', 'OBJECT'; +""", + // + """ +ALTER TABLE [NewCustomers] ADD CONSTRAINT [PK_NewCustomers] PRIMARY KEY ([Id]); +""", + // + """ +DROP TABLE [NewCustomers]; +""", + // + """ +DROP TABLE [historySchema].[HistoryTable]; +"""); + } + + [ConditionalFact] + public virtual async Task Temporal_multiop_rename_period_drop_table_create_as_regular() + { + await TestComposite( + [ + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("SystemTimeStart").ValueGeneratedOnAddOrUpdate(); + e.Property("SystemTimeEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + "Customers", tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "historySchema"); + ttb.HasPeriodStart("SystemTimeStart"); + ttb.HasPeriodEnd("SystemTimeEnd"); + })); + }), + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("NewSystemTimeStart").ValueGeneratedOnAddOrUpdate(); + e.Property("SystemTimeEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + "Customers", tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "historySchema"); + ttb.HasPeriodStart("NewSystemTimeStart"); + ttb.HasPeriodEnd("SystemTimeEnd"); + })); + }), + builder => { }, + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("Customers"); + }), + ]); + + AssertSql( +""" +EXEC sp_rename N'[Customers].[SystemTimeStart]', N'NewSystemTimeStart', 'COLUMN'; +""", + // + """ +ALTER TABLE [Customers] SET (SYSTEM_VERSIONING = OFF) +""", + // + """ +DROP TABLE [Customers]; +""", + // + """ +DROP TABLE [historySchema].[HistoryTable]; +""", + // + """ +CREATE TABLE [Customers] ( + [Id] int NOT NULL IDENTITY, + [Name] nvarchar(max) NULL, + CONSTRAINT [PK_Customers] PRIMARY KEY ([Id]) +); +"""); + } + + [ConditionalFact] + public virtual async Task Temporal_multiop_rename_column_drop_table_create_as_regular() + { + await TestComposite( + [ + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("SystemTimeStart").ValueGeneratedOnAddOrUpdate(); + e.Property("SystemTimeEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + "Customers", tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "historySchema"); + ttb.HasPeriodStart("SystemTimeStart"); + ttb.HasPeriodEnd("SystemTimeEnd"); + })); + }), + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("NewName"); + e.Property("SystemTimeStart").ValueGeneratedOnAddOrUpdate(); + e.Property("SystemTimeEnd").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + "Customers", tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "historySchema"); + ttb.HasPeriodStart("SystemTimeStart"); + ttb.HasPeriodEnd("SystemTimeEnd"); + })); + }), + builder => { }, + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.HasKey("Id"); + + e.ToTable("Customers"); + }), + ]); + + AssertSql( +""" +EXEC sp_rename N'[Customers].[Name]', N'NewName', 'COLUMN'; +""", + // + """ +ALTER TABLE [Customers] SET (SYSTEM_VERSIONING = OFF) +""", + // + """ +DROP TABLE [Customers]; +""", + // + """ +DROP TABLE [historySchema].[HistoryTable]; +""", + // + """ +CREATE TABLE [Customers] ( + [Id] int NOT NULL IDENTITY, + [Name] nvarchar(max) NULL, + CONSTRAINT [PK_Customers] PRIMARY KEY ([Id]) +); +"""); + } + [ConditionalFact] public override async Task Add_required_primitive_collection_to_existing_table() { diff --git a/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs index 79529ed9b1d..e3a82cc168d 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs @@ -2070,6 +2070,11 @@ public override Task Rename_sequence() public override Task Move_sequence() => AssertNotSupportedAsync(base.Move_sequence, SqliteStrings.SequencesNotSupported); + public override Task Multiop_rename_table_and_drop() + => AssertNotSupportedAsync( + base.Multiop_rename_table_and_drop, + SqliteStrings.InvalidMigrationOperation(nameof(DropPrimaryKeyOperation))); + [ConditionalFact] public override async Task Add_required_primitve_collection_to_existing_table() {