Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
doctrinebot opened this issue Sep 30, 2015 · 8 comments
Assignees

Comments

@doctrinebot
Copy link

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
Copy link
Author

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
Copy link

yakobe commented Jan 15, 2016

@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
Copy link
Member

@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
Copy link

senaria commented Jan 26, 2016

@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
Copy link
Member

@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
Copy link
Member

@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
Copy link

yakobe commented Apr 13, 2016

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
Copy link
Contributor

rodnaph commented Dec 7, 2016

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants