Consider redoing extensions, enums, ranges via ModelDiffer

[roji]

Our current support of PostgreSQL-specific database objects (extensions, enums, ranges, soon hopefully composites) is currently done purely via annotations - objects are represented as metadata on the model, which are then flowed into migrations via annotations on AlterDatabaseOperation. This is unlike EF Core's support for sequences, for example, which has metadata annotations (like us) but then picks up the sequences in the ModelDiffer, which then generates special {Create,Drop,Rename}SequenceOperation.

Our approach is problematic for the following reasons:

• Addition/removal of objects needs to be done in our MigrationsSqlGenerator - compare old and new annotations on an AlterDatabaseOperation, calculate what was added, what was removed, etc. This work conceptually belongs in ModelDiffer rather than in MigrationsSqlGenerator.
• It's impossible to implement correct DefaultSchema support, because we have no way of knowing that the DefaultSchema has changed, and that our database objects need to be moved accordingly (see Schema enums follow up (part 2) #685 (comment)).

In the past, we already have a discussion about providing our own ModelDiffer, which would generate custom migration operations (e.g. CreateEnumOperation). This would be a better design and align us with the way EF Core does things. It may be worth revisiting for 3.0 in case we change our minds.

/cc @austindrenski @ajcvickers @divega @bricelam

[roji]

Just a thought I had about this... Detecting changes in the ModelDiffer does not necessarily mean that we have to custom migrations for them. One of the previous main objections in dotnet/efcore#6524 was that custom migration operations would require generating code for them, in C# and potentially other languages, and that would become complicated.

However, at least in theory we can have our own ModelDiffer which finds the appropriate differences between enums, ranges and extensions, but then represents the appropriate operations to be done again, as annotations on an AlterDatabase migration. In other words, instead of flowing model metadata annotations directly to the migrations, they'd be picked up in a special way in the ModelDiffer, which would translate them to other annotations that represent actual operations that need to take place. This would imply some sort of scheme for representing "change this enum in this way" via an annotation on AlterDatabase, which isn't trivial but probably doable.

Just recording some thoughts here for when we get around to revisiting this...

[austindrenski]

I'm starting to look into this today. It has some potential overlap with #736, but I think it makes sense to start here since may require upstream refactoring (e.g. changing the namespace on
MigrationsModelDiffer), so probably best to identify those changes sooner rather than later.

[roji]

OK. I'd recommend not going too far into this without showing an approach we can discuss, to make sure we all agree on where we're going etc.

[austindrenski]

Sounds good, I'll open a PR once I have something basic put together.

[bricelam]

I'm sad you have to resort to replacing the model differ. I feel like making the core/relational model more extensible (beyond our current annotations mechanism) would be a better solution to the problem. Making schemas more first-class might help too.

In other words, I don't think providers should have to change the model differ, but if they add new concepts to the model, they should be able to define how they get diffed. This is the current approach with model annotations, but it's limited by the fact that annotations are simple key-value pairs and they force you have to perform structural diffing in during SQL generation.

[austindrenski]

@bricelam Having better schema support throughout the model would be great, and defining configurations without wading into the model differ itself would be nice too.

All things considered, the changes in #744 seem pretty narrow (to me, that is), but I can understand the hesitation to have providers wading into the model differ for every change.

I could imagine implementing the changes in #744 following the pattern that we currently use for CompositeMethodCallTranslator, but I'm not sure what type of changes that would require from the core project.

[bricelam]

Oh, and I'm certainly not pushing back on having Npgsql override it (the changes we made in EF Core 2.0 make it safe to do). It's just something that providers shouldn't ever have to override so when they do, it's a good sign we're missing extensiblity points elsewhere.