Doctrine DBAL
  1. Doctrine DBAL
  2. DBAL-106

Doctrine\DBAL\Schema\Comparator false positives

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2
    • Fix Version/s: 2.0.4
    • Component/s: Schema Managers
    • Labels:
      None
    • Environment:
      Doctrine 2.0.3, PHP 5.3.5, MySQL 5.5.9

      Description

      I am using the model class annotations for my Doctrine schema definition and a MySQL database for my target. When testing code that uses Doctrine\DBAL\Schema\Comparator, either by using $schemaTool->updateSchema($classes) which is defined in the core functionality or by running doctrine migrations:diff from the Doctrine Migrations project, a set of bugs are revealed. The symptoms of this are that immediately after calling $schemaTool->createSchema($classes) any call that uses the Comparator class to calculate the diffs between the real database and the schema definition produce a large set of false positives. Most of these false positives can be traced back to the logic in Doctrine\DBAL\Schema\Comparator::diffColumn(Column $column1, Column $column2). There are two issues here I can see right away:

      One is that if the model annotations rely on defaults rather than explicitly defining certain attributes, this function sees them as different. For example, if I have a column defined like this: @Column(type="decimal", scale=2, nullable=true) then the Comparator believes it is different from the real database because precision is 10 in the real data and 0 in my schema. If I change the line to this: if (($column1->getPrecision() ?: 10) != ($column2->getPrecision() ?: 10)) then it works, but I am sure a real solution needs to determine what the defaults actually are.

      The second issue is when I use the 'columnDefinition' attribute with a custom type to declare an enum or set, for instance. Since this function does not actually check what the columnDefinition field is set to at all, it has no way to know if I have actually made a change to the list of values, or anything else about that columnDefinition. I have been reading that using the comments field might be another way to do this, but I am not yet clear on what the best practice really is here. I know that http://www.doctrine-project.org/jira/browse/DBAL-42 touches on this with regard to storing Array type metadata in the comments field. I need more direction on how this should work in custom field types.

      In addition, I am getting this strange set of diffs on the index for a ManyToMany association's join table:

              $this->addSql("CREATE INDEX IDX_44B8C6D89AF7860 ON firm_firm (firm_id)");
              $this->addSql("DROP INDEX primary ON firm_firm");
              $this->addSql("CREATE UNIQUE INDEX primary ON firm_firm (firm_id, firm_id)");
      

      I am not sure why this only happens on this one table and not others, but it seems to be another case of the Comparator getting a false positive for some reason this time in Comparator::diffIndex(Index $index1, Index $index2). The model annotations for this association are:

        /**
         * @var Firm
         * @ManyToMany(targetEntity="Firm")
         */
        protected $Children = array();
      

      I will add more to this as I learn more details about the issues, but hopefully this is enough to start grokking the issues in Comparator. Getting the Comparator class in the core Doctrine\DBAL library working properly for calculating diffs will go a long way toward making well defined migrations a reality for Doctrine2 projects running in production. Hopefully, someone will also be able to address this minor issue in the Doctrine Migrations (http://www.doctrine-project.org/jira/browse/DMIG-21) so that it can be deployed without needing a patch.

      Thanks to everyone for Doctrine2, and I am looking forward to being among the first to put this new library into production on a real world project in a few months.

        Activity

        Lee Feistel created issue -
        Lee Feistel made changes -
        Field Original Value New Value
        Description I am using the model class annotations for my Doctrine schema definition and a MySQL database for my target. When testing code that uses Doctrine\DBAL\Schema\Comparator, either by using {{$schemaTool->updateSchema($classes)}} which is defined in the core functionality or by running {{doctrine migrations:diff}} from the Doctrine Migrations project, a set of bugs are revealed. The symptoms of this are that immediately after calling {{$schemaTool->createSchema($classes)}} any call that uses the Comparator class to calculate the diffs between the real database and the schema definition produce a large set of false positives. Most of these false positives can be traced back to the logic in {{Doctrine\DBAL\Schema\Comparator::diffColumn(Column $column1, Column $column2)}}. There are two issues here I can see right away:

        One is that if the model annotations rely on defaults rather than explicitly defining certain attributes, this function sees them as different. For example, if I have a column defined like this: {{@Column(type="decimal", scale=2, nullable=true)}} then the Comparator believes it is different from the real database because precision is 10 in the real data and 0 in my schema. If I change the line to this: {{if (($column1->getPrecision() ?: 10) != ($column2->getPrecision() ?: 10))}} then it works, but I am sure a real solution needs to determine what the defaults actually are.

        The second issue is when I use the 'columnDefinition' attribute with a custom type to declare an enum or set, for instance. Since this function does not actually check what the columnDefinition field is set to at all, it has no way to know if I have actually made a change to the list of values, or anything else about that columnDefinition. I have been reading that using the comments field might be another way to do this, but I am not yet clear on what the best practice really is here. I know that http://www.doctrine-project.org/jira/browse/DBAL-42 touches on this with regard to storing Array type metadata in the comments field. I need more direction on how this should work in custom field types.

        In addition, I am getting this strange set of diffs on the index for a ManyToMany association's join table:
        {noformat}
                $this->addSql("CREATE INDEX IDX_44B8C6D89AF7860 ON firm_firm (firm_id)");
                $this->addSql("DROP INDEX primary ON firm_firm");
                $this->addSql("CREATE UNIQUE INDEX primary ON firm_firm (firm_id, firm_id)");
        {noformat}

        I am not sure why this only happens on this one table and not others, but it seems to be another case of the Comparator getting a false positive for some reasonm this time in {{Comparator::diffIndex(Index $index1, Index $index2)}}. The model annotations for this association are:
        {noformat}
          /**
           * @var Firm
           * @ManyToMany(targetEntity="Firm")
           */
          protected $Children = array();
        {noformat}

        I will add more to this as I learn more details about the issues, but hopefully this is enough to start grokking the issues in Comparator. Getting the Comparator class in the core Doctrine\DBAL library working properly for calculating diffs will go a long way toward making well defined migrations a reality for Doctrine2 projects running in production. Hopefully, someone will also be able to address this minor issue in the Doctrine Migrations (http://www.doctrine-project.org/jira/browse/DMIG-21) so that it can be deployed without needing a patch.

        Thanks to everyone for Doctrine2, and I am looking forward to being among the first to put this new library into production on a real world project in a few months.
        I am using the model class annotations for my Doctrine schema definition and a MySQL database for my target. When testing code that uses Doctrine\DBAL\Schema\Comparator, either by using {{$schemaTool->updateSchema($classes)}} which is defined in the core functionality or by running {{doctrine migrations:diff}} from the Doctrine Migrations project, a set of bugs are revealed. The symptoms of this are that immediately after calling {{$schemaTool->createSchema($classes)}} any call that uses the Comparator class to calculate the diffs between the real database and the schema definition produce a large set of false positives. Most of these false positives can be traced back to the logic in {{Doctrine\DBAL\Schema\Comparator::diffColumn(Column $column1, Column $column2)}}. There are two issues here I can see right away:

        One is that if the model annotations rely on defaults rather than explicitly defining certain attributes, this function sees them as different. For example, if I have a column defined like this: {{@Column(type="decimal", scale=2, nullable=true)}} then the Comparator believes it is different from the real database because precision is 10 in the real data and 0 in my schema. If I change the line to this: {{if (($column1->getPrecision() ?: 10) != ($column2->getPrecision() ?: 10))}} then it works, but I am sure a real solution needs to determine what the defaults actually are.

        The second issue is when I use the 'columnDefinition' attribute with a custom type to declare an enum or set, for instance. Since this function does not actually check what the columnDefinition field is set to at all, it has no way to know if I have actually made a change to the list of values, or anything else about that columnDefinition. I have been reading that using the comments field might be another way to do this, but I am not yet clear on what the best practice really is here. I know that http://www.doctrine-project.org/jira/browse/DBAL-42 touches on this with regard to storing Array type metadata in the comments field. I need more direction on how this should work in custom field types.

        In addition, I am getting this strange set of diffs on the index for a ManyToMany association's join table:
        {noformat}
                $this->addSql("CREATE INDEX IDX_44B8C6D89AF7860 ON firm_firm (firm_id)");
                $this->addSql("DROP INDEX primary ON firm_firm");
                $this->addSql("CREATE UNIQUE INDEX primary ON firm_firm (firm_id, firm_id)");
        {noformat}

        I am not sure why this only happens on this one table and not others, but it seems to be another case of the Comparator getting a false positive for some reason this time in {{Comparator::diffIndex(Index $index1, Index $index2)}}. The model annotations for this association are:
        {noformat}
          /**
           * @var Firm
           * @ManyToMany(targetEntity="Firm")
           */
          protected $Children = array();
        {noformat}

        I will add more to this as I learn more details about the issues, but hopefully this is enough to start grokking the issues in Comparator. Getting the Comparator class in the core Doctrine\DBAL library working properly for calculating diffs will go a long way toward making well defined migrations a reality for Doctrine2 projects running in production. Hopefully, someone will also be able to address this minor issue in the Doctrine Migrations (http://www.doctrine-project.org/jira/browse/DMIG-21) so that it can be deployed without needing a patch.

        Thanks to everyone for Doctrine2, and I am looking forward to being among the first to put this new library into production on a real world project in a few months.
        Benjamin Eberlei made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Benjamin Eberlei made changes -
        Fix Version/s 2.0.4 [ 10131 ]
        Affects Version/s 2.0.2 [ 10118 ]
        Affects Version/s 2.0.3 [ 10119 ]
        Benjamin Eberlei made changes -
        Workflow jira [ 12512 ] jira-feedback2 [ 17708 ]
        Benjamin Eberlei made changes -
        Workflow jira-feedback2 [ 17708 ] jira-feedback3 [ 20063 ]

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Lee Feistel
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: