Doctrine Migrations
  1. Doctrine Migrations
  2. DMIG-30

Make :migrations:diff smart and generate Schema object changes, not SQL directly

    Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Labels:
      None

      Description

      Currently when using :migrations:diff the generated output will use addSql() exclusively. However there has to be a way to generate changes on the schema objects themselves by using the SchemaDiff objects and generating code from them.

        Activity

        Benjamin Eberlei created issue -
        Hide
        Tyler Sommer added a comment -

        I think this would be something that would greatly increase the usefulness of migrations.

        After looking things over, I think doing something like this could work:

        • Instantiate an instance of Doctrine\ORM\Tools\Comparator, and retrieve a Doctrine\DBAL\Schema\SchemaDiff using $comparator->compare($fromSchema, $toSchema);
        • Iterate over each of the SchemaDiff's properties to retrieve new tables, dropped tables, columns, indexes, etc to generate both up and down code

        It seems fairly simple, but I get kind of lost thinking about how it can be tested.

        Another option, which may in the end be easier but more dirty, is to create a 'PhpPlatform' that only actually implements the getCreateTableSQL, getDropTableSQL, etc methods of Doctrine\DBAL\Platforms\AbstractPlatform.

        Show
        Tyler Sommer added a comment - I think this would be something that would greatly increase the usefulness of migrations. After looking things over, I think doing something like this could work: Instantiate an instance of Doctrine\ORM\Tools\Comparator, and retrieve a Doctrine\DBAL\Schema\SchemaDiff using $comparator->compare($fromSchema, $toSchema); Iterate over each of the SchemaDiff's properties to retrieve new tables, dropped tables, columns, indexes, etc to generate both up and down code It seems fairly simple, but I get kind of lost thinking about how it can be tested. Another option, which may in the end be easier but more dirty, is to create a 'PhpPlatform' that only actually implements the getCreateTableSQL, getDropTableSQL, etc methods of Doctrine\DBAL\Platforms\AbstractPlatform.
        Hide
        Tyler Sommer added a comment - - edited

        I've gone and followed what I outlined in my last comment, but there is a small snag.

        Comparator and SchemaDiff check for schema changes that are not possible to make using public methods. An example is a dropped index. Both Comparator and SchemaDiff support dropping indexes, but there is no Table#dropIndex to actually make it happen.

        However, if you use Reflection to modify the underlying Table#$_indexes array, then the proper SQL will be generated by SchemaDiff. Terrible and dirty, I know. We should do something about it Should I create a ticket?

        Anyway, I've attached a patch (based on the current master, 9e81984) of what I've come up with. I think I've covered everything. This will even generate code to drop indexes and foreign keys (using Reflection). I've tried it on a schema of around 90 entities and it works beautifully. Of course, it's definitely not done or ready, but I wanted to see what was thought about what I've done.

        Finally, what would be considered an acceptable test for this kind of thing? I suppose if I refactor all of the code generating bits into individual methods, I could pretty easily test for expected output. Am I on the right track?

        edit: by the way, use migrations:diff --no-platform to generate the migration

        Show
        Tyler Sommer added a comment - - edited I've gone and followed what I outlined in my last comment, but there is a small snag. Comparator and SchemaDiff check for schema changes that are not possible to make using public methods. An example is a dropped index. Both Comparator and SchemaDiff support dropping indexes, but there is no Table#dropIndex to actually make it happen. However, if you use Reflection to modify the underlying Table#$_indexes array, then the proper SQL will be generated by SchemaDiff. Terrible and dirty, I know. We should do something about it Should I create a ticket? Anyway, I've attached a patch (based on the current master, 9e81984) of what I've come up with. I think I've covered everything. This will even generate code to drop indexes and foreign keys (using Reflection). I've tried it on a schema of around 90 entities and it works beautifully. Of course, it's definitely not done or ready, but I wanted to see what was thought about what I've done. Finally, what would be considered an acceptable test for this kind of thing? I suppose if I refactor all of the code generating bits into individual methods, I could pretty easily test for expected output. Am I on the right track? edit: by the way, use migrations:diff --no-platform to generate the migration
        Tyler Sommer made changes -
        Field Original Value New Value
        Attachment DMIG-30.patch [ 11161 ]
        Hide
        David Simon added a comment -

        Note that there's also a PR for this change at https://github.com/doctrine/migrations/pull/79.

        I just tried it out myself, rebasing on latest master, and it works great. Please consider merging, developers, this feature is awesome!

        Show
        David Simon added a comment - Note that there's also a PR for this change at https://github.com/doctrine/migrations/pull/79 . I just tried it out myself, rebasing on latest master, and it works great. Please consider merging, developers, this feature is awesome!

        This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

        • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DMIG-30, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Benjamin Eberlei
          • Votes:
            2 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated: