Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using ValueGeneratedOnAdd on a property that is seeded causes UpdateData calls in every subsequent migration #21661

Closed
ChristopherHaws opened this issue Jul 17, 2020 · 4 comments · Fixed by #21683
Labels
area-migrations-seeding closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@ChristopherHaws
Copy link
Contributor

ChristopherHaws commented Jul 17, 2020

Every time I create a new migration, even when no data has changed, my migration contains a call to UpdateData when one of the properties is configured with HasDefaultValueSql and HasConversion. If I remove the HasDefaultValueSql call, the issue does not happen.

Steps to reproduce

EFCore 3.1.6:
https://github.com/ChristopherHaws/bug-repro-efcore-migrations

EF Core 5.0.0-preview.6.20312.4:
https://github.com/ChristopherHaws/bug-repro-efcore-migrations/tree/dotnet-5

Given the following DbContext configuration, if you call dotnet ef migrations add twice without changing configuration, the migration script will contain an UpdateData call to the TimeZone field.

public class ApplicationDbContext : DbContext
{
    public ApplicationDbContext(DbContextOptions<ApplicationDbContext> options)
        : base(options)
    {
    }

    public DbSet<TestEntity> TestEntities { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<TestEntity>(entity =>
        {
            entity.HasKey(x => x.Code).IsClustered();
            entity.Property(x => x.TimeZone).IsRequired().HasDefaultValueSql("('UTC')").HasConversion(x => x.Id, x => TimeZoneInfo.FindSystemTimeZoneById(x));

            entity.HasData(new TestEntity(
                code: "code-1",
                timeZone: TimeZoneInfo.FindSystemTimeZoneById("Eastern Standard Time")
            ));
        });
    }
}

public class TestEntity
{
    public TestEntity(string code, TimeZoneInfo timeZone)
    {
        Code = code;
        TimeZone = timeZone;
    }

    public string Code { get; set; }
    public TimeZoneInfo TimeZone { get; set; }
}

The second call to dotnet ef migrations add results in the following migration:

public partial class NoChanges : Migration
{
    protected override void Up(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.UpdateData(
            table: "TestEntities",
            keyColumn: "Code",
            keyValue: "code-1",
            column: "TimeZone",
            value: "Eastern Standard Time");
    }

    protected override void Down(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.UpdateData(
            table: "TestEntities",
            keyColumn: "Code",
            keyValue: "code-1",
            column: "TimeZone",
            value: "Eastern Standard Time");
    }
}

Output for dotnet ef migrations add NoChanges2 --verbose

Using project 'C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData.csproj'.Using startup project 'C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData.csproj'.
Writing 'C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\obj\EFCoreMigrationUpdateData.csproj.EntityFrameworkCore.targets'...
dotnet msbuild /target:GetEFProjectMetadata /property:EFProjectMetadataFile=C:\Users\cribs\AppData\Local\Temp\tmpBE36.tmp /verbosity:quiet /nologo C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData.csproj
Writing 'C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\obj\EFCoreMigrationUpdateData.csproj.EntityFrameworkCore.targets'...
dotnet msbuild /target:GetEFProjectMetadata /property:EFProjectMetadataFile=C:\Users\cribs\AppData\Local\Temp\tmpBFFD.tmp /verbosity:quiet /nologo C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData.csproj
Build started...
dotnet build C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData.csproj /verbosity:quiet /nologo

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.90
Build succeeded.
dotnet exec --depsfile C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\bin\Debug\net5.0\EFCoreMigrationUpdateData.deps.json --additionalprobingpath C:\Users\cribs\.nuget\packages --runtimeconfig C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMig
nUpdateData\bin\Debug\net5.0\EFCoreMigrationUpdateData.runtimeconfig.json C:\Users\cribs\.nuget\packages\dotnet-ef\3.1.6\tools\netcoreapp3.1\any\tools\netcoreapp2.0\any\ef.dll migrations add NoChanges2 --assembly C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigratio
teData\bin\Debug\net5.0\EFCoreMigrationUpdateData.dll --startup-assembly C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\bin\Debug\net5.0\EFCoreMigrationUpdateData.dll --project-dir C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateD
--language C# --working-dir C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData --verbose --root-namespace EFCoreMigrationUpdateData
Using assembly 'EFCoreMigrationUpdateData'.
Using startup assembly 'EFCoreMigrationUpdateData'.
Using application base 'C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\bin\Debug\net5.0'.
Using working directory 'C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData'.
Using root namespace 'EFCoreMigrationUpdateData'.
Using project directory 'C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\'.
Finding DbContext classes...
Finding IDesignTimeDbContextFactory implementations...
Found IDesignTimeDbContextFactory implementation 'ApplicationDbContextFactory'.
Found DbContext 'ApplicationDbContext'.
Finding application service provider...
Finding Microsoft.Extensions.Hosting service provider...
No static method 'CreateHostBuilder(string[])' was found on class 'Program'.
No application service provider was found.
Finding DbContext classes in the project...
Using DbContext factory 'ApplicationDbContextFactory'.
Finding design-time services for provider 'Microsoft.EntityFrameworkCore.SqlServer'...
Using design-time services from provider 'Microsoft.EntityFrameworkCore.SqlServer'.
Finding design-time services referenced by assembly 'EFCoreMigrationUpdateData'.
No referenced design-time services were found.
Finding IDesignTimeServices implementations in assembly 'EFCoreMigrationUpdateData'...
No design-time services were found.
Context 'ApplicationDbContext' started tracking 'TestEntity' entity. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
Context 'ApplicationDbContext' started tracking 'TestEntity' entity. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
An 'TestEntity' entity tracked by 'ApplicationDbContext' changed from 'Added' to 'Unchanged'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
An 'TestEntity' entity tracked by 'ApplicationDbContext' changed from 'Added' to 'Unchanged'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
DetectChanges starting for 'ApplicationDbContext'.
DetectChanges completed for 'ApplicationDbContext'.
DetectChanges starting for 'ApplicationDbContext'.
Unchanged 'TestEntity.TimeZone' detected as changed and will be marked as modified. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see property values.
An 'TestEntity' entity tracked by 'ApplicationDbContext' changed from 'Unchanged' to 'Modified'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
DetectChanges completed for 'ApplicationDbContext'.
Context 'ApplicationDbContext' started tracking 'TestEntity' entity. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
Context 'ApplicationDbContext' started tracking 'TestEntity' entity. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
An 'TestEntity' entity tracked by 'ApplicationDbContext' changed from 'Added' to 'Unchanged'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
An 'TestEntity' entity tracked by 'ApplicationDbContext' changed from 'Added' to 'Unchanged'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
DetectChanges starting for 'ApplicationDbContext'.
DetectChanges completed for 'ApplicationDbContext'.
DetectChanges starting for 'ApplicationDbContext'.
Unchanged 'TestEntity.TimeZone' detected as changed and will be marked as modified. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see property values.
An 'TestEntity' entity tracked by 'ApplicationDbContext' changed from 'Unchanged' to 'Modified'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see key values.
DetectChanges completed for 'ApplicationDbContext'.
Writing migration to 'C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\Migrations\20200717050229_NoChanges2.cs'.
Writing model snapshot to 'C:\dev\bug-reports\EFCoreMigrationUpdateData\EFCoreMigrationUpdateData\Migrations\ApplicationDbContextModelSnapshot.cs'.
'ApplicationDbContext' disposed.
Done. To undo this action, use 'ef migrations remove'

-->

Further technical details

EF Core version: EF Core 3.1.6 (and prior) as well as EF Core 5.0.0-preview.6.20312.4
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.1 as well as .NET 5.0.100-preview.6.20318.15
Operating system: Windows 10
IDE: PowerShell or Visual Studio 2019 16.7.0 Preview 4.0

@smitpatel
Copy link
Contributor

Given TimeZoneInfo is a custom type with value converter, I suspect EF Core is failing to compare it correctly. Providing a custom value comparer along with value converter should fix the issue. See documentation on value comparers on how to configure EF Core.

@ChristopherHaws
Copy link
Contributor Author

ChristopherHaws commented Jul 17, 2020

@smitpatel I switched it to use the following and it still does not work properly:

entity.Property(x => x.TimeZone).IsRequired().HasDefaultValueSql("('UTC')")
    .HasConversion(x => x.Id, x => TimeZoneInfo.FindSystemTimeZoneById(x))
    .Metadata.SetValueComparer(new ValueComparer<TimeZoneInfo>((a, b) => a.Id == b.Id, x => x.GetHashCode()));

Also, why would this issue happen only when using HasDefaultValueSql?

FYI:

var a = TimeZoneInfo.FindSystemTimeZoneById("Eastern Standard Time");
var b = TimeZoneInfo.FindSystemTimeZoneById("Eastern Standard Time");

Console.WriteLine(a == b); // False
Console.WriteLine(a.Id == b.Id); // True
Console.WriteLine(a.GetHashCode() == b.GetHashCode()); // True
Console.WriteLine(a.Equals(b)); // True
Console.WriteLine(Object.Equals(a, b)); // True
Console.WriteLine(Object.ReferenceEquals(a, b)); // False

@ChristopherHaws
Copy link
Contributor Author

ChristopherHaws commented Jul 18, 2020

@smitpatel After playing around a bit, I can confirm that this has nothing to do with value conversion, but rather seeding any value (including built-in CLR types) that use ValueGeneratedOnAdd. I will update the title to reflect this.

The following entity setup also has the issue:

public class ApplicationDbContext : DbContext
{
    public ApplicationDbContext(DbContextOptions<ApplicationDbContext> options)
        : base(options)
    {
    }

    public DbSet<TestEntity> TestEntities { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<TestEntity>(entity =>
        {
            entity.HasKey(x => x.Code).IsClustered();
            entity.Property(x => x.TimeZone).ValueGeneratedOnAdd();

            entity.HasData(new TestEntity(
                code: "code-1",
                timeZone: "Eastern Standard Time"
            ));
        });
    }
}

public class TestEntity
{
    public TestEntity(string code, string timeZone)
    {
        Code = code;
        TimeZone = timeZone;
    }

    public string Code { get; set; }
    public string TimeZone { get; set; }
}

@ChristopherHaws ChristopherHaws changed the title Add migration with seed data that hasn't changed causes unnecessary UpdateData calls Using HasDefaultValueSql on a property that is seeded causes UpdateData calls in every subsequent migration Jul 18, 2020
@ChristopherHaws ChristopherHaws changed the title Using HasDefaultValueSql on a property that is seeded causes UpdateData calls in every subsequent migration Using ValueGeneratedOnAdd on a property that is seeded causes UpdateData calls in every subsequent migration Jul 18, 2020
ChristopherHaws pushed a commit to ChristopherHaws/efcore that referenced this issue Jul 18, 2020
@ChristopherHaws
Copy link
Contributor Author

I believe that I found the root cause of the issue, so I created a unit test and a PR with my changes. It looks like ValueGeneratedOnAdd properties were not being accounted for when comparing the values in the MigrationsModelDiffer.

ChristopherHaws pushed a commit to ChristopherHaws/efcore that referenced this issue Jul 21, 2020
@AndriySvyryd AndriySvyryd added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug labels Jul 21, 2020
@AndriySvyryd AndriySvyryd added this to the 5.0.0 milestone Jul 21, 2020
benoutram added a commit to dfe-analytical-services/explore-education-statistics that referenced this issue Aug 6, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-rc1 Aug 14, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-rc1, 5.0.0 Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations-seeding closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
4 participants