diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index af2ecba9ac2..de01334dd12 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -2676,8 +2676,6 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema case AddColumnOperation addColumnOperation: { - operations.Add(addColumnOperation); - // when adding a period column, we need to add it as a normal column first, and only later enable period // removing the period information now, so that when we generate SQL that adds the column we won't be making them // auto generated as period it won't work, unless period is enabled but we can't enable period without adding the @@ -2694,6 +2692,29 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema addColumnOperation.DefaultValue = DateTime.MaxValue; } + // when adding sparse column to temporal table, we need to disable versioning. + // This is because it may be the case that HistoryTable is using compression (by default) + // and the add column operation fails in that situation + // in order to make it work we need to disable versioning (if we haven't done it already) + // and de-compress the HistoryTable + if (addColumnOperation[SqlServerAnnotationNames.Sparse] as bool? == true) + { + if (!temporalInformation.DisabledVersioning + && !temporalInformation.ShouldEnableVersioning) + { + DisableVersioning( + tableName, + schema, + temporalInformation, + suppressTransaction, + shouldEnableVersioning: true); + } + + DecompressTable(temporalInformation.HistoryTableName!, temporalInformation.HistoryTableSchema, suppressTransaction); + } + + operations.Add(addColumnOperation); + // when adding (non-period) column to an existing temporal table we need to check if we have disabled versioning // due to some other operations in the same migration (e.g. delete column) // if so, we need to also add the same column to history table @@ -2707,6 +2728,10 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema operations.Add(addHistoryTableColumnOperation); } } + else + { + operations.Add(addColumnOperation); + } break; } @@ -2798,8 +2823,15 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema // for alter column operation converting column from nullable to non-nullable in the temporal table // we must disable versioning in order to properly handle it // specifically, switching values in history table from null to the default value - if (alterColumnOperation.OldColumn.IsNullable - && !alterColumnOperation.IsNullable + var changeToNonNullable = alterColumnOperation.OldColumn.IsNullable + && !alterColumnOperation.IsNullable; + + // for alter column converting to sparse we also need to disable versioning + // in case HistoryTable is compressed (so that we can de-compress it) + var changeToSparse = alterColumnOperation.OldColumn[SqlServerAnnotationNames.Sparse] as bool? != true + && alterColumnOperation[SqlServerAnnotationNames.Sparse] as bool? == true; + + if ((changeToNonNullable || changeToSparse) && !temporalInformation.DisabledVersioning && !temporalInformation.ShouldEnableVersioning) { @@ -2811,6 +2843,11 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema shouldEnableVersioning: true); } + if (changeToSparse) + { + DecompressTable(temporalInformation.HistoryTableName!, temporalInformation.HistoryTableSchema, suppressTransaction); + } + operations.Add(alterColumnOperation); // when modifying a period column, we need to perform the operations as a normal column first, and only later enable period @@ -3043,6 +3080,38 @@ void EnablePeriod(string table, string? schema, string periodStartColumnName, st }); } + void DecompressTable(string tableName, string? schema, bool suppressTransaction) + { + var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); + + var decompressTableCommand = new StringBuilder() + .Append("IF EXISTS (") + .Append("SELECT 1 FROM [sys].[tables] [t] ") + .Append("INNER JOIN [sys].[partitions] [p] ON [t].[object_id] = [p].[object_id] ") + .Append($"WHERE [t].[name] = '{tableName}' "); + + if (schema != null) + { + decompressTableCommand.Append($"AND [t].[schema_id] = schema_id('{schema}') "); + } + + decompressTableCommand.AppendLine("AND data_compression <> 0)") + .Append("EXEC(") + .Append(stringTypeMapping.GenerateSqlLiteral("ALTER TABLE " + + Dependencies.SqlGenerationHelper.DelimitIdentifier(tableName, schema) + + " REBUILD PARTITION = ALL WITH (DATA_COMPRESSION = NONE)" + + Dependencies.SqlGenerationHelper.StatementTerminator)) + .Append(")") + .AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator); + + operations.Add( + new SqlOperation + { + Sql = decompressTableCommand.ToString(), + SuppressTransaction = suppressTransaction + }); + } + static TOperation CopyColumnOperation(ColumnOperation source) where TOperation : ColumnOperation, new() { diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs index c94b3e66c38..886d72e2593 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs @@ -6980,7 +6980,146 @@ FROM [sys].[default_constraints] [d] """); } - [ConditionalFact(Skip = "Issue #32154")] + [ConditionalFact] + public virtual async Task Add_sparse_column_to_temporal_table() + { + await Test( + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("Start").ValueGeneratedOnAddOrUpdate(); + e.Property("End").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + e.ToTable( + tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable"); + ttb.HasPeriodStart("Start"); + ttb.HasPeriodEnd("End"); + })); + }), + builder => { }, + builder => builder.Entity( + "Customer", e => + { + e.Property("MyColumn").IsSparse(); + }), + model => + { + var table = Assert.Single(model.Tables); + Assert.Equal("Customer", table.Name); + Assert.NotNull(table[SqlServerAnnotationNames.IsTemporal]); + Assert.Equal("HistoryTable", table[SqlServerAnnotationNames.TemporalHistoryTableName]); + Assert.Equal("Start", table[SqlServerAnnotationNames.TemporalPeriodStartPropertyName]); + Assert.Equal("End", table[SqlServerAnnotationNames.TemporalPeriodEndPropertyName]); + Assert.Collection( + table.Columns, + c => Assert.Equal("Id", c.Name), + c => Assert.Equal("Name", c.Name), + c => Assert.Equal("MyColumn", c.Name)); + Assert.Same( + table.Columns.Single(c => c.Name == "Id"), + Assert.Single(table.PrimaryKey!.Columns)); + }); + + AssertSql( +""" +ALTER TABLE [Customer] SET (SYSTEM_VERSIONING = OFF) +""", + // + """ +IF EXISTS (SELECT 1 FROM [sys].[tables] [t] INNER JOIN [sys].[partitions] [p] ON [t].[object_id] = [p].[object_id] WHERE [t].[name] = 'HistoryTable' AND data_compression <> 0) +EXEC(N'ALTER TABLE [HistoryTable] REBUILD PARTITION = ALL WITH (DATA_COMPRESSION = NONE);'); +""", + // + """ +ALTER TABLE [Customer] ADD [MyColumn] int SPARSE NULL; +""", + // + """ +ALTER TABLE [HistoryTable] ADD [MyColumn] int SPARSE NULL; +""", + // + """ +DECLARE @historyTableSchema sysname = SCHEMA_NAME() +EXEC(N'ALTER TABLE [Customer] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + '].[HistoryTable]))') +"""); + } + + [ConditionalFact] + public virtual async Task Add_sparse_column_to_temporal_table_with_custom_schemas() + { + await Test( + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Name"); + e.Property("Start").ValueGeneratedOnAddOrUpdate(); + e.Property("End").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + e.ToTable("Customers", "mySchema", + tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable", "myHistorySchema"); + ttb.HasPeriodStart("Start"); + ttb.HasPeriodEnd("End"); + })); + }), + builder => { }, + builder => builder.Entity( + "Customer", e => + { + e.Property("MyColumn").IsSparse(); + }), + model => + { + var table = Assert.Single(model.Tables); + Assert.Equal("Customers", table.Name); + Assert.Equal("mySchema", table.Schema); + Assert.NotNull(table[SqlServerAnnotationNames.IsTemporal]); + Assert.Equal("HistoryTable", table[SqlServerAnnotationNames.TemporalHistoryTableName]); + Assert.Equal("myHistorySchema", table[SqlServerAnnotationNames.TemporalHistoryTableSchema]); + Assert.Equal("Start", table[SqlServerAnnotationNames.TemporalPeriodStartPropertyName]); + Assert.Equal("End", table[SqlServerAnnotationNames.TemporalPeriodEndPropertyName]); + Assert.Collection( + table.Columns, + c => Assert.Equal("Id", c.Name), + c => Assert.Equal("Name", c.Name), + c => Assert.Equal("MyColumn", c.Name)); + Assert.Same( + table.Columns.Single(c => c.Name == "Id"), + Assert.Single(table.PrimaryKey!.Columns)); + }); + + AssertSql( +""" +ALTER TABLE [mySchema].[Customers] SET (SYSTEM_VERSIONING = OFF) +""", + // + """ +IF EXISTS (SELECT 1 FROM [sys].[tables] [t] INNER JOIN [sys].[partitions] [p] ON [t].[object_id] = [p].[object_id] WHERE [t].[name] = 'HistoryTable' AND [t].[schema_id] = schema_id('myHistorySchema') AND data_compression <> 0) +EXEC(N'ALTER TABLE [myHistorySchema].[HistoryTable] REBUILD PARTITION = ALL WITH (DATA_COMPRESSION = NONE);'); +""", + // + """ +ALTER TABLE [mySchema].[Customers] ADD [MyColumn] int SPARSE NULL; +""", + // + """ +ALTER TABLE [myHistorySchema].[HistoryTable] ADD [MyColumn] int SPARSE NULL; +""", + // + """ +ALTER TABLE [mySchema].[Customers] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [myHistorySchema].[HistoryTable])) +"""); + } + + [ConditionalFact] public virtual async Task Convert_regular_column_of_temporal_table_to_sparse() { await Test( @@ -7036,6 +7175,15 @@ await Test( AssertSql( """ +ALTER TABLE [Customer] SET (SYSTEM_VERSIONING = OFF) +""", + // + """ +IF EXISTS (SELECT 1 FROM [sys].[tables] [t] INNER JOIN [sys].[partitions] [p] ON [t].[object_id] = [p].[object_id] WHERE [t].[name] = 'HistoryTable' AND data_compression <> 0) +EXEC(N'ALTER TABLE [HistoryTable] REBUILD PARTITION = ALL WITH (DATA_COMPRESSION = NONE);'); +""", + // + """ DECLARE @var0 sysname; SELECT @var0 = [d].[name] FROM [sys].[default_constraints] [d] @@ -7043,6 +7191,168 @@ FROM [sys].[default_constraints] [d] WHERE ([d].[parent_object_id] = OBJECT_ID(N'[Customer]') AND [c].[name] = N'MyColumn'); IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [Customer] DROP CONSTRAINT [' + @var0 + '];'); ALTER TABLE [Customer] ALTER COLUMN [MyColumn] int SPARSE NULL; +""", + // + """ +DECLARE @var1 sysname; +SELECT @var1 = [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'[HistoryTable]') AND [c].[name] = N'MyColumn'); +IF @var1 IS NOT NULL EXEC(N'ALTER TABLE [HistoryTable] DROP CONSTRAINT [' + @var1 + '];'); +ALTER TABLE [HistoryTable] ALTER COLUMN [MyColumn] int SPARSE NULL; +""", + // + """ +DECLARE @historyTableSchema sysname = SCHEMA_NAME() +EXEC(N'ALTER TABLE [Customer] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + '].[HistoryTable]))') +"""); + } + + [ConditionalFact] + public virtual async Task Convert_sparse_column_of_temporal_table_to_regular() + { + await Test( + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.Property("Start").ValueGeneratedOnAddOrUpdate(); + e.Property("End").ValueGeneratedOnAddOrUpdate(); + e.HasKey("Id"); + + e.ToTable( + tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable"); + ttb.HasPeriodStart("Start"); + ttb.HasPeriodEnd("End"); + })); + e.HasData( + new { MyColumn = 1 }, + new { MyColumn = 2 }, + new { MyColumn = (int?)null }, + new { MyColumn = (int?)null }); + }), + builder => builder.Entity( + "Customer", e => + { + e.Property("MyColumn").IsSparse(); + }), + builder => builder.Entity( + "Customer", e => + { + e.Property("MyColumn"); + }), + model => + { + var table = Assert.Single(model.Tables); + Assert.Equal("Customer", table.Name); + Assert.NotNull(table[SqlServerAnnotationNames.IsTemporal]); + Assert.Equal("HistoryTable", table[SqlServerAnnotationNames.TemporalHistoryTableName]); + Assert.Equal("Start", table[SqlServerAnnotationNames.TemporalPeriodStartPropertyName]); + Assert.Equal("End", table[SqlServerAnnotationNames.TemporalPeriodEndPropertyName]); + + Assert.Collection( + table.Columns, + c => Assert.Equal("Id", c.Name), + c => Assert.Equal("MyColumn", c.Name)); + Assert.Same( + table.Columns.Single(c => c.Name == "Id"), + Assert.Single(table.PrimaryKey!.Columns)); + }); + + AssertSql( +""" +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'[Customer]') AND [c].[name] = N'MyColumn'); +IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [Customer] DROP CONSTRAINT [' + @var0 + '];'); +ALTER TABLE [Customer] ALTER COLUMN [MyColumn] int NULL; +"""); + } + + [ConditionalFact] + public virtual async Task Convert_regular_table_with_sparse_column_to_temporal() + { + await Test( + builder => builder.Entity( + "Customer", e => + { + e.Property("Id").ValueGeneratedOnAdd(); + e.HasKey("Id"); + e.Property("MyColumn").IsSparse(); + e.HasData( + new { MyColumn = 1 }, + new { MyColumn = 2 }, + new { MyColumn = (int?)null }, + new { MyColumn = (int?)null }); + }), + builder => builder.Entity( + "Customer", e => + { + e.ToTable("Customers"); + }), + builder => builder.Entity( + "Customer", e => + { + e.Property("Start").ValueGeneratedOnAddOrUpdate(); + e.Property("End").ValueGeneratedOnAddOrUpdate(); + e.ToTable( + "Customers", + tb => tb.IsTemporal( + ttb => + { + ttb.UseHistoryTable("HistoryTable"); + ttb.HasPeriodStart("Start"); + ttb.HasPeriodEnd("End"); + })); + }), + model => + { + var table = Assert.Single(model.Tables); + Assert.Equal("Customers", table.Name); + Assert.NotNull(table[SqlServerAnnotationNames.IsTemporal]); + Assert.Equal("HistoryTable", table[SqlServerAnnotationNames.TemporalHistoryTableName]); + Assert.Equal("Start", table[SqlServerAnnotationNames.TemporalPeriodStartPropertyName]); + Assert.Equal("End", table[SqlServerAnnotationNames.TemporalPeriodEndPropertyName]); + + Assert.Collection( + table.Columns, + c => Assert.Equal("Id", c.Name), + c => Assert.Equal("MyColumn", c.Name)); + Assert.Same( + table.Columns.Single(c => c.Name == "Id"), + Assert.Single(table.PrimaryKey!.Columns)); + }); + + AssertSql( +""" +ALTER TABLE [Customers] ADD [End] datetime2 NOT NULL DEFAULT '9999-12-31T23:59:59.9999999'; +""", + // + """ +ALTER TABLE [Customers] ADD [Start] datetime2 NOT NULL DEFAULT '0001-01-01T00:00:00.0000000'; +""", + // + """ +ALTER TABLE [Customers] ADD PERIOD FOR SYSTEM_TIME ([Start], [End]) +""", + // + """ +ALTER TABLE [Customers] ALTER COLUMN [Start] ADD HIDDEN +""", + // + """ +ALTER TABLE [Customers] ALTER COLUMN [End] ADD HIDDEN +""", + // + """ +DECLARE @historyTableSchema sysname = SCHEMA_NAME() +EXEC(N'ALTER TABLE [Customers] SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + '].[HistoryTable]))') """); }