SqlServer Migrations: Don't omit ALTER COLUMN when old type is unknown (

#23084) The fix for #21364 broke migrations created prior to version 3.1.0 which did not specify the old column type. In this case, we don't know whether the column was previously part of an index or not, so we should always include the ALTER COLUMN statement just in case. Note, the old column type is always specified for migrations created with versions 3.1.0 and newer. Fixes #23045
dotnet · Oct 24, 2020 · 33570d0 · 33570d0
1 parent d812063
commit 33570d0
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 10 deletions.
diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
@@ -291,6 +291,14 @@ protected override void Generate(
                 return;
             }
 
+            var columnType = operation.ColumnType
+                ?? GetColumnType(
+                    operation.Schema,
+                    operation.Table,
+                    operation.Name,
+                    operation,
+                    model);
+
             var narrowed = false;
             var oldColumnSupported = IsOldColumnSupported(model);
             if (oldColumnSupported)
@@ -299,22 +307,14 @@ protected override void Generate(
                 {
                     throw new InvalidOperationException(SqlServerStrings.AlterIdentityColumn);
                 }
-
-                var type = operation.ColumnType
-                    ?? GetColumnType(
-                        operation.Schema,
-                        operation.Table,
-                        operation.Name,
-                        operation,
-                        model);
                 var oldType = operation.OldColumn.ColumnType
                     ?? GetColumnType(
                         operation.Schema,
                         operation.Table,
                         operation.Name,
                         operation.OldColumn,
                         model);
-                narrowed = type != oldType
+                narrowed = columnType != oldType
                     || operation.Collation != operation.OldColumn.Collation
                     || !operation.IsNullable && operation.OldColumn.IsNullable;
             }
@@ -328,7 +328,7 @@ protected override void Generate(
             var alterStatementNeeded = narrowed
                 || !oldColumnSupported
                 || operation.ClrType != operation.OldColumn.ClrType
-                || operation.ColumnType != operation.OldColumn.ColumnType
+                || columnType != operation.OldColumn.ColumnType
                 || operation.IsUnicode != operation.OldColumn.IsUnicode
                 || operation.IsFixedLength != operation.OldColumn.IsFixedLength
                 || operation.MaxLength != operation.OldColumn.MaxLength

diff --git a/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs
@@ -266,6 +266,47 @@ FROM [sys].[default_constraints] [d]
 ALTER TABLE [Person] ALTER COLUMN [Name] nvarchar(30) NULL;
 GO
 
+CREATE INDEX [IX_Person_Name] ON [Person] ([Name]);
+");
+        }
+
+        [ConditionalFact]
+        public virtual void AlterColumnOperation_with_added_index_no_oldType()
+        {
+            Generate(
+                modelBuilder => modelBuilder
+                    .HasAnnotation(CoreAnnotationNames.ProductVersion, "2.1.0")
+                    .Entity<Person>(
+                        x =>
+                        {
+                            x.Property<string>("Name");
+                            x.HasIndex("Name");
+                        }),
+                new AlterColumnOperation
+                {
+                    Table = "Person",
+                    Name = "Name",
+                    ClrType = typeof(string),
+                    IsNullable = true,
+                    OldColumn = new AddColumnOperation { ClrType = typeof(string), IsNullable = true }
+                },
+                new CreateIndexOperation
+                {
+                    Name = "IX_Person_Name",
+                    Table = "Person",
+                    Columns = new[] { "Name" }
+                });
+
+            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'[Person]') AND [c].[name] = N'Name');
+IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [Person] DROP CONSTRAINT [' + @var0 + '];');
+ALTER TABLE [Person] ALTER COLUMN [Name] nvarchar(450) NULL;
+GO
+
 CREATE INDEX [IX_Person_Name] ON [Person] ([Name]);
 ");
         }