4.x Migrations without Phinx

[markstory]

This is a (multiple allowed):

• bug

• enhancement

• feature-discussion (RFC)

• CakePHP Version: 5.x

• Migrations plugin version: 4.x

• Bake plugin version (if relevant): n/a

• Database server (MySQL, SQLite, Postgres): n/a

• PHP Version: 8.1+

• Platform / OS: n/a

We've recently had challenges with the ongoing maintenance and release cycles for the migrations plugin and phinx. These challenges have taken the form of:

1. Hard to maintain tests in Migrations. Debugging tests for migrations is frustrating and time-consuming, as the translation layers and indirection necessaitate integration testing.
2. Botched releases for both migrations and phinx. With migrations depending on phinx, and phinx depending on cakephp/orm, releases can be challenging to co-ordinate..

To allieviate these issues in the future, I propose that we remove phinx as a dependency from migrations. I think this would make migrations easier to test and maintain, and remove release orchestration issues. Furthermore, I think it would allow both packages to focus on what they are good at going forwards. Migrations can focus on supporting new features in CakePHP, and phinx can continue being a fantastic migrations library with a small footprint.

Backwards Compatibility

Ideally the user facing API for Migrations does not change in substantial ways. I see the user facing APIs for migrations to be:

1. The CLI commands used to create, inspect and execute migrations.
2. Migration files.

The day to day usage of migrations on the CLI shouldn't change in significant ways, and migrations that ran in the past should continue to run in the future. Minor adjustments will be required to user land code as they will need to rename Phinx to Migrations. I don't think we should include any class aliases for Phinx in future Migrations releases. Instead we should provide compatible interfaces and rector rules to aid in updating user land code.

Testing the backwards compatibility for migrations will require help from the community as our current test suites don't cover every scenario that may exist in application code.

Simplifing migrations

These changes present several opportunities to simplfy the migrations plugin:

1. We can remove translation layers for CLI commands.
2. We can remove translation logic for database connections.
3. We can leverage cakephp/database for driver and schema reflection

Keep what is working well

Instead of starting over, I would port code from phinx into migrations and adapt the ported code to new database infrastructure. This will not only save us time and effort, but also maintain more backwards compatibility.

More to come

This issue doesn't contain enough detail to cover all the changes that will be required. Instead I'd like to get alignment on the high level direction, and then develop a more refined plan.

[othercorey]

This will need a test case covering all of the supported configs in the migrations files if we expect backwards compatibility.

That will (in essence) means we have to implement a simple version of phinx in migrations.

Do we know how many phinx-only schema options there are in migrations files?

[markstory]

That will (in essence) means we have to implement a simple version of phinx in migrations.

Yes, we'd need to port & adapt all of the logic required to convert Migration objects into SQL statements. The most complex of these will be for sqlite as dropping a column there is still a mess.

That will (in essence) means we have to implement a simple version of phinx in migrations.

I'm not sure, I'm hoping to preserve those options by cloning the existing code into migrations as a starting point. I've not had much time to follow up on this issue, but I would like to make time for it in the upcoming weeks.

[PaulHendriks]

Will you be monitoring future changes in Phinx, to keep up sort of speak? Or will you fork and not look back.
My biggest concern is taking on the responsibility that now lays with another project. It does allow CakePHP core developers to have more influence on migration functionality, that's a plus.

[othercorey]

What may not be clear from this proposal is how difficult is it to work in the existing migrations plugin code.

One of the benefits would be removing that wrapper layer. There has been some recent work, but this plugin is often the last thing the core team wants to work on.

The Phinx project is starting to express more nterest in not being tied to CakePHP and as seen in the most recent Phinx and CakePHP 5 release, that coordination does not often go well.

[umer936]

What are thoughts about using some other Migrations library (e.g. https://www.doctrine-project.org/projects/migrations.html ?) and wrapper any Cake specific things? Does that just end up back at the same place it is now?

Keeping up with SQL versions and changes seems... not appealing. Even in current Migrations, I've had to do plain SQL queries because of issues in foreignKey differences in Postgres vs MySQL and managing that type of stuff seems hard.

[markstory]

What are thoughts about using some other Migrations library (e.g. https://www.doctrine-project.org/projects/migrations.html ?) and wrapper any Cake specific things? Does that just end up back at the same place it is now?

It could work, but we'll end up with similar library translation issues where we need to convert column types between cake & doctrine.

Keeping up with SQL versions and changes seems... not appealing. Even in current Migrations, I've had to do plain SQL queries because of issues in foreignKey differences in Postgres vs MySQL and managing that type of stuff seems hard.

It hasn't been too bad for us so far, each SQL dialect has its oddities, but once those are isolated schema changes mostly 'just work'. What kind foreign key issues have you run into that required manual SQL operations?

[umer936]

It hasn't been too bad for us so far, each SQL dialect has its oddities, but once those are isolated schema changes mostly 'just work'. What kind foreign key issues have you run into that required manual SQL operations?

It's very possible there are better ways to handle this, but here I needed a check for which DB type.

I think it's always been related to tables that have data already and such - not like when running the Migrations for the first time.

Migration: ChangeGuestUserId

// Up is the same, just with more stuff 
    public function down(): void
    {
        $isPostgres = $this->getAdapter()->getAdapterType() == 'pgsql';
        if ($isPostgres) {
            $this->query('BEGIN')->execute();
        } else {
            $this->query("SET FOREIGN_KEY_CHECKS=0;")->execute();
        }
        $builder = $this->getQueryBuilder();
        $builder
            ->update('action_log')
            ->set('user_id', -1)
            ->where(['user_id' => GUEST_USER_ID])
            ->execute();
        $builder
            ->update('action_log')
            ->set('user_id', -2)
            ->where(['user_id' => DELETED_USER_ID])
            ->execute();
        $builder = $this->getQueryBuilder();
        $builder
            ->update('users')
            ->set('id', -1)
            ->where(['id' => GUEST_USER_ID])
            ->execute();
        $builder = $this->getQueryBuilder();
        $builder
            ->update('users')
            ->set('id', -2)
            ->where(['id' => DELETED_USER_ID])
            ->execute();
        if ($isPostgres) {
            $this->query('COMMIT')->execute();
        } else {
            $this->query("SET FOREIGN_KEY_CHECKS=1;")->execute();
        }
    }

Migration: AddFileXmlToPlotLayouts

    public function up()
    {
        $table = $this->table('plot_layouts');
        if ($this->getAdapter()->getAdapterType() == 'pgsql') {
            $table->addColumn('file_xml', 'binary', [
                'default' => null,
                'null' => false,
            ]);
        } else {
            $table->addColumn('file_xml', 'mediumblob', [
                'default' => null,
                'null' => false,
            ]);
        }
        $table->update();

        $rows = $this->fetchAll('SELECT id, file FROM plot_layouts');

        foreach ($rows as $row) {
            if (file_exists(SAVED_LAYOUT_LOCATION . $row [1])) {
                $fileContents = file_get_contents(SAVED_LAYOUT_LOCATION . $row [1]);
                $builder = $this->getQueryBuilder();
                $builder
                    ->update('plot_layouts')
                    ->set('file_xml', $fileContents)
                    ->where(['id' => $row [0]])
                    ->execute();
            }
        }

        $table->removeColumn('file');
        $table->update();
    }

[markstory]

Thank you for the sample migrations. These look like operations we already have in CakePHP's database API so you would be able to use those methods to do some of these changes.

Needing to vary the column type on the driver class shouldn't need to exist. Looking into this further, I see we're missing good abstract column types for binary columns. Platform support is a bit funny here, as MySQL and SQLServer support length based blobs, sqlite and postgres do not. I'll open a separate issue to discuss this further.

Edit: Took another look and we will support addColumn('file_xml', 'binary', ['length' => TableSchema::LENGTH_LONG]) as a type to generate either a sized type or a general one depending the the connection driver.

[markstory]

4.3.0 has been released now, and includes the new backend functionality. If you have the time please give it a try and let us know how it works for your application's or plugin's migrations.

[umer936]

Another related topic is keeping track of Seeds.

Since it no longer relies on phinx, we're not limited to phinxlog... Should we have a seedslog? I regularly add new seeds and run them manually, but it would be handy if Cake knew which Seeds had already been run.

(I actually don't know if seeding used phinx at all. Never looked into it.)

[markstory]

Since it no longer relies on phinx, we're not limited to phinxlog... Should we have a seedslog? I regularly add new seeds and run them manually, but it would be handy if Cake knew which Seeds had already been run.

Yes we could have state tracking for seeds in the future. I would also like to combine the various phinxlog tables into a single that understands plugins better.

[challgren]

4.3.0 has been released now, and includes the new backend functionality. If you have the time please give it a try and let us know how it works for your application's or plugin's migrations.

I've tested it with Postgres and if you use a different schema than public, seeding the database no longer works.

[markstory]

Pull request for issue in #647 (comment) fixed in #728

[umer936]

A new test suite in phpunit on one of my applications was failing to set up through migrations on a local system but working correctly in a container. It was failing a foreign key check that was valid. (mariadb)

I'm not sure what the issue was but enabling the builtin backend instead of phinx resolved the issue, so that's good news 😃

[markstory]

Thanks for the feedback @umer936 I'm happy to hear that the builtin backend was able to preserve compatibility and fix a bug for you.