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

EF Core 3.0 : Why does it automatically generate a unique index? #15512

Closed
ZhangGaoxing opened this issue Apr 28, 2019 · 9 comments · Fixed by #18521
Closed

EF Core 3.0 : Why does it automatically generate a unique index? #15512

ZhangGaoxing opened this issue Apr 28, 2019 · 9 comments · Fixed by #18521
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-3.0 type-bug
Milestone

Comments

@ZhangGaoxing
Copy link

I have a class (I removed some irrelevant properties)

public class Comment
{
    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public long CommentID { get; set; }

    public long? ReplyCommentID { get; set; }

    public long? ParentCommentID { get; set; }

    [ForeignKey("ParentCommentID")]
    public virtual Comment ParentComment { get; set; }

    [ForeignKey("ReplyCommentID")]
    public virtual Comment ReplyComment { get; set; }
}

In my DbContext

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Comment>()
        .ToTable("Comments")
        .HasQueryFilter(x => x.IsDeleted == false);

    var cascadeFKs = modelBuilder.Model.GetEntityTypes()
        .SelectMany(t => t.GetForeignKeys())
        .Where(fk => !fk.IsOwnership && fk.DeleteBehavior == DeleteBehavior.Cascade);
    foreach (var fk in cascadeFKs)
        fk.DeleteBehavior = DeleteBehavior.Restrict;
}

Steps to reproduce

In PMC run this:

PM> dotnet ef migrations add init

After run it, the tool generates the following:

migrationBuilder.CreateIndex(
    name: "IX_Comments_ParentCommentID",
    table: "Comments",
    column: "ParentCommentID",
    unique: true,
    filter: "[ParentCommentID] IS NOT NULL");

migrationBuilder.CreateIndex(
    name: "IX_Comments_ReplyCommentID",
    table: "Comments",
    column: "ReplyCommentID");

Interestingly, only ParentCommentID generates a unique index, while ReplyCommentID does not. I don't want to specify IsUnique(false) in DbContext. So, what exactly is this situation? 😄

Further technical details

EF Core version: 3.0.0-preview3.19153.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10 1903
IDE: Visual Studio 2019 16.0.2

@divega divega added this to the 3.0.0 milestone Apr 29, 2019
@divega
Copy link
Contributor

divega commented Apr 29, 2019

Note from triage: we believe that in this case we should throw because it is non-deterministic whether the FK should be unique.

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Apr 29, 2019

But we'll also try just configuring it as non-unique to avoid a breaking change.

@ajcvickers
Copy link
Contributor

@divega @AndriySvyryd Why is it not just two one-to-many relationships? (In other words, what is it that makes these different from a normal reference-only navigation setup, which we configure as one-to-many?)

@AndriySvyryd
Copy link
Member

@ajcvickers They are detected as one one-to-one relationship and then split into two once we process the data annotations.

@ajcvickers
Copy link
Contributor

@AndriySvyryd Why 1:1?

@AndriySvyryd
Copy link
Member

@ajcvickers Because both navigations are not collections

@ajcvickers
Copy link
Contributor

@AndriySvyryd But isn't this two different relationships? (Since there are two FKs, and each navigation is associated with a different FK. So the two navigations cannot be inverses of the same relationship. Which means we have one unidirectional reference navigation property for each relationship...which means one-to-many because this will work for one-to-many or one-to-one (if used as one-to-one) even if it isn't fully constrained to one-to-one.)

I'm clearly missing something critical here, but I don't know what question to ask to figure it out!

@AndriySvyryd
Copy link
Member

@ajcvickers Yes, that's what should happen, but before the FKs are associated with the navigations they are configured as a single one-to-one relationship and later it becomes a unidirectional one-to-one relationship.

@ajcvickers
Copy link
Contributor

Talked to @AndriySvyryd in person and agreed that this should be two 1:* relationships and so the correct fix (i.e. not just to avoid a breaking change) is to make the FK not unique. (Note that this may be hard to implement due to the design of the model building pipeline.)

Throwing as an ambiguous case would not be the correct fix here; we would only do this if making the correct fix is prohibitively expensive, and we can't come up with anything else.

@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog Jun 28, 2019
@AndriySvyryd AndriySvyryd modified the milestones: Backlog, 3.1.0 Sep 5, 2019
@AndriySvyryd AndriySvyryd removed their assignment Oct 22, 2019
@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 Oct 22, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0, 3.1.0-preview2 Oct 24, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0-preview2, 3.1.0 Dec 2, 2019
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. customer-reported punted-for-3.0 type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants