PomeloFoundation / Pomelo.EntityFrameworkCore.MySql

Entity Framework Core provider for MySQL and MariaDB built on top of MySqlConnector
MIT License
2.69k stars 381 forks source link

Existing EF migrations broke when I migrated from .NET 6 to .NET 8 (byte[] and Guid) #1823

Closed YumeiTenerife closed 7 months ago

YumeiTenerife commented 10 months ago

Steps to reproduce

I had some existing EF migrations (code first) generated using .NET 6 (and Pomelo 6.0.2). These migrations had some Guid properties that were mapped with the following code:

  migrationBuilder.CreateTable(
      name: "MyTable",
      columns: table => new
      {
          Id = table.Column<int>(type: "int", nullable: false)
              .Annotation("MySql:ValueGenerationStrategy", MySqlValueGenerationStrategy.IdentityColumn),
          SomeGuid = table.Column<byte[]>(type: "binary(16)", nullable: false),
      }

After upgrading to .NET 8 and started using Pomelo 8.0.0-beta.2, my existing migrations stopped working. When running the migrations to create a new database using the following command:

dotnet ef database update

I get an exception. Look at "The issue" for more details.

NOTE: I was able to fix it replacing in all the existing migrations < byte[] > for < Guid >. But ideally it should be backward compatible.

The issue

Exception issue:

Build started...
Build succeeded.
System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindCollectionMapping(RelationalTypeMappingInfo info, Type modelType, Type providerType, CoreTypeMapping elementMapping)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.<>c.<FindMappingWithConversion>b__8_0(ValueTuple`4 k, RelationalTypeMappingSource self)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindMappingWithConversion(RelationalTypeMappingInfo mappingInfo, Type providerClrType, ValueConverter customConverter)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindMappingWithConversion(RelationalTypeMappingInfo mappingInfo, IReadOnlyList`1 principals)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindMapping(IProperty property)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Property.<>c.<get_TypeMapping>b__91_0(IProperty property)
   at Microsoft.EntityFrameworkCore.Internal.NonCapturingLazyInitializer.EnsureInitialized[TParam,TValue](TValue& target, TParam param, Func`2 valueFactory)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Property.get_TypeMapping()
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Property.Microsoft.EntityFrameworkCore.Metadata.IReadOnlyProperty.FindTypeMapping()
   at Microsoft.EntityFrameworkCore.RelationalPropertyExtensions.FindRelationalTypeMapping(IReadOnlyProperty property)
   at Microsoft.EntityFrameworkCore.RelationalPropertyExtensions.GetColumnType(IReadOnlyProperty property)
   at Microsoft.EntityFrameworkCore.RelationalPropertyExtensions.GetColumnType(IProperty property)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.RelationalModel.AddDefaultMappings(RelationalModel databaseModel, IEntityType entityType, IRelationalTypeMappingSource relationalTypeMappingSource)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.RelationalModel.Create(IModel model, IRelationalAnnotationProvider relationalAnnotationProvider, IRelationalTypeMappingSource relationalTypeMappingSource, Boolean designTime)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.RelationalModel.Add(IModel model, IRelationalAnnotationProvider relationalAnnotationProvider, IRelationalTypeMappingSource relationalTypeMappingSource, Boolean designTime)
   at Microsoft.EntityFrameworkCore.Infrastructure.RelationalModelRuntimeInitializer.InitializeModel(IModel model, Boolean designTime, Boolean prevalidation)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelRuntimeInitializer.Initialize(IModel model, Boolean designTime, IDiagnosticsLogger`1 validationLogger)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.FinalizeModel(IModel model)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.GenerateUpSql(Migration migration, MigrationsSqlGenerationOptions options)
   at Pomelo.EntityFrameworkCore.MySql.Migrations.Internal.MySqlMigrator.GenerateUpSql(Migration migration, MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.GenerateScript(String fromMigration, String toMigration, MigrationsSqlGenerationOptions options)
   at Pomelo.EntityFrameworkCore.MySql.Migrations.Internal.MySqlMigrator.GenerateScript(String fromMigration, String toMigration, MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.ScriptMigration(String fromMigration, String toMigration, MigrationsSqlGenerationOptions options, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.ScriptMigrationImpl(String fromMigration, String toMigration, Boolean idempotent, Boolean noTransactions, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.ScriptMigration.<>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.

Further technical details

MySQL version: 8.0 Operating system: Windows Pomelo.EntityFrameworkCore.MySql version: 8.0.0-beta.2 Microsoft.AspNetCore.App version: .NET 8

Other details about my project setup:

sushilmate commented 10 months ago

facing the same issue. not sure when this will be released. we cant used in beta version in prod. few users suggesting to downgrade to version 6.

YumeiTenerife commented 10 months ago

facing the same issue. not sure when this will be released. we cant used in beta version in prod. few users suggesting to downgrade to version 6.

Yeah definitely not for production code, i would downgrade until then. I am hoping that there is a fix by the time we go to production.

lauxjpn commented 8 months ago

@YumeiTenerife @sushilmate I am unable to reproduce this issue in Pomelo 8.0.x. Has this been fixed between 8.0.0-beta.2 and 8.0.0?

If not, please post a sample model class with this issue, its configuration/Fluent API and your .UseMySql() call (including the connection string, without sensitive information), so we can reproduce the issue. Thanks!

sushilmate commented 8 months ago

lauxjpn

it was in the older version. beta & 8.0 look better. tested on both. works fine. thank you

YumeiTenerife commented 7 months ago

Hello @lauxjpn , I still have the same issue. We use code first approach.

1) Using .NET 6 (and Pomelo 6.0.2), I created a database model:

public class MyTable {
    [Key]
    public int Id { get; set; }

    [Required]
    public Guid SomeGuid { get; set; }
}

2) Created the migration using: dotnet ef migrations add NewMigration

3) This created the following migration file:

    protected override void Up(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.AlterDatabase()
            .Annotation("MySql:CharSet", "utf8mb4");

        migrationBuilder.CreateTable(
              name: "MyTable",
              columns: table => new
              {
                  Id = table.Column<int>(type: "int", nullable: false)
                      .Annotation("MySql:ValueGenerationStrategy", MySqlValueGenerationStrategy.IdentityColumn),
                  SomeGuid = table.Column<**byte[]**>(type: "binary(16)", nullable: false),
              },
              constraints: table =>
              {
                  table.PrimaryKey("PK_MyTable", x => x.Id);
              })
              .Annotation("MySql:CharSet", "utf8mb4");
    }

4) I upgraded to .NET 8, using Pomelo 8.0.0. Then I tried to run my existing migration to create a new DB: From the console: dotnet ef database update OR, to generate the script instead: dotnet ef migrations script

In both cases, we get the following error:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindCollectionMapping(RelationalTypeMappingInfo info, Type modelType, Type providerType, CoreTypeMapping elementMapping)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.<>c.<FindMappingWithConversion>b__8_0(ValueTuple`4 k, RelationalTypeMappingSource self)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindMappingWithConversion(RelationalTypeMappingInfo mappingInfo, Type providerClrType, ValueConverter customConverter)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindMappingWithConversion(RelationalTypeMappingInfo mappingInfo, IReadOnlyList`1 principals)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindMapping(IProperty property)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Property.<>c.<get_TypeMapping>b__91_0(IProperty property)
   at Microsoft.EntityFrameworkCore.Internal.NonCapturingLazyInitializer.EnsureInitialized[TParam,TValue](TValue& target, TParam param, Func`2 valueFactory)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Property.get_TypeMapping()
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Property.Microsoft.EntityFrameworkCore.Metadata.IReadOnlyProperty.FindTypeMapping()
   at Microsoft.EntityFrameworkCore.RelationalPropertyExtensions.FindRelationalTypeMapping(IReadOnlyProperty property)
   at Microsoft.EntityFrameworkCore.RelationalPropertyExtensions.GetColumnType(IReadOnlyProperty property)
   at Microsoft.EntityFrameworkCore.RelationalPropertyExtensions.GetColumnType(IProperty property)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.RelationalModel.AddDefaultMappings(RelationalModel databaseModel, IEntityType entityType, IRelationalTypeMappingSource relationalTypeMappingSource)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.RelationalModel.Create(IModel model, IRelationalAnnotationProvider relationalAnnotationProvider, IRelationalTypeMappingSource relationalTypeMappingSource, Boolean designTime)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.RelationalModel.Add(IModel model, IRelationalAnnotationProvider relationalAnnotationProvider, IRelationalTypeMappingSource relationalTypeMappingSource, Boolean designTime)
   at Microsoft.EntityFrameworkCore.Infrastructure.RelationalModelRuntimeInitializer.InitializeModel(IModel model, Boolean designTime, Boolean prevalidation)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelRuntimeInitializer.Initialize(IModel model, Boolean designTime, IDiagnosticsLogger`1 validationLogger)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.FinalizeModel(IModel model)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.GenerateUpSql(Migration migration, MigrationsSqlGenerationOptions options)
   at Pomelo.EntityFrameworkCore.MySql.Migrations.Internal.MySqlMigrator.GenerateUpSql(Migration migration, MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.GenerateScript(String fromMigration, String toMigration, MigrationsSqlGenerationOptions options)
   at Pomelo.EntityFrameworkCore.MySql.Migrations.Internal.MySqlMigrator.GenerateScript(String fromMigration, String toMigration, MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.ScriptMigration(String fromMigration, String toMigration, MigrationsSqlGenerationOptions options, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.ScriptMigrationImpl(String fromMigration, String toMigration, Boolean idempotent, Boolean noTransactions, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.ScriptMigration.<>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)

I also tried to create a NEW migration using the new pomelo version, and the migration generated was similar, except that now the Guid was no longer a byte[], but now we get: SomeGuid = table.Column<Guid>(type: "binary(16)", nullable: false),

This script works fine, but the old one generated with the previous version no longer works.

YumeiTenerife commented 7 months ago

Also some notes: This error does not happen while doing any query. Only when running the migration to create a new database. I will try to setup an example and send that to you. Thank you!

lauxjpn commented 7 months ago

I will try to setup an example and send that to you. Thank you!

Might not be necessary. We will try to reproduce the issue with the information you provided.

lauxjpn commented 7 months ago

By default, Pomelo 6.0.2 should have generated the following line in the migration Up() method:

SomeGuid = table.Column<Guid>(type: "char(36)", nullable: false, collation: "ascii_general_ci")

Your migration's Up() method contains the following line instead:

SomeGuid = table.Column<byte[]>(type: "binary(16)", nullable: false),

@YumeiTenerife Please provide us with the following information marked in bold:

If not, please post a sample model class with this issue, its configuration/Fluent API and your .UseMySql() call (including the connection string, without sensitive information), so we can reproduce the issue.

YumeiTenerife commented 7 months ago

Yes, sorry, I missed that. The connection string looks like this: var connectionString = $"server={host};port={port};database={database};uid={username};pwd={password};GuidFormat=Binary16";

UseMySql(connectionString, new MySqlServerVersion(new Version(8, 0, 31));)

lauxjpn commented 7 months ago

I am still unable to reproduce this issue.

I created the following console project using Pomelo 6.0.2:

Program.cs ```c# using System; using System.Diagnostics; using System.Linq; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; namespace IssueConsoleTemplate; public class MyTable { public int Id { get; set; } public Guid SomeGuid { get; set; } } public class Context : DbContext { public DbSet MyTables { get; set; } protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) { if (!optionsBuilder.IsConfigured) { var connectionString = "server=127.0.0.1;port=3306;user=root;password=;database=Issue1823_01;GuidFormat=Binary16"; var serverVersion = ServerVersion.AutoDetect(connectionString); optionsBuilder .UseMySql(connectionString, serverVersion) .LogTo(Console.WriteLine, LogLevel.Information) .EnableSensitiveDataLogging() .EnableDetailedErrors(); } } protected override void OnModelCreating(ModelBuilder modelBuilder) { modelBuilder.Entity( entity => { entity.Property(e => e.SomeGuid) .IsRequired(); entity.HasData( new MyTable { Id = 1, SomeGuid = Guid.Parse("5aa12cf0-bae3-406e-9ab6-92b1cf5fa9e1"), }); }); } } internal static class Program { private static void Main() { /* using var context = new Context(); context.Database.EnsureDeleted(); context.Database.EnsureCreated(); var vanillaIceCream = context.MyTables .Single(i => i.SomeGuid == Guid.Parse("5aa12cf0-bae3-406e-9ab6-92b1cf5fa9e1")); Trace.Assert(vanillaIceCream.Id == 1); */ } } ```

I then ran the following CLI command (using dotnet-ef version 6.0.7):

dotnet ef migrations add 'Pomelo6'

It generated the following file (similar to yours):

20240305151045_Pomelo6.cs ```c# using System; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Migrations; #nullable disable namespace IssueConsoleTemplate.Migrations { public partial class Pomelo6 : Migration { protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.AlterDatabase() .Annotation("MySql:CharSet", "utf8mb4"); migrationBuilder.CreateTable( name: "MyTables", columns: table => new { Id = table.Column(type: "int", nullable: false) .Annotation("MySql:ValueGenerationStrategy", MySqlValueGenerationStrategy.IdentityColumn), SomeGuid = table.Column(type: "binary(16)", nullable: false) }, constraints: table => { table.PrimaryKey("PK_MyTables", x => x.Id); }) .Annotation("MySql:CharSet", "utf8mb4"); migrationBuilder.InsertData( table: "MyTables", columns: new[] { "Id", "SomeGuid" }, values: new object[] { 1, new Guid("5aa12cf0-bae3-406e-9ab6-92b1cf5fa9e1") }); } protected override void Down(MigrationBuilder migrationBuilder) { migrationBuilder.DropTable( name: "MyTables"); } } } ```

I then switched to Pomelo 8.0.0.

Finally, I ran the following CLI command (using dotnet-ef version 8.0.2):

dotnet ef migrations script

It generated the following output without any exception:

Output (SQL) ```sql Build started... Build succeeded. warn: 05.03.2024 16:22:56.607 CoreEventId.SensitiveDataLoggingEnabledWarning[10400] (Microsoft.EntityFrameworkCore.Infrastructure) Sensitive data logging is enabled. Log entries and exception messages may include sensitive application data; this mode should only be enabled during development. CREATE TABLE IF NOT EXISTS `__EFMigrationsHistory` ( `MigrationId` varchar(150) CHARACTER SET utf8mb4 NOT NULL, `ProductVersion` varchar(32) CHARACTER SET utf8mb4 NOT NULL, CONSTRAINT `PK___EFMigrationsHistory` PRIMARY KEY (`MigrationId`) ) CHARACTER SET=utf8mb4; START TRANSACTION; ALTER DATABASE CHARACTER SET utf8mb4; CREATE TABLE `MyTables` ( `Id` int NOT NULL AUTO_INCREMENT, `SomeGuid` binary(16) NOT NULL, CONSTRAINT `PK_MyTables` PRIMARY KEY (`Id`) ) CHARACTER SET=utf8mb4; INSERT INTO `MyTables` (`Id`, `SomeGuid`) VALUES (1, 0x5AA12CF0BAE3406E9AB692B1CF5FA9E1); INSERT INTO `__EFMigrationsHistory` (`MigrationId`, `ProductVersion`) VALUES ('20240305151045_Pomelo6', '8.0.0'); COMMIT; ```

I did not change the connection string in any way between upgrading from Pomelo 6.0.2 to 8.0.0.

@YumeiTenerife Please verify my code and steps on your end, to ensure that you get the same result as I do.

YumeiTenerife commented 7 months ago

That's so strange! I am looking again at the differences and I can see that I also have in the context the following:

     modelBuilder.Entity<MyTable>()
            .Property(a => a.SomeGuid)
            .HasColumnType("binary(16)");

would you mind trying that, in the model creating, when using .NET 6? entity.Property(e => e.SomeGuid) .HasColumnType("binary(16)"); .IsRequired();

Thank you so much for trying to reproduce this!

lauxjpn commented 7 months ago

Using .HasColumnType("binary(16)") doesn't make a difference, which makes sense, because as you can see in the 20240305151045_Pomelo6.cs file, binary(16) was already used without specifying it explicitly (because of your GuidFormat=Binary16 connection string option):

SomeGuid = table.Column<Guid>(type: "binary(16)", nullable: false)

Independent of that, your Up() method shows table.Column<byte[]>, while the one that was generated for me uses table.Column<Guid>:

Your line:

SomeGuid = table.Column<byte[]>(type: "binary(16)", nullable: false)

My line:

SomeGuid = table.Column<Guid>(type: "binary(16)", nullable: false)

So in your 6.0 model, SomeGuid seems to have a CLR type of byte[] for some reason.

@YumeiTenerife Are you using a value converter somewhere in your 6.0 code to convert your Guid to byte[] (e.g. by using .HasConversion<byte[]>())?

YumeiTenerife commented 7 months ago

Thanks for replying! No converters are used... In fact, that script works fine in .NET 6 (with byte[]) but fails with .NET 8. That's so weird, I have no idea how it got that byte[] there. In fact, all the migration scripts generated while using .NET 6 had that same issue. I'll try to reproduce it with your code and see if I can figure out why it generated the byte[].

Thanks and sorry for the trouble!

lauxjpn commented 7 months ago

@YumeiTenerife Any update on this issue or should we close this for now?

YumeiTenerife commented 7 months ago

Sorry! I went away on vacation and I just returned. I will try to start over and see if I am able to reproduce it. It seems like all projects had the same issue but maybe that was caused by a different setting that it was set at that time? If you did all those steps and could not reproduce it, no worries, please feel free to close this ticket. I'll create a new one if I can reproduce it with all the steps.

Thank you so much!