Support custom migration operations

[roji]

This tracks support for provider-specific migration operations which aren't part of the basic set defined in the core. An example is migration support for ensuring PostgreSQL extensions, as implemented in Npgsql.

This includes:

• The ability to register a provider-specific MigrationsModelDiffer service, which would be able to compare two models and generate the custom migration operation. This is tracked by Make IMigrationsModelDiffer a provider service #6522.
• The ability to register a provider-specific CSharpMigrationOperationGenerator service, which would be able to render the custom migration operation to C# code.

Note that NpgsqlCSharpMigrationOperationGenerator generates migration code containing .EnsurePostgresExtension(...), which is is implemented in Npgsql's MigrationBuilderExtensions. First, this means that you have to scaffold the migration with the Npgsql provider in order to get it; it would theoretically be nice to scaffold migrations for all providers, but as @ajcvickers says here this seems a bit unrealistic.

Second, .EnsurePostgresExtension(...) only adds the custom migration operation if the current provider is Npgsql (by checking builder.ActiveProvider), otherwise you get a custom Npgsql migration in the SqlServer migrations SQL generator. I hope this technique makes sense.

[bricelam]

Howdy Shay,

We discussed this at length yesterday in the design meeting, and we came to the conclusion that we would like to pursue an approach similar to how core metadata works. In this approach, the domain model (MigrationOperation classes) is fixed but annotatable. Providers can enrich the user experience by providing extension methods over the annotations. Allowing the provider to scaffold these extension methods it tracked as #6546.

Not allowing custom operations at this point will keep the migrations pipeline simple allowing it to be more easily extended. For example, if someone writes an F# scaffolder and we allowed custom operations in providers, these operations would fail to scaffold. Whereas with annotations, the F# scaffolder can have a general way of rendering these that will always work, but the provider would also be given a chance to scaffold these using provider-specific extension methods for the current language if possible.

We also decided not to allow providers to override the model differ at this point. Instead of having providers replace it's functionality, we would like to work with them to enrich it's functionality and extensibility to satisfy their requirements. This keeps the interaction between the provider and the model differ simpler making it easier to implement new providers.

We would love to work with you on converting your custom operations to use annotations instead. Hopefully this will help us find more feature gap before the 1.1.0 release.

[roji]

Hi @bricelam, thanks for giving this your attention and discussing it internally.

I understand the concern with regards to supporting additional languages for generated migration code (e.g. F#). I'll try to work on migrating from custom migrations to annotations instead and will report any issues (have opened npgsql/efcore.pg#101 to track this). I'll also be glad to try out provider-rendered annotations (#6546) when you have something ready.

PPS In the design meeting notes you have the example code migrationsBuilder.AlterDatabase().Annotation("Npgsql:.PostGIS", "'PostGIS', '', ''"), where the annotation name should be something like Npgsql:PostgresExtension rather than Npgsql:.PostGIS.

[bricelam]

lol, I tried reading the code to see what the model annotation looked like. I guess my brain makes a bad compiler/debugger.

[roji]

FYI, finished reimplementing the extension migrations as annotations on AlterDatabaseOperation. Adding HasPostgresExtension() calls in OnModelCreating now generates migrations which look like this:

protected override void Up(MigrationBuilder migrationBuilder)
{
    migrationBuilder.AlterDatabase()
        .Annotation("Npgsql:PostgresExtension:.postgis", "'postgis', '', ''")
        .Annotation("Npgsql:PostgresExtension:.uuid-ossp", "'uuid-ossp', '', ''")
        .OldAnnotation("Npgsql:PostgresExtension:.uuid-ossp", "'uuid-ossp', '', ''");
}

protected override void Down(MigrationBuilder migrationBuilder)
{
    migrationBuilder.AlterDatabase()
        .Annotation("Npgsql:PostgresExtension:.uuid-ossp", "'uuid-ossp', '', ''")
        .OldAnnotation("Npgsql:PostgresExtension:.postgis", "'postgis', '', ''")
        .OldAnnotation("Npgsql:PostgresExtension:.uuid-ossp", "'uuid-ossp', '', ''");
}

NpgsqlMigrationsSqlGenerator translate these to PostgreSQL's CREATE EXTENSION statements.

This migration code ain't exactly pretty, but hopefully #6546 will help out with that.

[bricelam]

Awesome! I'm glad you were able to make it functional. lol, I agree it ain't pretty.

[roji]

OK, I admit I'm no longer sure whether modeling something like PostgreSQL extensions is such a good idea - whether via custom migration classes or AlterDatabase migrations. I'm considering dropping any specific support for extensions in the EFCore provider and simply telling users to drop to raw SQL in their migrations and execute CREATE EXTENSION IF NOT EXISTS postgis. The added value of this modeling is no longer clear - it's uglier than the raw SQL approach (even if #6546 will help, but that seems like a pretty complex solution for a problem that doesn't necessarily exist) and doesn't provide much value.

I originally thought there would be some value in being able to check what extensions are used in a given model - a validator could make sure that the PostGIS extension is there if PostGIS types are used, but that amounts to throwing an exception from a EFCore modeler rather than having PostgreSQL bomb. I think I was also simply overexcited about the possibility of modeling arbitrary things in EFCore migrations etc.

What are your thoughts on this? Do you see any value in pursuing this?

[bricelam]

cc @rowanmiller

I could go either way. It is nice if users can add GIS properties to their types, call EnsureCreated, and everything "just works". But having to issue the CREATE EXTENSION statement during app startup isn't horrible either.

[roji]

@rowanmiller any comment on #6524 (comment)?

[rowanmiller]

@roji yeah I tend to agree that there may be limited value. If anything, maybe just have an extension method on migration builder to create the extension (and it can just be a very thin wrapper over the Sql() API).

[roji]

@rowanmiller sorry for bothering your guys with this for so long. I started working on removing the special support for PostgreSQL extensions, but realized there's an issue.

When users are using migrations, it's OK to ask them to add the extension SQL themselves. However, if migrations aren't being used there's nowhere to set this up. Therefore, without some special support on the model (e.g. the somewhat ugly AlterDatabase solution discussed above) there's no way for a user to create a database from a model that relies on an extension...

Unless I'm missing something or you have a better idea, it seems I have no choice but to retain the AlterDatabase solution.

[rowanmiller]

Reopening to discuss in triage

[roji]

Just to give more scope to this...

At some point I'd like to be able to add support for PostgreSQL composite types - the ability to created a complex, user-defined type in the database and then define tables with columns of that type. Npgsql already supports this at the ADO.NET level.

The question here would be how/where to specify the creation of the data type - this is done with a CREATE TYPE statement (here are the docs). Much like extensions, types are database-scoped objects which aren't tables, indexes, or sequences. Unlike extensions, they are relatively complex (list of fields and their types), and so representing them as an annotation on AlterDatabase may, well, not be ideal.

At the end of the day, all this complexity could be resolved by simply providing some place on the model where the user could specify arbitrary SQL to be executed after database creation (it would have to be provider-specific though). They could create extensions, composite types, and tweak the database in ways not yet modeled by the provider. This would work whether migrations are in use or not, and although it would be a bit hacky it would provide a universal answer to all this.

Composite type support isn't urgent in any way, unlike extensions, which are more important.

[rowanmiller]

Ok, we generalized #5390 to explore this whole area in the 1.2 timeframe. We'll look at how we want folks to extend migrations based on provider specific elements in the model.

In the meantime, the approach that we have taken with memory-optimized tables for SQL Server is the recommended approach.