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

Duplicate foreign key generated after RC1 -> RC2 migration #5406

Closed
beho opened this issue May 18, 2016 · 15 comments
Closed

Duplicate foreign key generated after RC1 -> RC2 migration #5406

beho opened this issue May 18, 2016 · 15 comments
Assignees
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@beho
Copy link

beho commented May 18, 2016

Two simple models:

public abstract class TicketBatch
{
    [Key, DatabaseGenerated( DatabaseGeneratedOption.Identity )]
    public int Id { get; set; }
    public List<Ticket> Tickets { get; set; }
}

public class Ticket
{
    [Key, DatabaseGenerated( DatabaseGeneratedOption.None )]
    public Guid Guid { get; set; }

    public int TicketBatchId { get; set; }

    public TicketBatch TicketBatch { get; set; }
}

TicketBatch has two descendants that define only value (non-relationship) properties.

After manual migration of the project to 1.0.0-rc2-final, EF Core now wants to apply following in new migration:

migrationBuilder.AddColumn<int>(
            name: "TicketBatchId1",
            table: "Tickets",
            nullable: true);

migrationBuilder.CreateIndex(
            name: "IX_Tickets_TicketBatchId1",
            table: "Tickets",
            column: "TicketBatchId1");

When I explicitly state that the foreign key is TicketBatchId in ApplicationDbContext.OnModelCreating:

builder.Entity<Ticket>()
    .HasOne( t => t.TicketBatch )
    .WithMany( b => b.Tickets )
    .HasForeignKey( t => t.TicketBatchId );

then new column is not added. But that should not be necessary as conventions are followed.

Only similar issue I found is #5344 but I did not dig deeper.

Thanks!

@smitpatel
Copy link
Contributor

Explicit configuration should not be required. The only reason Migration would generate a different FK because it did not find it in ModelSnapshot file. Can you share ModelSnapshot file from RC1 time if you have. (It gets re-written after adding new migration so the present one might be different).

ModelSnapshot file has changed over the time due to some bug fixes. I am not sure if the file from RC1 would work straight-forward with RC2.
@rowanmiller - Does the same file should work or Migrations/ModelSnapshot should be deleted while upgrading?

@rowanmiller
Copy link
Contributor

I think this is model building, not migrations. It looks like we are introducing a shadow FK for the relationship (TicketBatchId1) rather than using the one that should match by convention (TicketBatchId).

@beho can you share the complete model so that we can work out why it is doing this?

@beho
Copy link
Author

beho commented May 19, 2016

Is there a private channel I could use for handing over model sources ?

@rowanmiller
Copy link
Contributor

rowanmiller commented May 23, 2016

@beho feel free to email it thru (just include a reference to the issue #)

@oyvindvol
Copy link

I am also having this problem. I migrated from RC1 to RC2, and I see a shadow FK in the generated queries in sql profiler. I have a large data model, but this problems seems to occur only for one of my tables.

I have a data model like this: Todo -|-----< Delegation

Also, if I try to create a new migration with the dotnet ef migrations add command, I get a .AddColumn<int>() trying to add the new shadow FK (TodoId1) to the Delegation table. In addition all the foreign keys are deleted and added again, all the primary keys are deleted and added again (this was a problem in RC1?) and all the tables are renamed (I know that this is supposed to happen when moving to EF Core).

@rowanmiller
Copy link
Contributor

@beho can you confirm that if you delete your RC1 migrations and then regenerate you get the correct thing?

@oyvindvol
Copy link

@rowanmiller I didn't get the right thing after deleting RC1 migrations.

@beho
Copy link
Author

beho commented May 26, 2016

@rowanmiller I deleted both migrations and model snapshot but new clean-slate migration still contains both TicketBatchId and TicketBatchId1.

Same with latest dev version from https://www.myget.org/gallery/aspnetvnext.

@divega
Copy link
Contributor

divega commented May 27, 2016

Thanks @beho for the full repro. In case it helps you workaround the issue, the Guid property on TicketBatch is what is triggering the behavior.

Here is a minimal repro:

using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;

namespace Repro5406
{
    public class TicketBatch
    {
        [Key] 
        public int Id { get; set; }
        // if the Guid property is present a shadow TicketBatchId1 FK is added to Ticket 
        // to support the one-to-many relationship
        public Guid Guid { get; set; } 
        public List<Ticket> Tickets { get; set; }
    }

    public class Ticket
    {
        [Key] 
        public Guid Guid { get; set; }

        public int TicketBatchId { get; set; }

        public TicketBatch TicketBatch { get; set; }
    }

   public class ApplicationDbContext: DbContext 
    {

        public DbSet<TicketBatch> Batches { get; set; }

        public DbSet<Ticket> Tickets { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer("Data Source=(localdb)\\mssqllocaldb;Initial Catalog=repro5406:Integrated Security=yes");
        }
    }
}

@divega
Copy link
Contributor

divega commented May 27, 2016

Assigning this to @smitpatel for investigation.

@smitpatel
Copy link
Contributor

Issue here is matching properties:
Somewhere in past we decided that if for one-to-one relationship if both the sides have matching properties then it is ambiguity therefore we do not do anything with relationship. This later throws exception as principal/dependent side are not set for one-to-one relationship.
Though currently, this logic is being applied to one-to-many relationship also. For the case of one-to-many relationship, we leave relationship as is. Though this doesn't throw any exception and we end up with a relationship with shadow fk property in final model.
IMO, we should not take matching properties on both side as "ambiguity" for the case of one-to-many relationship since we already know what is principal side. @AndriySvyryd - thoughts?

@divega
Copy link
Contributor

divega commented May 27, 2016

We agreed this should be fixed in RTM with @smitpatel @bricelam and @ajcvickers.

@jcmontx
Copy link

jcmontx commented Aug 19, 2016

@divega I'm having the same issue in production.

This is my relation:

modelBuilder.Entity<Consulta>(entity => { 
                entity.Property(c => c.ConsultaId)
                    .ValueGeneratedOnAdd();
                entity.HasOne(c => c.Auto)
                .WithMany(a => a.Consultas)
                .OnDelete(Microsoft.EntityFrameworkCore.Metadata.DeleteBehavior.Restrict);
            });

And this is the table I'm getting:
image

Although, when I perform an insert, the field "AutoId" is filled with the proper FK and "AutoId1" is ignored and is always null. Am I doing anything wrong or the bug haven't been fixed yet?

@divega
Copy link
Contributor

divega commented Aug 20, 2016

@jcmontes95 I am not sure it is exactly the same issue as the original. If this is happening with RTM bits, can you please create a new issue with a full repro?

@jcmontx
Copy link

jcmontx commented Aug 22, 2016

@divega Indeed, it is happening with RTM bits. I'll create the new issue with its repro soon.

@AndriySvyryd AndriySvyryd added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. area-model-building labels Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

7 participants