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 primary key constraint names after calling SetName on the entity type in a TPT scenario #22753

Closed
sandord opened this issue Sep 25, 2020 · 5 comments

Comments

@sandord
Copy link

sandord commented Sep 25, 2020

I'm on EF Core version 5.0.0-rc.2.20472.13 with SqlLite and Postgres and I noticed that I got clashing primary key constraint names in the generated migration.

Here's a complete sample code that reproduces the problem:

using Microsoft.EntityFrameworkCore;

namespace ConsoleApp6
{
    public static class Program
    {
        private static void Main()
        {
        }
    }

    public abstract class Player
    {
        public int Id { get; set; }
    }

    public class Hitter : Player
    {
        public int HomeRuns { get; set; }
    }

    public class Pitcher : Player
    {
        public int Strikeouts { get; set; }
    }

    public class BaseballDbContext : DbContext
    {
        protected override void OnConfiguring(DbContextOptionsBuilder options)
        {
            options.UseSqlite("Data Source=baseball.db");
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            // Map using TPT strategy.
            modelBuilder.Entity<Player>().ToTable("players");
            modelBuilder.Entity<Hitter>().ToTable("hitters");
            modelBuilder.Entity<Pitcher>().ToTable("pitchers");

            foreach (var entityType in modelBuilder.Model.GetEntityTypes())
            {
                var primaryKey = entityType.FindPrimaryKey();
                primaryKey.SetName(primaryKey.GetName().ToLowerInvariant());
            }
        }
    }
}

Here's the generated migration code. It assigns the same pk_players constraint name to each of the table's primary keys.

migrationBuilder.CreateTable(
    name: "Players",
    columns: table => new
    {
        Id = table.Column<int>(type: "INTEGER", nullable: false)
            .Annotation("Sqlite:Autoincrement", true)
    },
    constraints: table =>
    {
        table.PrimaryKey("pk_players", x => x.Id);    // <----- pk_players
    });

migrationBuilder.CreateTable(
    name: "hitters",
    columns: table => new
    {
        Id = table.Column<int>(type: "INTEGER", nullable: false)
            .Annotation("Sqlite:Autoincrement", true),
        HomeRuns = table.Column<int>(type: "INTEGER", nullable: false)
    },
    constraints: table =>
    {
        table.PrimaryKey("pk_players", x => x.Id);    // <----- pk_players
        table.ForeignKey(
            name: "FK_hitters_Players_Id",
            column: x => x.Id,
            principalTable: "Players",
            principalColumn: "Id",
            onDelete: ReferentialAction.Cascade);
    });

migrationBuilder.CreateTable(
    name: "pitchers",
    columns: table => new
    {
        Id = table.Column<int>(type: "INTEGER", nullable: false)
            .Annotation("Sqlite:Autoincrement", true),
        Strikeouts = table.Column<int>(type: "INTEGER", nullable: false)
    },
    constraints: table =>
    {
        table.PrimaryKey("pk_players", x => x.Id);    // <----- pk_players
        table.ForeignKey(
            name: "FK_pitchers_Players_Id",
            column: x => x.Id,
            principalTable: "Players",
            principalColumn: "Id",
            onDelete: ReferentialAction.Cascade);
    });

While SqlLite is fine with non-unique primary key constraint names, Postgres isn't and throws an SQL exception when applying the migration.

This problem is triggered by the following call, which in my opinion should be perfectly safe:

primaryKey.SetName(primaryKey.GetName().ToLowerInvariant());
@sandord sandord changed the title Changing primary key constraint name causes name from hierarchy root being used in TPT scenario Duplicate primary key names after calling SetName on the entity type in a TPT scenario Sep 25, 2020
@ajcvickers
Copy link
Member

@roji Will this impact the PostgreSQL table naming plug-in? (i.e. Does it try to set the PK constraint name like this?)

@sandord This is a limitation with the current design; we will need new API surface to be able to specify a different explicit constraint name for each table, since in the EF model all the types share a primary key. For now, the workaround is to rename the constraints in the migration.

@AndriySvyryd
Copy link
Member

Duplicate of #19811

@AndriySvyryd AndriySvyryd marked this as a duplicate of #19811 Sep 29, 2020
@sandord sandord changed the title Duplicate primary key names after calling SetName on the entity type in a TPT scenario Duplicate primary key constraint names after calling SetName on the entity type in a TPT scenario Sep 29, 2020
@sandord
Copy link
Author

sandord commented Sep 29, 2020

Duplicate of #19811

Not quite. This is about constraint names, not column names. I've updated the title and body to reflect this more explicitly.

@sandord
Copy link
Author

sandord commented Sep 29, 2020

@sandord This is a limitation with the current design; we will need new API surface to be able to specify a different explicit constraint name for each table, since in the EF model all the types share a primary key. For now, the workaround is to rename the constraints in the migration.

Thanks for looking into this. It still it feels a bit odd though that even doing primaryKey.SetName(primaryKey.GetName()); has the side effect of the constraint names changing instead of staying the same.

The proposed workaround is a bit painful for us since we're using tooling to automatically recreate initial migrations during the development stage.

@AndriySvyryd
Copy link
Member

Not quite. This is about constraint names, not column names. I've updated the title and body to reflect this more explicitly.

#19811 also covers keys, indexes and foreign keys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants