DDC-3924: doctrine:schema:update does not recognize that a FK index name has changed

[doctrinebot]

Jira issue originally created by user senaria:

Reproduce:

• Create a DB containing FK's (e.g. using doctrine:schema:create)
• change one of the foreign key index names to something different
• run app/console doctrine:schema:update --dump-sql

Actual:

• no change detected

Expected:

• FK index name change should be recognized

[doctrinebot]

Comment created by yakobeyak:

This is not just with foreign keys but all index types. it appears it does not recocnise that the name is different. This cause issues when trying to prepare migrations etc.

[yakobe]

@beberlei Could you perhaps have a look at this too see if this is actually a bug in your eyes, or maybe we missed something? Thanks

[deeky666]

@yakobe this sound more like a comparator issue in DBAL. Not sure this is ORM related. Can you please provide a concrete reproducable test case? Also which ORM/DBAL versions are you on? Thanks!

[senaria]

@deeky666 I created a project for reproducing this error. It has a simple relation between 2 entities. One index and one foreign key has been manipulated to be wrong.

In brief, its how I said above; changing a name of FK won't be recognized from the doctrine:schema:update command. See this screenshot. As you can see, the renamed IDX is recognized but not the renamed FK.

Reproduce:

• get the project
• create the db and the schema
• go inside the db to table test_one and rename the FK and IDX to whatever you like (eg fk_wrong and idx_wrong)
• run doctrine:schema:update -> only the IDX is recognized :cry:

[Ocramius]

@senaria what @deeky666 needs in order to reproduce the bug is a test that is working in isolation against the schema tool (see for example https://github.com/doctrine/doctrine2/blob/a4d84e0cd8cc4ea1b99f4fd7d1d560f3fbe27c30/tests/Doctrine/Tests/ORM/Functional/SchemaTool/DDC214Test.php for one test doing that).

[deeky666]

@senaria reading this I think we'll have to clarify some things first. What exactly is this issue about? The headline says it's about FK index name changes. FKs and indexes are distinct database objects and can be named differently. So according to your investigation, index renaming works as expected. FK renaming is currently unsupported by Doctrine. Actually this is related to DBAL and not ORM. DBAL's schema comparator does not take into account the name when comparing two FKs for differences. So if I understood everything correctly, then is not a bug but a feature request (should go into DBAL issue tracker though).

[yakobe]

Just to keep the thread i will continue here. We can create an issue in DBAL if this discussion is going anywhere...

It seems that foreign key name changes are ignored. There is a test in DBAL (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);

        $this->assertFalse($tableDiff);
    }

The simplest solution for our problem would be to add an extra check in https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Schema/Comparator.php#L389:

    public function diffForeignKey(ForeignKeyConstraint $key1, ForeignKeyConstraint $key2)
    {
        if (array_map('strtolower', $key1->getUnquotedLocalColumns()) != array_map('strtolower', $key2->getUnquotedLocalColumns())) {
            return true;
        }

        if (array_map('strtolower', $key1->getUnquotedForeignColumns()) != array_map('strtolower', $key2->getUnquotedForeignColumns())) {
            return true;
        }

        if ($key1->getUnqualifiedForeignTableName() !== $key2->getUnqualifiedForeignTableName()) {
            return true;
        }

        if ($key1->onUpdate() != $key2->onUpdate()) {
            return true;
        }

        if ($key1->onDelete() != $key2->onDelete()) {
            return true;
        }

        // start of change
        if ($key1->getName() !== $key2->getName()) {
            return true;
        }
        // end of change

        return false;
    }

Is there a reason for this? Or can this be changed?

[rodnaph]

I ran into this issue where I'd altered the schema (which updated some index names) in a branch, then coming back to master the schema tool did not recognise them as needing to be updated (just my busted development setup, but showed how the schema change tool ignored the different key names)

Trying the suggested addition of the key name check worked for me as a fix.

(No opinion on if this is a bug in anything, just reporting experience)