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 PK with HasColumnType causes strange migrations for Owned Types #27619

Closed
midczak opened this issue Mar 10, 2022 · 6 comments
Closed

Using PK with HasColumnType causes strange migrations for Owned Types #27619

midczak opened this issue Mar 10, 2022 · 6 comments
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-bug
Milestone

Comments

@midczak
Copy link

midczak commented Mar 10, 2022

Configuring table with PK that have custom column type (HasColumnType) and conversion (HasConversion) produces strange results in migrations

Steps to reproduce:

  1. Create entities with configuration like below
    public class OrderId
    {
        public Guid Value { get; set; }
    }

    public class Order
    {
        public OrderId Id { get; set; }
        public StreetAddress ShippingAddress { get; set; }
    }

    public class StreetAddress
    {
        public string Street { get; set; }
        public string City { get; set; }
    }

    internal class OrderConfiguration : IEntityTypeConfiguration<Order>
    {
        public void Configure(EntityTypeBuilder<Order> builder)
        {
            builder.HasKey(x => x.Id);
            builder.Property(x => x.Id)
                .HasColumnType("binary(16)")
                .HasConversion(v => v.Value, v => new OrderId { Value = v });

            builder.OwnsOne(
                o => o.ShippingAddress,
                sa =>
                {
                    sa.Property(p => p.Street).IsRequired();
                    sa.Property(p => p.City).IsRequired();
                });

            builder.Navigation(o => o.ShippingAddress)
                .IsRequired();
        }
    }
  1. run Add-Migration
public partial class Migration1 : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.CreateTable(
                name: "Orders",
                columns: table => new
                {
                    Id = table.Column<Guid>(type: "binary(16)", nullable: false),
                    ShippingAddress_Street = table.Column<string>(type: "nvarchar(max)", nullable: false),
                    ShippingAddress_City = table.Column<string>(type: "nvarchar(max)", nullable: false)
                },
                constraints: table =>
                {
                    table.PrimaryKey("PK_Orders", x => x.Id);
                });
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropTable(
                name: "Orders");
        }
    }

The result is correct

  1. Run Add-Migration again.
 public partial class Migration2: Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropForeignKey(
                name: "FK_Orders_Orders_ShippingAddress_OrderId",
                table: "Orders");

            migrationBuilder.DropUniqueConstraint(
                name: "AK_Orders_TempId",
                table: "Orders");

            migrationBuilder.DropColumn(
                name: "ShippingAddress_OrderId",
                table: "Orders");

            migrationBuilder.DropColumn(
                name: "TempId",
                table: "Orders");
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.AddColumn<byte[]>(
                name: "ShippingAddress_OrderId",
                table: "Orders",
                type: "binary(16)",
                nullable: false,
                defaultValue: new byte[0]);

            migrationBuilder.AddColumn<byte[]>(
                name: "TempId",
                table: "Orders",
                type: "varbinary(900)",
                nullable: false,
                defaultValue: new byte[0]);

            migrationBuilder.AddUniqueConstraint(
                name: "AK_Orders_TempId",
                table: "Orders",
                column: "TempId");

            migrationBuilder.AddForeignKey(
                name: "FK_Orders_Orders_ShippingAddress_OrderId",
                table: "Orders",
                column: "ShippingAddress_OrderId",
                principalTable: "Orders",
                principalColumn: "TempId",
                onDelete: ReferentialAction.Cascade);
        }
    }

Migration produces unexpected results, event there were no changes in the model. Every next migration will produce this result.

Include provider and version information

EF Core version: 5.0.15
Database provider: tested on Microsoft.EntityFrameworkCore.SqlServer and Pomelo.EntityFrameworkCore.MySql
Target framework: .NET 5.0
Operating system: Windows 10
IDE: VS 2022 (version 17.1.0)

@midczak midczak changed the title PK with HasColumnType generates strange migration for Owned Types Using PK with HasColumnType causes strange migrations for Owned Types Mar 11, 2022
@ajcvickers
Copy link
Member

@midczak Have you tried this with 6.0.3?

@midczak
Copy link
Author

midczak commented Mar 11, 2022

@ajcvickers Just tested with 6.0.3, the bug is still there

@midczak
Copy link
Author

midczak commented Mar 14, 2022

@ajcvickers Is there any workaround for this in v5.* or v6.*?

@ajcvickers
Copy link
Member

@midczak Try converting directly to binary. This will avoid EF Core composing two converters together to go from your type to Guid, and then Guid to binary.

@AndriySvyryd
Copy link
Member

Related: #27549

@ajcvickers ajcvickers added the punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. label Sep 16, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0, Backlog Sep 16, 2022
@AndriySvyryd AndriySvyryd added the verify-fixed This issue is likely fixed in new query pipeline. label Oct 4, 2022
@bricelam
Copy link
Contributor

bricelam commented Aug 1, 2023

Verified that this has been fixed sometime during the 7.0 release.

@bricelam bricelam closed this as completed Aug 1, 2023
@bricelam bricelam modified the milestones: Backlog, 7.0.0 Aug 1, 2023
@AndriySvyryd AndriySvyryd removed their assignment Aug 2, 2023
@AndriySvyryd AndriySvyryd added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed verify-fixed This issue is likely fixed in new query pipeline. labels Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-bug
Projects
None yet
Development

No branches or pull requests

4 participants