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

Migrations: Adding properties to owned types causes strange renames #9873

Closed
kykc opened this Issue Sep 21, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@kykc

kykc commented Sep 21, 2017

Observed behavior

When I'm trying to add another owned type property to already existing entity, instead of just adding new fields, migration renames one of the existing properties fields to the new name and creates new fields for already existing property. The resulting Up method looks like this:

        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.RenameColumn(
                name: "AmountCharged_Currency",
                table: "BusinessTransactions",
                newName: "ChargeFee_Currency");

            migrationBuilder.RenameColumn(
                name: "AmountCharged_Amount",
                table: "BusinessTransactions",
                newName: "ChargeFee_Amount");

            migrationBuilder.AddColumn<decimal>(
                name: "AmountCharged_Amount",
                table: "BusinessTransactions",
                type: "decimal(18, 2)",
                nullable: false,
                defaultValue: 0m);

            migrationBuilder.AddColumn<int>(
                name: "AmountCharged_Currency",
                table: "BusinessTransactions",
                type: "int",
                nullable: false,
                defaultValue: 0);
        }

Expected behavior

        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.AddColumn<decimal>(
                name: "ChargeFee_Amount",
                table: "BusinessTransactions",
                type: "decimal(18, 2)",
                nullable: false,
                defaultValue: 0m);

            migrationBuilder.AddColumn<int>(
                name: "ChargeFee_Currency",
                table: "BusinessTransactions",
                type: "int",
                nullable: false,
                defaultValue: 0);
        }

Steps to reproduce

Minimal testcase is better than a thousand words:

  1. Clone testcase repo
  2. Uncomment two lines in Program.cs
  3. Add-Migration TestNewField in nuget console

Note: this repo also contains borken branch which contains all migrations and snapshots as it happened on my side, so master is before, borken is after.

Further technical details

EF Core version: 2.0.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10 1703
IDE: Visual Studio 2017 15.3.5

@kykc

This comment has been minimized.

kykc commented Sep 22, 2017

Futher experiments:

  1. Setting field names explicitly like this achieves nothing:
var x = modelBuilder.Entity<BusinessTransaction>().OwnsOne(p => p.Profit);
x.Property(m => m.Amount).HasColumnName("Profit_Amount");
x.Property(m => m.Currency).HasColumnName("Profit_Currency");
  1. Setting owned type automatically for each property of desired type (MoneyAmount in this example) achieves nothing:
foreach (var entity in modelBuilder.Model.GetEntityTypes().ToList())
{
	foreach (var property in entity.ClrType.GetProperties().ToList())
	{
		if (property.PropertyType == typeof(MoneyAmount))
		{
			modelBuilder.Entity(entity.ClrType).OwnsOne(typeof(MoneyAmount), property.Name);
		}
	}
}
  1. If I correct generated Up and Down methods by hand further migrations (without owned types) and subsequent Update-Database calls seem to work as expected since generated snapshots are correct, the problem is only in the migration.
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.AddColumn<decimal>(
                name: "ChargeFee_Amount",
                table: "BusinessTransactions",
                type: "decimal(18, 2)",
                nullable: false,
                defaultValue: 0m);

            migrationBuilder.AddColumn<int>(
                name: "ChargeFee_Currency",
                table: "BusinessTransactions",
                type: "int",
                nullable: false,
                defaultValue: 0);
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropColumn(
                name: "ChargeFee_Amount",
                table: "BusinessTransactions");

            migrationBuilder.DropColumn(
                name: "ChargeFee_Currency",
                table: "BusinessTransactions");
        }
    }

Is it save thing to do? I've tried to never touch code-generated migrations before.

  1. There's a strange thing in the snapshot that I've spotted: for one field BusinessTransactionId isn't nullable, and for others it is:
                        b.OwnsOne("efc_minimal_testcase.MoneyAmount", "AmountCharged", b1 =>
                        {
                            b1.Property<int>("BusinessTransactionId");

and

                        b.OwnsOne("efc_minimal_testcase.MoneyAmount", "ChargeFee", b1 =>
                        {
                            b1.Property<int?>("BusinessTransactionId");

@ajcvickers ajcvickers added this to the 2.1.0 milestone Sep 22, 2017

@ajcvickers ajcvickers added the type-bug label Sep 22, 2017

@bricelam

This comment has been minimized.

Member

bricelam commented Sep 25, 2017

I suspect this was fixed by #9469. I'll confirm before closing.

@bricelam

This comment has been minimized.

Member

bricelam commented Sep 26, 2017

Negative. This was not fixed by that change.

@bricelam

This comment has been minimized.

Member

bricelam commented Sep 26, 2017

For properties on owned types we probably just need to include the navigation property when pairing up old and new properties.

@bricelam bricelam changed the title from Strange migrations behavior when adding owned type column to Migrations: Adding properties to owned types causes strange renames Jan 3, 2018

bricelam added a commit to bricelam/EFCore that referenced this issue Jan 5, 2018

bricelam added a commit to bricelam/EFCore that referenced this issue Jan 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment