Ef6: Constraint not being re-created after being dropped automatically (changed in 6.2)

Created on 13 Sep 2019  路  9Comments  路  Source: dotnet/ef6

If a table is created with a primary key default value of (newsequentialid()), and then in a subsequent migration that column is ALTERed, then the default will be removed and not recreated. This occurs in 6.2 and 6.3 but NOT in 6.1.

Model/Context Setup

    public class FooTable
    {
        public Guid Id { get; set; }
    }

    public class FooTableMap : EntityTypeConfiguration<FooTable>
    {
        public FooTableMap()
        {
           Property(t => t.Id).HasColumnName("FooId").HasDatabaseGeneratedOption(DatabaseGeneratedOption.Identity);
        }
    }

    public class FooContext : DbContext
    {
        public DbSet<FooTable> Foos { get; set; }

        public FooContext() : base("EFContext")
        {
        }

        protected override void OnModelCreating(DbModelBuilder modelBuilder)
        {
            modelBuilder.Configurations.AddFromAssembly(typeof(FooTableMap).Assembly);
        }
    }

Repro

Create an initial migration, you get something like this:

    public partial class InitialCreate : DbMigration
    {
        public override void Up()
        {
            CreateTable(
                "dbo.FooTables",
                c => new
                    {
                        FooId = c.Guid(nullable: false, identity: true),  // <<< Identity == true!
                    })
                .PrimaryKey(t => t.FooId);

        }

        public override void Down()
        {
            DropTable("dbo.FooTables");
        }
    }

Run this migration and you will get the table with the default value as expected.

Now add a new migration like this:

    public partial class NewMigration : DbMigration
    {
        public override void Up()
        {
            DropPrimaryKey("dbo.FooTables");
            AlterColumn("dbo.FooTables", "FooId", c => c.Guid(nullable: false, identity: true));  // <<< IDENTITY is *still* true
            AddPrimaryKey("dbo.FooTables", "FooId");
        }

        public override void Down()
        {
        }
    }

The migration defines the column exactly as it was in the original migration.
Run the migration:

  • On 6.1 the default value will still remain
  • On 6.2 the default value will be removed

The SQL in 6.1 looks like:

ALTER TABLE [dbo].[FooTables] ALTER COLUMN [FooId] [uniqueidentifier] NOT NULL

in 6.2 (and 6.3) it is:

SELECT @var0 = name
FROM sys.default_constraints
WHERE parent_object_id = object_id(N'dbo.FooTables')
AND col_name(parent_object_id, parent_column_id) = 'FooId';
IF @var0 IS NOT NULL
    EXECUTE('ALTER TABLE [dbo].[FooTables] DROP CONSTRAINT [' + @var0 + ']')
ALTER TABLE [dbo].[FooTables] ALTER COLUMN [FooId] [uniqueidentifier] NOT NULL

The drop contraint removes the default value, and nothing adds it back in.

This issues was introduced by this commit https://github.com/aspnet/EntityFramework6/pull/137/commits/15de49003c43365307408b408dc57252847f9803

Possible Fix
Add an additional WHERE clause to the contraint sql to exclude default values? (AND type <> 'D' at the end)

Further technical details

EF version: 6.2
Database Provider: MSSQL
Operating system: Windows 10
IDE: Visual Studio 2019 / Rider 2019.2
[EfMigrationTest.zip]

Complete repro solution (using EF6.2 - downgrade to 6.1 and re-run migrations to see the old SQL)

(https://github.com/aspnet/EntityFramework6/files/3609403/EfMigrationTest.zip)

type-bug

All 9 comments

@slovely When you say, "Now add a new migration like this:"

```C#
public partial class NewMigration : DbMigration
{
public override void Up()
{
DropPrimaryKey("dbo.FooTables");
AlterColumn("dbo.FooTables", "FooId", c => c.Guid(nullable: false, identity: true)); // <<< IDENTITY is still true
AddPrimaryKey("dbo.FooTables", "FooId");
}

    public override void Down()
    {
    }
}

```
is this a migration that we scaffold for some model change? Or is it purely created by hand?

Hi @ajcvickers - for this contrived example, creating it by hand is fine. (ie. Add-Migration MyTestMigration which generates an empty file and add the code above in).

The newid() default value on the primary key column will be lost.

@slovely I'm actually asking in what situation we generate this migration, if at all.

@ajcvickers apologies - no, I don't _think_ that code was generated. This occurred when migrating a .NET 4.6 application to dotnetcore (so moved to EF6.3), which had ~500 migrations stretching back years. So I can't be certain that the migration wasn't generated.
My problem isn't that though, it's that the PK column is losing the default value. Altering the column (even in this contrived example to be the same as it already was) causes the default value to be dropped and not recreated.

@slovely Agreed. However, I'm trying to establish how likely it is for people to hit this. Bugs on EF6 have to have a very high impact for them to be fixed; we don't have resources to fix everything. This is especially true if there is a workaround.

@ajcvickers - Fair enough :) I would guess it is pretty low-impact, especially as it already existed in 6.2. It's possible some more people might upgrade from <6.1 to 6.3 for the .net core support though.

Just in case someone stumbles on this issue the workaround we did was to modify the migrations from:

AlterColumn("dbo.FooTables", "FooId", c => c.Guid(nullable: false, identity: true)); 

to:

AlterColumn("dbo.FooTables", "FooId", c => c.Guid(nullable: false, defaultValueSql: "newsequentialid()")); 

Please feel free to close, or if you think it's worthwhile for me to create a PR with a possible fix (not looked at the migration code before so can't promise I will manage it!)

Putting on the backlog now to re-triage with other issues later.

Thank you. By the way - we upgraded a fairly large app from EF6.1 to EF6.3, moved from 4.6.1 to dotnetcore v3, and it was really easy. Also CPU usage appears to be nearly 50% less, with no effort spent on optimisations. Just a little "thank you" to offset the bug 馃槂

Thanks, @slovely. Appreciate the feedback. 馃槃

Was this page helpful?
0 / 5 - 0 ratings