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

TPC uses multiple sequences instead of a single one #35363

Open
samyonr opened this issue Dec 20, 2024 · 0 comments
Open

TPC uses multiple sequences instead of a single one #35363

samyonr opened this issue Dec 20, 2024 · 0 comments

Comments

@samyonr
Copy link

samyonr commented Dec 20, 2024

In the documentation of EF Core Inheritance, the Key generation section states that a single key should be created in TPC strategy:

Things get a bit more complicated for TPC. First, it’s important to understand that EF Core requires that all entities in a hierarchy have a unique key value, even if the entities have different types. For example, using our example model, a Dog cannot have the same Id key value as a Cat. Second, unlike TPT, there is no common table that can act as the single place where key values live and can be generated. This means a simple Identity column cannot be used.

For databases that support sequences, key values can be generated by using a single sequence referenced in the default constraint for each table. This is the strategy used in the TPC tables shown above, where each table has the following:

[Id] int NOT NULL DEFAULT (NEXT VALUE FOR [AnimalSequence])

AnimalSequence is a database sequence created by EF Core. This strategy is used by default for TPC hierarchies when using the EF Core database provider for SQL Server.

A similar example appears in the Table-per-concrete-type configuration section.

That's indeed the case, most of the time. However, certain configurations generate separate sequences per concrete type, until a DbSet<> of the abstract class is added.

See the full example in the following gist, or here: EFCoreDropSequenceOrder.zip

Here's the entities' setup:

public interface IEntity
{
    public int Id { get; set; }
}

public abstract class Pet : IEntity // Step 3: rename Animal to Pet and add migration
{
    // added in Step 1
    public int Id { get; set; }
    public int Age { get; set; }
    
    // added in Step 2
    public string? Name { get; set; }
}

public class Dog : Pet
{
    public bool LovesChasingSticks { get; set; }
}

public class Cat : Pet
{
    public bool LovesSleeping { get; set; }
}

In the project, there are three migrations. In the first one, the abstract class was called Animal instead of Pet, and it didn't include the Name property. Then, I added the Name property (the second migration) and renamed Animal to Pet (the third migration).

Here's the db context:

public class AppDbContext(DbContextOptions options) : DbContext(options)
{
    // Step 1: Add the DbSet properties for concrete types, generate migration
    public DbSet<Dog> Dogs { get; set; }
    public DbSet<Cat> Cats { get; set; }
    
    // Step 2: Add the DbSet<Animal> property for the base abstract type, generate migration
    // Step 3: Rename the Animal abstract class to Pet, generate migration
    public DbSet<Pet> Animals { get; set; }
    
    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);
        modelBuilder.HasDefaultSchema("AnimalsDb");
        
        // instead of:
        // modelBuilder.Entity<Animal>(entity => { entity.UseTpcMappingStrategy(); });
        // modelBuilder.Entity<Dog>(entity => { entity.UseTpcMappingStrategy(); });
        // modelBuilder.Entity<Cat>(entity => { entity.UseTpcMappingStrategy(); });
        // do it with reflection:
        var entityTypes = modelBuilder.Model.GetEntityTypes()
            .Where(t => typeof(IEntity).IsAssignableFrom(t.ClrType) && !t.ClrType.IsInterface);

        foreach (var clrType in entityTypes.Select(e => e.ClrType))
        {
            modelBuilder.Entity(clrType, entity => { entity.Property<int>(nameof(IEntity.Id)).ValueGeneratedOnAdd(); });
            modelBuilder.Entity(clrType).UseTpcMappingStrategy();
        }
    }
}

If I'm using the commented-out code, all works well. i.e. a single sequence is generated and used by both tables. But instead, in my setup, all the mapped entities implement the IEntity interface. In my full setup, hundreds of mapped entities implement a similar IEntity interface, adding multiple properties, like CreatedAt and LastUpdatedAt. With so many entities, interfaces, and abstract classes, it makes sense to use reflection instead of explicitly mapping each one. Above is a simplified version that reproduces the issue, resembling the full setup.

As with the entities, it also includes three steps. In the first one, DbSet<> properties for Dog and Cat are available, but not for Animal. In the second step, a DbSet<> for Animal is added, and in the third step, Animal is renamed Pet.

Here's the first migration:

protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.EnsureSchema(
                name: "AnimalsDb");

            migrationBuilder.CreateSequence(
                name: "CatSequence",
                schema: "AnimalsDb");

            migrationBuilder.CreateSequence(
                name: "DogSequence",
                schema: "AnimalsDb");

            migrationBuilder.CreateTable(
                name: "Cats",
                schema: "AnimalsDb",
                columns: table => new
                {
                    Id = table.Column<int>(type: "integer", nullable: false, defaultValueSql: "nextval('\"AnimalsDb\".\"CatSequence\"')"),
                    LovesSleeping = table.Column<bool>(type: "boolean", nullable: false),
                    Age = table.Column<int>(type: "integer", nullable: false)
                },
                constraints: table =>
                {
                    table.PrimaryKey("PK_Cats", x => x.Id);
                });

            migrationBuilder.CreateTable(
                name: "Dogs",
                schema: "AnimalsDb",
                columns: table => new
                {
                    Id = table.Column<int>(type: "integer", nullable: false, defaultValueSql: "nextval('\"AnimalsDb\".\"DogSequence\"')"),
                    LovesChasingSticks = table.Column<bool>(type: "boolean", nullable: false),
                    Age = table.Column<int>(type: "integer", nullable: false)
                },
                constraints: table =>
                {
                    table.PrimaryKey("PK_Dogs", x => x.Id);
                });
        }

In it, two sequences are added: CatSequence and DogSequence, contradicting the documentation. That wasn't a problem for me until I added the second migration, which adds a DbSet<Animal> to the context (which is useful for various APIs, e.g. GraphQL):

protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropSequence(
                name: "CatSequence",
                schema: "AnimalsDb");

            migrationBuilder.DropSequence(
                name: "DogSequence",
                schema: "AnimalsDb");

            migrationBuilder.CreateSequence(
                name: "AnimalSequence",
                schema: "AnimalsDb");

            migrationBuilder.AlterColumn<int>(
                name: "Id",
                schema: "AnimalsDb",
                table: "Dogs",
                type: "integer",
                nullable: false,
                defaultValueSql: "nextval('\"AnimalsDb\".\"AnimalSequence\"')",
                oldClrType: typeof(int),
                oldType: "integer",
                oldDefaultValueSql: "nextval('\"AnimalsDb\".\"DogSequence\"')");

            migrationBuilder.AddColumn<string>(
                name: "Name",
                schema: "AnimalsDb",
                table: "Dogs",
                type: "text",
                nullable: true);

            migrationBuilder.AlterColumn<int>(
                name: "Id",
                schema: "AnimalsDb",
                table: "Cats",
                type: "integer",
                nullable: false,
                defaultValueSql: "nextval('\"AnimalsDb\".\"AnimalSequence\"')",
                oldClrType: typeof(int),
                oldType: "integer",
                oldDefaultValueSql: "nextval('\"AnimalsDb\".\"CatSequence\"')");

            migrationBuilder.AddColumn<string>(
                name: "Name",
                schema: "AnimalsDb",
                table: "Cats",
                type: "text",
                nullable: true);
        }

This migration is problematic because:

  1. The order in which it is executed won't work. Deleting the previous sequences, adding a new one, and only then altering the table causes: Npgsql.PostgresException : 2BP01: cannot drop sequence "<some-name>" because other objects depend on it To fix it, the order should be: add the new sequence, alter the table, and remove the old sequences. See related issue: Migrations command in incorrect order when dealing with sequences  #33130.
  2. Even if the order of migration operations is correct, there's still a problem: the IDs between the Dog and the Cat tables aren't unique. A single sequence is required to guarantee the uniqueness of the IDs within these two tables. Now, one has to find some migration algorithm, like: updating all Dog IDs to be even and Cat IDs to be odd (harder with more tables that use the same sequence), updating all the foreign keys, etc. (such operation is probably out of scope of EF Core). This problem would be avoided if a single sequence was used in the first place.

The third migration, renaming Animal to Pet, has the same wrong order of operations as in #33130:

protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropSequence(
                name: "AnimalSequence",
                schema: "AnimalsDb");

            migrationBuilder.CreateSequence(
                name: "PetSequence",
                schema: "AnimalsDb");

            migrationBuilder.AlterColumn<int>(
                name: "Id",
                schema: "AnimalsDb",
                table: "Dogs",
                type: "integer",
                nullable: false,
                defaultValueSql: "nextval('\"AnimalsDb\".\"PetSequence\"')",
                oldClrType: typeof(int),
                oldType: "integer",
                oldDefaultValueSql: "nextval('\"AnimalsDb\".\"AnimalSequence\"')");

            migrationBuilder.AlterColumn<int>(
                name: "Id",
                schema: "AnimalsDb",
                table: "Cats",
                type: "integer",
                nullable: false,
                defaultValueSql: "nextval('\"AnimalsDb\".\"PetSequence\"')",
                oldClrType: typeof(int),
                oldType: "integer",
                oldDefaultValueSql: "nextval('\"AnimalsDb\".\"AnimalSequence\"')");
        }

Here, as before, the sequence is first dropped, then a new sequence is created, and only then the table is altered.

I'm using:
Microsoft.EntityFrameworkCore.Design Version="9.0.0"
Microsoft.EntityFrameworkCore.Relational Version="9.0.0"
Npgsql.EntityFrameworkCore.PostgreSQL Version="9.0.2"
net9.0

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

1 participant