dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.48k stars 3.13k forks source link

Clustered index not created for alternate key when property is named "Id" #30133

Open ajaffie opened 1 year ago

ajaffie commented 1 year ago

Issue

When specifying a clustered index for an alternate key and the property is named "Id", the clustered index is not added to the model or subsequent migrations. I attached a full repro project, but here are some relevant snippets:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    base.OnModelCreating(modelBuilder);
    modelBuilder.Entity<MyEntity>(builder =>
    {
        builder.Property(e => e.Id)
            .HasValueGenerator<SequentialGuidValueGenerator>()
            .ValueGeneratedOnAdd();
        builder.HasAlternateKey(e => e.Id)
            .IsClustered();
        builder.HasKey(e => e.PrimaryKey)
            .IsClustered(false);
    });
}
protected override void Up(MigrationBuilder migrationBuilder)
{
    migrationBuilder.CreateTable(
        name: "MyEntity",
        columns: table => new
        {
            PrimaryKey = table.Column<Guid>(type: "uniqueidentifier", nullable: false),
            Id = table.Column<Guid>(type: "uniqueidentifier", nullable: false),
            Data = table.Column<string>(type: "nvarchar(max)", nullable: false)
        },
        constraints: table =>
        {
            table.PrimaryKey("PK_MyEntity", x => x.PrimaryKey)
                .Annotation("SqlServer:Clustered", false);
        });
}
modelBuilder.Entity("ClusteredAlternateKeyRepro.MyEntity", b =>
    {
        b.Property<Guid>("PrimaryKey")
            .ValueGeneratedOnAdd()
            .HasColumnType("uniqueidentifier");

        b.Property<string>("Data")
            .IsRequired()
            .HasColumnType("nvarchar(max)");

        b.Property<Guid>("Id")
            .ValueGeneratedOnAdd()
            .HasColumnType("uniqueidentifier");

        b.HasKey("PrimaryKey");

        SqlServerKeyBuilderExtensions.IsClustered(b.HasKey("PrimaryKey"), false);

        b.ToTable("MyEntity");
    });

Workaround

To work around this, I renamed the property in code and mapped to the "Id" column to avoid renaming the existing column.

EF Core version: Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 6, 7 Operating system: Windows IDE: Rider ClusteredAlternateKeyRepro.zip

roji commented 1 year ago

This doesn't seem to be related specifically to index clustering - the alternative key Id doesn't even get a unique index created for it:

await using var ctx = new BlogContext();
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();

public class BlogContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(@"Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0;Encrypt=false")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Blog>(builder =>
        {
            builder.HasAlternateKey(e => e.Id);
            builder.HasKey(e => e.PrimaryKey);
        });
    }
}

public class Blog
{
    public int PrimaryKey { get; set; }
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
}

Resulting table:

CREATE TABLE [Blogs] (
    [PrimaryKey] int NOT NULL IDENTITY,
    [Id] int NOT NULL,
    [FirstName] nvarchar(max) NOT NULL,
    [LastName] nvarchar(max) NOT NULL,
    CONSTRAINT [PK_Blogs] PRIMARY KEY ([PrimaryKey])
);

Renaming Id to Id2 makes this go away. Changing the order and defining PrimaryKey as the key first also makes it go away.

ajcvickers commented 1 year ago

Note for triage: not a regression.