Comparator does not detect renamed foreign keys

[yakobe]

The origin of this problem is here: doctrine/orm#4783
The comparator does not detect a change for names of foreign keys. It works fine for indexes, but foreign keys are just ignored.

A test for this change might look something like this (to match the index test):
https://github.com/doctrine/dbal/blob/master/tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php#L628

public function testCompareForeignKeyBasedOnPropertiesNotName()
    {
        $tableA = new Table("foo");
        $tableA->addColumn('id', 'integer');
        $tableA->addNamedForeignKeyConstraint('foo_constraint', 'bar', array('id'), array('id'));

        $tableB = new Table("foo");
        $tableB->addColumn('ID', 'integer');
        $tableB->addNamedForeignKeyConstraint('bar_constraint', 'bar', array('id'), array('id'));

        $c = new Comparator();
        $tableDiff = $c->diffTable($tableA, $tableB);

        $tableDiff = new TableDiff('foo');
        $tableDiff->fromTable = $tableA;
        $tableDiff->renamedForeignKeys['foo_constraint'] = new ForeignKeyConstraint(array('id'),'bar', array('id'), 'bar_constraint');

        $this->assertEquals(
            $tableDiff,
            $c->diffTable($tableA, $tableB)
        );
    }

[senaria]

Is there any news about this? This possible solution looks like it would really help us out. Would be great if you could have a closer look.

[yakobe]

Is it possible to get some feedback about this issue? Is it a confirmed bug? Is there any possibility of it being resolved? Can I or other members of the community help in any way or is it not a desired change?
It would be really useful to get some sort of status / opinion here.

[Ocramius]

As far as I know, this would currently just drop and recreate a FK. Not sure if worth fixing atm.

[yakobe]

It causes a problem because it does not detect if a foreign key has been renamed. This means that it does not show up when creating automatic migrations, so the key keeps the old name after migrations.

If something more major then changes and want to drop this key, the new name is used in the generated migration. This new name is then not recognised when running the migration in production and everything crashes. Hard.

Normal indexes are handled correctly, but these are not. It's casuing us all manor of problems when preparing migrations. 😞

[Ocramius]

when running the migration in production and everything crashes. Hard.

You use doctrine/migrations or a similar tool for production handling, I hope?

@yakobe picking up the issue yourself is going to be the simplest way to get this fixed - unable to do it myself atm.

[yakobe]

You use doctrine/migrations or a similar tool for production handling, I hope?

Yes, we are using doctrine migrations.

Just so I understand, this is would be an acceptable change? If so, I will look a bit deeper into the code, perhaps i can create a PR.

[Ocramius]

Yes, although you must make sure that cascade operations are also equivalent.

[yakobe]

Yes, although you must make sure that cascade operations are also equivalent.

I would just try to fix it in a similar way to how the indexes work (and make the example test above work). What are the cascade operations that you are referring to?

[Ocramius]

What are the cascade operations that you are referring to?

Foreign keys may define cascade operation types - these must match in order for a rename to be suitable. If a cascade is changed, then a drop + recreate is needed.