npgsql / efcore.pg

Entity Framework Core provider for PostgreSQL
PostgreSQL License
1.52k stars 223 forks source link

Migrations references actual enum types which break them #3211

Closed snebjorn closed 2 months ago

snebjorn commented 2 months ago

Using the actual enum type in migrations is problematic - I'll explain

Given this model

public enum Mood
{
    [PgName("thrilled")]
    Happy,
    Sad,
}

public class Blog
{
    public int Id { get; set; }
    public Mood Mood { get; set; }
}

Generate our initial migration and apply it to the database.

migrationBuilder
    .AlterDatabase()
    .Annotation("Npgsql:Enum:mood", "thrilled,sad");

migrationBuilder.CreateTable(
    name: "Blogs",
    columns: table => new
    {
        Mood = table.Column<Mood>( // references the Mood enum type
            type: "mood",
            nullable: false,
            defaultValue: Mood.Happy // at this point in time Mood.Happy is "thrilled"
        ),
// omitted for brevity

Now remove [PgName("thrilled")] from Mood. Write the next migration and apply it to the database.

migrationBuilder.Sql(@"ALTER TYPE mood RENAME VALUE 'thrilled' TO 'happy';");

Up to this point everything have worked.

Now drop the database and run the migrations.

dotnet ef database drop
dotnet ef database update

An exception will be thrown saying:

invalid input value for enum mood: "happy"

This happens because we first run the initial migration (mentioned above) which have defaultValue: Mood.Happy which used to be defaultValue: "thrilled" when it was generated and ran the first time. But now Mood.Happy is "happy" which isn't a valid value.

The SQL is probably easier to reason about

dotnet ef database drop
dotnet ef migrations script -o changes.sql

Will look like this

-- initial migration
START TRANSACTION;
CREATE TYPE mood AS ENUM ('thrilled', 'sad');
CREATE TABLE "Blogs" (
    "Id" integer GENERATED BY DEFAULT AS IDENTITY,
    "Mood" mood NOT NULL DEFAULT 'happy'::mood, -- should be thrilled
    -- omitted for brevity
 )
COMMIT;
-- next migration
START TRANSACTION;
ALTER TYPE mood RENAME VALUE 'thrilled' TO 'happy'
COMMIT;

The problem isn't just with the default value. Since the migration references the actual enum type that enum can never be deleted else it'll cause compile errors. This is a bit critical 😄

I was using:

    <PackageReference Include="Microsoft.EntityFrameworkCore" Version="8.0.6" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="8.0.6" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Relational" Version="8.0.6" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="8.0.6" />
    <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="8.0.4" />
roji commented 2 months ago

Duplicate of #2962

roji commented 2 months ago

See other issues referenced from #2962 which also discuss this.

snebjorn commented 2 months ago

That's unfortunate.

I don't know the inner workings of EF. But this comment

this is by-design. Types that the provider handles natively are considered to be stable, and we can't think of any better rule than this.

got me wondering if it is possible for the Npgsql provider to supply its own stable enum type and then use value converters - or something like that

Perhaps the migration can look like this

table.Column<NpgsqlEnumType>( // references the stable provider enum type
    type: "mood", // this is the actual enum type
    nullable: false,
    defaultValue: "thrilled" // just a string here
)
roji commented 2 months ago

The problem is that if you add a value converter, that's also going to get used e.g. when sending enum parameters to the database, in queries and updates. There's no concept of one type used only for migrations (e.g. NpgsqlEnumType), and another used for the rest.

It really is an unfortunate and tricky problem. On the other hand, people have seemed to generally be OK to keep the enum types around in the codebase, and to remove them only when squashing the migrations, removing all trace/reference of them. It's definitely not perfect, but not completely terrible either.