Migrations: Fix some bugs

azure-pipelines.yml

@@ -104,10 +104,6 @@ stages:
  pool:
  vmImage: macOS-10.14
  steps:
- # HACK: Use an older version to work around #16667
- - bash: brew install https://raw.githubusercontent.com/Homebrew/homebrew-core/835ae521c065524abdf66578e68032fa24bce514/Formula/libspatialite.rb
- displayName: Install SpatiaLite
- continueOnError: true
  - ${{ if ne(variables['System.TeamProject'], 'public') }}:
  - task: Bash@3
  displayName: Setup Private Feeds Credentials

src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs

@@ -1011,7 +1011,7 @@ protected virtual IEnumerable<MigrationOperation> Diff(
 
  Initialize(
  alterColumnOperation, target, targetTypeMapping,
- target.IsNullable, targetMigrationsAnnotations, inline: true);
+ target.IsNullable, targetMigrationsAnnotations, inline: !source.IsNullable);
 
  Initialize(
  alterColumnOperation.OldColumn, source, sourceTypeMapping,

src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs

@@ -283,7 +283,9 @@ protected override void Generate(
  operation.Name,
  operation.OldColumn,
  model);
- narrowed = type != oldType || !operation.IsNullable && operation.OldColumn.IsNullable;
+ narrowed = type != oldType
+ || operation.Collation != operation.OldColumn.Collation
+ || !operation.IsNullable && operation.OldColumn.IsNullable;
  }
 
  if (narrowed)

test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs

@@ -1912,6 +1912,48 @@ public void Alter_column_nullability()
  });
  }
 
+ [ConditionalFact]
+ public void Alter_column_nullability_to_required()
+ {
+ Execute(
+ source => source.Entity(
+ "Bison",
+ x =>
+ {
+ x.ToTable("Bison", "dbo");
+ x.Property<int>("Id");
+ x.Property<string>("Name")
+ .HasColumnType("nvarchar(30)")
+ .IsRequired(false)
+ .HasDefaultValueSql("CreateBisonName()");
+ }),
+ target => target.Entity(
+ "Bison",
+ x =>
+ {
+ x.ToTable("Bison", "dbo");
+ x.Property<int>("Id");
+ x.Property<string>("Name")
+ .HasColumnType("nvarchar(30)")
+ .IsRequired()
+ .HasDefaultValueSql("CreateBisonName()");
+ }),
+ operations =>
+ {
+ Assert.Equal(1, operations.Count);
+
+ var operation = Assert.IsType<AlterColumnOperation>(operations[0]);
+ Assert.Equal("dbo", operation.Schema);
+ Assert.Equal("Bison", operation.Table);
+ Assert.Equal("Name", operation.Name);
+ Assert.Equal(typeof(string), operation.ClrType);
+ Assert.Equal("nvarchar(30)", operation.ColumnType);
+ Assert.False(operation.IsNullable);
+ Assert.Equal(string.Empty, operation.DefaultValue);
+ Assert.Equal("CreateBisonName()", operation.DefaultValueSql);
+ });
+ }
+
  [ConditionalFact]
  public void Alter_column_type()
  {

test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs

@@ -502,7 +502,8 @@ 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'[People]') AND [c].[name] = N'SomeColumn');
 IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];');
-ALTER TABLE [People] ALTER COLUMN [SomeColumn] nvarchar(max) NOT NULL;");
+ALTER TABLE [People] ALTER COLUMN [SomeColumn] nvarchar(max) NOT NULL;
+ALTER TABLE [People] ADD DEFAULT N'' FOR [SomeColumn];");
  }
 
  [ConditionalFact]
@@ -519,6 +520,7 @@ FROM [sys].[default_constraints] [d]
 WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'SomeColumn');
 IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];');
 ALTER TABLE [People] ALTER COLUMN [SomeColumn] nvarchar(450) NOT NULL;
+ALTER TABLE [People] ADD DEFAULT N'' FOR [SomeColumn];
 CREATE INDEX [IX_People_SomeColumn] ON [People] ([SomeColumn]);");
  }
 
@@ -536,6 +538,7 @@ FROM [sys].[default_constraints] [d]
 WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'FirstName');
 IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];');
 ALTER TABLE [People] ALTER COLUMN [FirstName] nvarchar(450) NOT NULL;
+ALTER TABLE [People] ADD DEFAULT N'' FOR [FirstName];
 CREATE INDEX [IX_People_FirstName_LastName] ON [People] ([FirstName], [LastName]);");
  }
 
@@ -661,6 +664,37 @@ FROM [sys].[default_constraints] [d]
 ALTER TABLE [People] ALTER COLUMN [Name] nvarchar(max) COLLATE German_PhoneBook_CI_AS NULL;");
  }
 
+ [ConditionalFact]
+ public virtual async Task Alter_column_set_collation_with_index()
+ {
+ await Test(
+ builder => builder.Entity(
+ "People", e =>
+ {
+ e.Property<string>("Name");
+ e.HasIndex("Name");
+ }),
+ builder => { },
+ builder => builder.Entity("People").Property<string>("Name")
+ .UseCollation(NonDefaultCollation),
+ model =>
+ {
+ var nameColumn = Assert.Single(Assert.Single(model.Tables).Columns);
+ Assert.Equal(NonDefaultCollation, nameColumn.Collation);
+ });
+
+ AssertSql(
+ @"DROP INDEX [IX_People_Name] ON [People];
+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'[People]') AND [c].[name] = N'Name');
+IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];');
+ALTER TABLE [People] ALTER COLUMN [Name] nvarchar(450) COLLATE German_PhoneBook_CI_AS NULL;
+CREATE INDEX [IX_People_Name] ON [People] ([Name]);");
+ }
+
  [ConditionalFact]
  public override async Task Alter_column_reset_collation()
  {
@@ -711,6 +745,7 @@ FROM [sys].[default_constraints] [d]
 WHERE ([d].[parent_object_id] = OBJECT_ID(N'[People]') AND [c].[name] = N'SomeColumn');
 IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];');
 ALTER TABLE [People] ALTER COLUMN [SomeColumn] nvarchar(450) NOT NULL;
+ALTER TABLE [People] ADD DEFAULT N'' FOR [SomeColumn];
 CREATE INDEX [IX_People_SomeColumn] ON [People] ([SomeColumn]) INCLUDE ([SomeOtherColumn]);");
  }
 
@@ -1470,7 +1505,7 @@ public override async Task Add_primary_key_composite_with_name()
  public virtual async Task Add_primary_key_nonclustered()
  {
  await Test(
- builder => builder.Entity("People").Property<string>("SomeField"),
+ builder => builder.Entity("People").Property<string>("SomeField").IsRequired().HasMaxLength(450),
  builder => { },
  builder => builder.Entity("People").HasKey("SomeField").IsClustered(false),
  model =>
@@ -1482,14 +1517,6 @@ await Test(
  });
 
  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'[People]') AND [c].[name] = N'SomeField');
-IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];');
-ALTER TABLE [People] ALTER COLUMN [SomeField] nvarchar(450) NOT NULL;",
- //
  @"ALTER TABLE [People] ADD CONSTRAINT [PK_People] PRIMARY KEY NONCLUSTERED ([SomeField]);");
  }