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

EFCore 1.1 doesn't use an existing property for the FK if it's part of the primary key #7127

Closed
gius opened this issue Nov 25, 2016 · 5 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@gius
Copy link

gius commented Nov 25, 2016

The problem occurs in much complicated entities setup but I have isolated the problem to the following.

Steps to reproduce

Have entity whose primary key is also a reference:

public class OrderDetail
{
  public int OrderID { get; set; }
  public Order Order { get; set; }
  ...
}
// in DbContext.OnModelCreating

modelBuilder.Entity<OrderDetail>().HasKey(x => x.OrderID);

The issue

Versions prior to EFCore version 1.1 created both, primary key as well as foreign key, all worked properly. After updating to version 1.1, SQL generated by EF is wrong - it expects a new column OrderID1.

Obviously the model is wrong - when adding a new migration (before the EF update, this migration would be empty), the migration tries to drop foreign key "FK_OrderDetail_Order_OrderID", drop index "IX_OrderDetail_OrderID", alter column "OrderID" to make it "SqlServerValueGenerationStrategy.IdentityColumn", add new column "OrderID1" (nullable), and create index and foreign key for the new column "OrderID1".

Note that the reference from OrderDetail to Order tables is not explicitly described, can be only inferred from OrderID and Order properties.

Is this expected?

Further technical details

EF Core version: 1.1.0
Operating system: Windows 10
Visual Studio version: VS 2015

Update 1

I've tried to explicitly set the reference as follows:

// in DbContext.OnModelCreating
modelBuilder.Entity<OrderDetail>().HasKey(x => x.OrderID);
modelBuilder.Entity<OrderDetail>().HasOne(x => x.Order).WithOne();

And it did not help. However, changing the order of the lines seems to have solved the problem:

// in DbContext.OnModelCreating
modelBuilder.Entity<OrderDetail>().HasOne(x => x.Order).WithOne();
modelBuilder.Entity<OrderDetail>().HasKey(x => x.OrderID);

Update 2

It also seems that many of implicit indexes on foreign keys have been removed but explicit call to .HasIndex fixes this. Especially indexes on primary keys that are also foreign keys and indexes on the first column of composed primary keys.

@AndriySvyryd
Copy link
Member

@gius Could you provide a complete model and context? I am not able to repro the issue with just the above code.
Note that indexes being removed on the primary key is by design - they are redundant.

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Dec 8, 2016

@gius Nevermind, I found the issue. It is still by design that EF doesn't use OrderID if the relationship hasn't been specified because you only have one navigation property, so the relationship might be one-to-many. However adding

modelBuilder.Entity<OrderDetail>().HasOne(x => x.Order).WithOne();

should be enough, independently of the order.

@AndriySvyryd
Copy link
Member

Fixed in 16eb5b3

@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 8, 2016
@AndriySvyryd AndriySvyryd removed their assignment Dec 21, 2016
@AndriySvyryd AndriySvyryd reopened this Jan 11, 2017
@AndriySvyryd AndriySvyryd changed the title EFCore 1.1 Ignores implicit reference when part of primary key EFCore 1.1 doesn't use an existing property for the FK if it's part of the primary key Jan 13, 2017
@Eilon
Copy link
Member

Eilon commented Jan 19, 2017

This patch bug is approved. Please use the normal code review process w/ a PR and make sure the fix is in the correct branch, then close the bug and mark it as done.

@anpete
Copy link
Contributor

anpete commented Feb 23, 2017

✅ Verified

@AndriySvyryd AndriySvyryd removed their assignment Apr 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

6 participants