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.77k stars 3.19k forks source link

Build a better way to manage custom migration operations #34708

Open GabrielHSFerreira opened 1 month ago

GabrielHSFerreira commented 1 month ago

Problem: Currently, it is common to call .Sql(), or make an extension method for MigrationBuilder, and call inside some generated migration when we need to execute some custom operation using the migration feature. That can become a problem on a scenario where resetting the migration history is required, since one would have to review all migrations in search of custom calls.

Seeding of data, creation of procedures, views, full-text catalogs and any other model changes operation that is not natively supported by Entity Framework Core can be easily lost.

Proposal: What I propose is that we build a feature for managing custom commands similar to entities fluent API configuration and EF should include these custom commands in the migrations and snapshot as well update when something changes.

So, just like we can do: modelBuilder.Entity<SomeEntity>().SomeConfiguration();

Or:

public void SomeEntityConfiguration : IEntityTypeConfiguration<SomeEntity>
{
    public void Configure(EntityTypeBuilder<SomeEntity> builder)
    {
        builder.SomeConfiguration();
    }
}

We would do: modelBuilder.Custom<CustomTypeIdentifier>().RunSql("CREATE FULLTEXT CATALOG ftCatalog AS DEFAULT;");

Or:

public void SomeEntityConfiguration : ICustomTypeConfiguration<CustomTypeIdentifier>
{
    public void Configure(CustomTypeBuilder<CustomTypeIdentifier> builder)
    {
        builder.RunSql("CREATE FULLTEXT CATALOG ftCatalog AS DEFAULT;");
    }
}

Please do not get too attached to my poor example. It is just a way I found to relate to fluent entity configuration.

roji commented 1 month ago

I'm not sure what in the above proposal solves the problem of resetting the migrations... It's certainly easy to wrap custom SQL in extension methods (or some builder API, or whatever), but addressing the migration reset scenario probably requires a much more fundamental design rethink.

GabrielHSFerreira commented 1 month ago

I'm not sure what in the above proposal solves the problem of resetting the migrations... It's certainly easy to wrap custom SQL in extension methods (or some builder API, or whatever), but addressing the migration reset scenario probably requires a much more fundamental design rethink.

I completely agree that is a hard task. My proposal above is about a configuration interface for the users. Actually I do not know enough about the migration generation engine to propose the low level design, so my intent is kickoff the discussion.

GabrielHSFerreira commented 1 month ago

As a huge EF fan (both Vanilla and Core), I believe it can improve even more it's code first capabilities, making it easy to fully manage database design.

roji commented 1 month ago

I'm pretty sure we have something about this already in the backlog; at the very least it has been raised many times before, notably as part of the discussion around migrations squashing (#2174).

@maumar does this specifically ring a bell in terms of duplicates?

maumar commented 2 weeks ago

I don't know of an exact duplicate, but I do agree that the underlying problem would be solved by squashing. I'm also not a fan of embedding this information into the model. We would run into some issues with ordering of operations - we can't reason about order of a given custom SQL operation vis-a-vis regular migration operations that EF natively generates (DROP, RENAME etc). User would need to provide a hint like, "this custom operation has the precedence of, say, RENAME COLUMN" but that seems very clunky.

Also, there is a question of down migration. Currently for custom operations users can/should provide a counterpart to the custom SQL they add, but if they have no intention of ever reverting the migration, it's not an issue if the custom SQL for Down method is missing. If we decided to put this "officially" in the model, we would probably be stricter about it (and maybe require SQL for down?)

I'm thinking keeping it at the custom SQL info at the migration level could be the right way, as long as there is a reasonable automation/detection so that users don't need to manually deal with it.