Skip to content

Commit

Permalink
Migrations: Fix some bugs
Browse files Browse the repository at this point in the history
Changes:
- Synthesize DEFAULT value when column altered to NOT NULL (fixes #19882)
- SQL Server: Rebuild indexes when column collation is altered (fixes #21547)
  • Loading branch information
bricelam committed Jul 24, 2020
1 parent e1f253b commit e167e91
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
47 changes: 37 additions & 10 deletions test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]);");
}

Expand All @@ -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]);");
}

Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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]);");
}

Expand Down Expand Up @@ -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 =>
Expand All @@ -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]);");
}

Expand Down

0 comments on commit e167e91

Please sign in to comment.