Unable to create migration using Pomelo's connector when Option types are already present in model

[GunnerGuyven]

Describe the bug
A migration cannot be created when using Pomelo.EntityFrameworkCore.MySql as the connector when an option type is present on the existing (pre-migration) model.

To Reproduce
Starting with the following model:

module DBModel

open Microsoft.EntityFrameworkCore
open EntityFrameworkCore.FSharp.Extensions

[<CLIMutable>]
type Blog =
    { Id: int
      Title: string
      Content: string }

type DBModelContext() =
    inherit DbContext()

    [<DefaultValue>]
    val mutable blogs: DbSet<Blog>

    member this.Blogs
        with get () = this.blogs
        and set v = this.blogs <- v

    override _.OnModelCreating builder = builder.RegisterOptionTypes()

    override _.OnConfiguring options =
        options.UseMySql(
            "server=localhost;user=testdb;password=abc123;database=testdb",
            MariaDbServerVersion(System.Version(10, 8, 3))
        )
        |> ignore

No Database is necessary, this issue is only concerned with creating migrations.
Steps to reproduce the behavior:

1. dotnet ef migrations add test1 (no issue)
2. dotnet ef migrations add test2 (no issue)
3. alter model to

[<CLIMutable>]
type Blog =
    { Id: int
      Title: string
      Content: string option }

4. dotnet ef migrations add test3 (no issue)
5. dotnet ef migrations add test4 (FAIL)

# dotnet ef migrations add test4 
Build started...
Build succeeded.
System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsModelDiffer.Initialize(ColumnOperation columnOperation, IColumn column, RelationalTypeMapping typeMapping, Boolean isNullable, IEnumerable`1 migrationsAnnotations, Boolean inline)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsModelDiffer.Diff(IColumn 
source, IColumn target, DiffContext diffContext)+MoveNext()
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsModelDiffer.DiffCollection[T](IEnumerable`1 sources, IEnumerable`1 targets, DiffContext diffContext, Func`4 diff, Func`3 add, Func`3 remove, Func`4[] predicates)+MoveNext()
   at System.Linq.Enumerable.ConcatIterator`1.MoveNext()
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsModelDiffer.Diff(ITable source, ITable target, DiffContext diffContext)+MoveNext()
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsModelDiffer.DiffCollection[T](IEnumerable`1 sources, IEnumerable`1 targets, DiffContext diffContext, Func`4 diff, Func`3 add, Func`3 remove, Func`4[] predicates)+MoveNext()
   at System.Linq.Enumerable.ConcatIterator`1.MoveNext()
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsModelDiffer.Sort(IEnumerable`1 operations, DiffContext diffContext)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationsModelDiffer.GetDifferences(IRelationalModel source, IRelationalModel target)
   at Microsoft.EntityFrameworkCore.Migrations.Design.MigrationsScaffolder.ScaffoldMigration(String migrationName, String rootNamespace, String subNamespace, String language)        
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.AddMigration(String name, String outputDir, String contextType, String namespace)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigrationImpl(String name, 
String outputDir, String contextType, String namespace)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigration.<>c__DisplayClass0_0.<.ctor>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.<>c__DisplayClass3_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
Object reference not set to an instance of an object.

Expected behavior
I would expect migration 4 to succeed.

Additional context
This works with Microsoft.EntityFrameworkCore.Sqlite so it may be a failure of the Pomelo connector specifically, but I am unsure. I've tested all of the 6.0.x releases of Pomelo's connector with the same result.

[simon-reynolds]

Hi @GunnerGuyven
So Migration 4 is failing even though it has no changes compared to 3? So 4 should just be an empty migration?

Sounds like it could be related to Pomelo if it works with Microsoft.EntityFrameworkCore.Sqlite, I notice the stacktrace doesn't mention this projects override of the ModelDiffer, EntityFrameworkCore.FSharp.Migrations.Internal. FSharpMigrationsModelDiffer

@bricelam, quick question, in IDesignTimeServices.ConfigureDesignTimeServices we call services.AddSingleton<IMigrationsModelDiffer, FSharpMigrationsModelDiffer>()
If the Pomelo Sqlite connector also overrides IMigrationsModelDiffer would EF just pick up whichever one was registered last?

[GunnerGuyven]

That's right, there is no model change. I have narrowed this issue to the presence of the option type itself in the previous migration. You can construct an elaborate model involving option types and so long as there are no previous migrations (involving option types), the migration will succeed.

[GunnerGuyven]

I have created a project demonstrating this issue

git clone https://github.com/GunnerGuyven/ef-pomelo-fsharp-migrate-failure
cd .\ef-pomelo-fsharp-migrate-failure\
dotnet tool restore
dotnet restore
dotnet ef migrations add test4

And opened a ticket with Pomelo to match this one (PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#1700)

[GunnerGuyven]

@simon-reynolds Researching the codebase of Pomelo, I see that they do indeed also add an IMigrationsModelDiffer to the EF builder. This seems done to handle some MySql specific peculiarities with datetime and timestamp fields in certain situations. Surely there is a way of navigating when two handlers have been registered in this manner by different stakeholders?

If there is a 'good citizen' approach to this problem, what would it be and which library should hold that responsibility?

[bricelam]

bricelam, quick question, in IDesignTimeServices.ConfigureDesignTimeServices we call services.AddSingleton<IMigrationsModelDiffer, FSharpMigrationsModelDiffer>()
If the Pomelo Sqlite connector also overrides IMigrationsModelDiffer would EF just pick up whichever one was registered last?

The model differ is not designed to be overridden by providers or extensions. Additional functionality should instead be enabled using model annotations.

This seems done to handle some MySql specific peculiarities with datetime and timestamp fields in certain situations.

I vaguely remember this. I think there's an EF issue to review this code and see if there's anything we can add to make this better.

What functionality is currently added by the F# override? Could it be part of the core EF implementation?

[simon-reynolds]

The F# override is to add support for the Option<_> type, it's a wrapper similar to Nullable<_>
F# tries to avoid use of null as much as possible, so it uses the option type instead that wraps an item that may or may not be capable of having a null value

The F# model differ matches conventions by marking every column that isn't a PrimaryKey as non-nullable unless the CLR type is either Nullable<_> or Option<_>
Since Option<_> is F# specific, I don't know if there's appetite in EF Core for including it?

[bricelam]

Hmm, it feels like this shouldn't be needed in the differ. An F#-specific model building convention should mark the IColumn as IsNullable when the CLR type uses option.

/cc @AndriySvyryd