Pomelo.entityframeworkcore.mysql: Migrations that used to work, now fail with 2.1.0-rc1-final

[WillooWisp]

Steps to reproduce

Ideally include a complete code listing that we can run to reproduce the issue.
Alternatively, you can provide a project/solution that we can run.

The issue

Describe what is not working as expected.
Migrations that used to work in 2.0.1 now fails.
Exceptions are thrown for dropping indexes, even though now explicit index drops are performed. Complete tables with explicit indexes are being dropped though.
Exceptions are being thrown due to long identifier names, FK_ names.

If you are seeing an exception, include the full exceptions details (message and stack trace).

Exception message:
Cannot drop index 'IX_Attachments_FileId': needed in a foreign key constraint

Stack trace:
-       $exception  {MySql.Data.MySqlClient.MySqlException (0x80004005): Cannot drop index 'IX_Attachments_FileId': needed in a foreign key constraint ---> MySql.Data.MySqlClient.MySqlException (0x80004005): Cannot drop index 'IX_Attachments_FileId': needed in a foreign key constraint
   at MySqlConnector.Core.ResultSet.ReadResultSetHeaderAsync(IOBehavior ioBehavior) in C:\projects\mysqlconnector\src\MySqlConnector\Core\ResultSet.cs:line 43
   at MySql.Data.MySqlClient.MySqlDataReader.ActivateResultSet(ResultSet resultSet) in C:\projects\mysqlconnector\src\MySqlConnector\MySql.Data.MySqlClient\MySqlDataReader.cs:line 92
   at MySql.Data.MySqlClient.MySqlDataReader.ReadFirstResultSetAsync(IOBehavior ioBehavior) in C:\projects\mysqlconnector\src\MySqlConnector\MySql.Data.MySqlClient\MySqlDataReader.cs:line 324
   at MySql.Data.MySqlClient.MySqlDataReader.CreateAsync(MySqlCommand command, CommandBehavior behavior, IOBehavior ioBehavior) in C:\projects\mysqlconnector\src\MySqlConnector\MySql.Data.MySqlClient\MySqlDataReader.cs:line 309
   at MySqlConnector.Core.TextCommandExecutor.ExecuteReaderAsync(String commandText, MySqlParameterCollection parameterCollection, CommandBehavior behavior, IOBehavior ioBehavior, CancellationToken cancellationToken) in C:\projects\mysqlconnector\src\MySqlConnector\Core\TextCommandExecutor.cs:line 73
   at MySqlConnector.Core.TextCommandExecutor.ExecuteNonQueryAsync(String commandText, MySqlParameterCollection parameterCollection, IOBehavior ioBehavior, CancellationToken cancellationToken) in C:\projects\mysqlconnector\src\MySqlConnector\Core\TextCommandExecutor.cs:line 26
   at MySql.Data.MySqlClient.MySqlCommand.ExecuteNonQuery() in C:\projects\mysqlconnector\src\MySqlConnector\MySql.Data.MySqlClient\MySqlCommand.cs:line 60
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteNonQuery(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteNonQuery(IEnumerable`1 migrationCommands, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration)
   at Backend.Startup.Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory, IServiceScopeFactory scopeFactory, IHubContext`1 hubContext, IApplicationLifetime applicationLifetime, IHttpContextAccessor httpContextAccessor, IOptions`1 applicationOptionsStore) in D:\Projects\Git\Angular\web-iot-starter\net\Backend\Backend\Startup.cs:line 384}    MySql.Data.MySqlClient.MySqlException

Further technical details

MySQL version: '5.7.17'
Operating system: Windows 10 Spring Creators Update
Pomelo.EntityFrameworkCore.MySql version: 2.1.0-rc1-final

Other details about my project setup:

[caleblloyd]

Do you have the steps to recreate? I.e.

1. Generate a table with a foreign key X
2. Rename column Y to Z

Would like to investigate, need to be able to recreate though.

[WillooWisp]

I will try to reproduce a small sample asap.

[WillooWisp]

I've now shrunken our project to a bare minimum in order for the error to still show up. I still hope this is representative enough for this problem. I hope this helps!

MigrationsTestApp.zip

[WillooWisp]

Any luck pinpointing the issue?

[caleblloyd]

I likely won't be able to dig in until the weekend

[TriJetScud]

A possible hint on fixing this would be at commit https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/commit/1191bc26b740f2514ffced8f3eabb46088b68574

[WillooWisp]

@caleblloyd Any luck finding time for investigating this issue any further?

[caleblloyd]

I have not yet; it's a must-fix for 2.1.0, adding help-wanted tag to see if some can contribute to get it done faster

[WillooWisp]

Who wants to be the hero of the week and help out with this blocking issue?

[caleblloyd]

@WillooWisp can you open a PR for it?

[WillooWisp]

What I meant was that it would be great if someone with enough knowledge about this could dig into it and open a PR.

[WillooWisp]

Is there any other recommended MySQL provider for EF Core out there, since this no longer seems that supported I mean. Does anyone know?

[caleblloyd]

this no longer seems that supported I mean

This provider is community supported. It gets better when more people contribute and collaborate. See #522

---

The process for working on an issue like this would be to clone the repository, make the cloned repository a dependency of your project, and step-through debug the issue.

To work around a migration issue, you can easily add SQL to a migration by hand to fix columns. This issue should not be blocking.

[mguinness]

Is there any other recommended MySQL provider for EF Core out there

The only one I know of (but haven't used) that supports EF Core 2.1 is Devart, but that is a paid product. So if 2.1 is essential for you then that would be your best option until work is completed on this RC. If 2.1 isn't essential for you then stick with 2.0 as that is the current stable release.

[TriJetScud]

.NET Core 2.0 will be EOL'd on October 2018.

Hopefully there will be a contributor will step up and fix this blocking bug before the EOL deadline passes.

[YandyZaldivar]

Give a break to @caleblloyd he will work on this when he had some free time.

We are all waiting for the 2.1 final release.

[caleblloyd]

I will try to as I get free time but it would be great if some people in the community who have a good understanding of C# and a strong reliance on EF and MySQL could become core contributors. As #522 states I no longer use MySQL at my day job. It is always easier to debug this provider when you actively work on a big project that uses it, otherwise you don't hit all of the edge cases. For the issues in 2.1-rc like this that affect small a handful of projects these bugs are much more easily fixed by those who are actively working in those projects.

2.1-rc actually passes many more of the EF core tests than ever before, so I'd consider it pretty stable in its current state. The Microsoft team did a lot of work to get it that way.

Fixing every single upgrade issue in migrations might be tough, as stated before you can easily add SQL to a migration by hand to fix columns and get your migrations to work. Lots of ORMs don't have code first and I feel like with EF people forget that is an option sometimes. Sometimes hand patching the migration and moving on is the best course of action. Hopefully having the new EF functional tests in place would prevent this from happening in the future.

[ajcvickers]

@caleblloyd Would you consider releasing the RTM without this issue being fixed?
/cc @divega

[caleblloyd]

Yes in general if a manual migration fix can be applied and it can be documented in the release notes. If someone could comment on the manual migration fix that would be nice.

[AndriySvyryd]

This seems to be an issue with how the migrations have been generated previously. I don't think there's a reasonable way of accounting for it in the provider without changing the migrations manually.

@WillooWisp The repro you provided seems to have manually edited migrations, so I am not sure how well it represents the real app, but here are two ways of fixing it:

1. Identify the migration that's causing the error. Remove it and all migrations after it then create a new migration.
2. If you have to keep the existing migrations then identify the foreign key using the column that's being altered and add corresponding DropForeignKey and AddForeignKey calls around it in the migration.
   For the repro project it's FK_Attachments_FileConfigs_FileId, so the AddedUnknown migration would look like this:
   ```C#
   migrationBuilder.DropForeignKey(
   name: "FK_Attachments_FileConfigs_FileId",
   table: "Attachments");

migrationBuilder.AlterColumn(
name: "FileId",
table: "Attachments",
nullable: false,
oldClrType: typeof(Guid),
oldNullable: true);

migrationBuilder.AddForeignKey(
name: "FK_Attachments_FileConfigs_FileId",
table: "Attachments",
column: "FileId",
principalTable: "FileConfigs",
principalColumn: "Id",
onDelete: ReferentialAction.Cascade);
```

[WillooWisp]

@AndriySvyryd Thanks for your comments, well it represents the full migration set in my real app very well, only scaled down. The full migration set have not been edited manually, all migrations are generated, but in this case I guess it's one of the earlier migrations from the EF Core 1.0 time.

What I don't understand is why the migrations are valid with EF Core 2.0 and currently released pomelo version, compared to EF Core 2.1 and new pomelo version where it fails. Is it breaking changes in EF Core 2.1 or in the pomelo framework that causes this?

[AndriySvyryd]

@WillooWisp This change was introduced in f5a1e8845b1db3a62829124ea96906e7e9d99f60. Now migrations drop and recreate indexes when the column type changes, but if it was working before perhaps this is not needed.

@caleblloyd and other MySQL experts: Are there cases when the index needs to be dropped before a column definition is changed?

[caleblloyd]

I don't believe we saw an issue with the old behavior. This SO answer seems to state that it is taken care of in the ALTER TABLE process:

https://dba.stackexchange.com/questions/60526/what-effect-does-changing-the-data-type-of-an-existing-indexed-field-have-on-the

[AndriySvyryd]

Ok, then I'll send a PR to go back to the old behavior.

[caleblloyd]

Great I'll publish an RC2 once that makes it in

[caleblloyd]

Fixed in 2.1.0-rc2-final

[WillooWisp]

Thanks guys, I'll try it out ASAP. Good work!