MySQL Bugs: #74726: EF migrations fail on long foreign keys

Bug #74726	EF migrations fail on long foreign keys
Submitted:	7 Nov 2014 8:51	Modified:	13 Nov 2015 16:26
Reporter:	Yannic Staudt	Email Updates:
Status:	Verified	Impact on me:	None
Category:	Connector / NET	Severity:	S2 (Serious)
Version:	6.9.4, 6.9.5, 6.9.6, 6.9.7, 6.9.8	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	entity framework, long foreign key, migrations

Description:
The Entity Framework migrations fail because of a - excuse the wording - stupid fix introduced in Connector/NET 6.6.6

Citing the changelog:
---
When using EntityFramework 4.3 and Code First, generated foreign key identifiers could be longer than 64 characters, causing MySQLException errors. The fix renames any too-long identifiers using the name format FK_guid, where guid is a global unique identifier generated at runtime. (Bug #16286397, Bug #67285)
---

This issue is that on subsequent migrations (foreign key rename as an example) the migration script can't "find" that random GUID because it has absolutely no relation to the original foreign key name...

The example below is a bit beyond what actually would be required, but it shows the error. In a real use case we have keys like: "FK_TSFSPMethodTestSpecifications_TSFSPMethods_TSFSPMethod_Id" and longer begin generated.

How to repeat:
public class SomeFancyNamedData
{
   public int Id { get; set; }
   public ICollection<SomeOtherNamedEntityData> Entity1Collection { get; set; }
}

public class SomeOtherNamedEntityData
{
   public int Id { get; set; }
   public SomeFancyNamedData SomeFancyData { get; set; }
   public int SomeFancyNamedDataId { get; set; }
}

// this should generate an foreign key like
// FK_SomeFancyNamedDatas_SomeOtherNamedEntityDatas_SomeOtherNamedEntityDataId
// which is clearly longer than 64 chars

Now generate a migration and apply it to the db.
Then rename the SomeFancyNamedData entity to SomeFancyNamedDataIAddedSomething (and all the linked properties: foreign key id in the second entity etc...).
This will make the next migration want to rename the FK. Which it currently won't be able to do because the Connector/NET will have generate a FK like: FK_*GUID HERE* thus the migration will fail.

Suggested fix:
Generate the foreign key name in non-random manner, and make it in a way that using the same input twice will result in the same result twice...

Here an example:
private string EscapeForeignKeyName(string fkName)
{
    if (fkName.Length > 64)
    {
        StringBuilder sb = new StringBuilder();

        var part1 = fkName.Substring(0, 51);
        sb.Append(part1);
        sb.Append("_");

        var part2 = fkName.Substring(51);
        var md5 = System.Security.Cryptography.MD5.Create();
        byte[] buf = System.Text.Encoding.UTF8.GetBytes(part2);
        byte[] hash = md5.ComputeHash(buf, 0, buf.Length);

        for (int i = 0; i < 6; i++)
            sb.Append(hash[i].ToString("x2"));

        return sb.ToString();
    }

    return fkName;
}

=> it truncates too-long foreign keys at char 51 and uses the "too much part" (char 52->EOSTR) as input to MD5() of which the first six values are added to the resulting foreign key.

The result for the foreign key:
FK_SomeFancyNamedDatas_SomeOtherNamedEntityDatas_SomeOtherNamedEntityDataId

Would be:
FK_SomeFancyNamedDatas_SomeOtherNamedEntityDatas_So_c5a6b65f9c8a

Which leaves much entropy and is repeatable.

In a quick walk-through the Connector/NET 6.9.4's sources I found only two places where to use the escaping:

Project: "MySql.Data.Entity"
File: "MySqlMigrationSqlGenerator.cs"
Class: "MySqlMigrationSqlGenerator"

1/ Add the above method to the class
2/ Line 616:

Replace:
string fkName = op.Name;
if (fkName.Length > 64)
{
    fkName = "FK_" + Guid.NewGuid().ToString().Replace("-", "");
}

With:
string fkName = EscapeForeignKeyName(op.Name);

3/ Line ~705:

Replace:
sb = sb.AppendFormat("alter table `{0}` drop foreign key `{1}`", op.DependentTable, op.Name);

With:
sb = sb.AppendFormat("alter table `{0}` drop foreign key `{1}`", op.DependentTable, EscapeForeignKeyName(op.Name));

Hello Yannic Staudt,

Thank you for the bug report.
Verified based on internal discussion with dev's.

Thanks,
Chiranjeevi.

Bug#75831 marked as duplicate of this one.

And one more connector released with this stupid bug still included. IT IS UNBELIEVABLE! I'm starting to get really disappointed.

This is a very suprising bug in the connector. In what world does using a GUID for naming seem like a good idea when you need to re-reference the name later?

Are there any suggested work around for this? I have an existing DB where I need to change the foreign and have no consistent way of doing this. Can I determine the current name at runtime somehow?

Thanks,

Karl

We do use a patched version of Connector/NET and did re-build the foreign key declarations half-automatically (with a tricky SQL script our DBA implemented).

The patched version uses the fix I proposed in my bug report initially. It's not a perfect solution but we didn't have any issue with it 'till now.

Issue is still there in 6.9.7; 

I fixed it this way: http://k16c.eu/2015/09/21/how-to-make-code-first-migrations-truly-usable/

Here's a derived MySQL Migrations Generator as a Nuget package: https://www.nuget.org/packages/Pysco68.MySQL.FixedMigrationsGenerator/

Error still present in Connector/NET 6.9.8

4 years later this issue is still present.

An apparent fix to this issue is to register a new RelationalMaxIdentifierLengthConvention(64) somewhere in MySqlConventionSetBuilder.cs.

See also https://github.com/aspnet/EntityFrameworkCore/commit/e8fcce13870909944e88a0cfba88b37453fc8..., which fixed the EF Core issue 10213, EF Generates too long index name, https://github.com/aspnet/EntityFrameworkCore/issues/10213.

Thanks Mr. Koen D'Hondt for bringing this long outstanding bug back to attention, and for your analysis of the bug.
This bug makes the EFCore package somehow unusable if you want to apply EF migrations, so it is indeed very unfortunate that this still isn't fixed, more than 4 years after the original bug report.
And even more unfortunate, since it doesn't even seem that difficult to fix, as Mr. D'Hondt's analysis points out.