Migrator truncates phinxlog of migration sets whose tables are not dropped.

[nishimura-d]

This is a (multiple allowed):

• [x] bug

• CakePHP Version: 4.4.6

• Migrations plugin version: 3.6.0

• Database server (MySQL, SQLite, Postgres): MySQL 8.0.29

• PHP Version: 8.1.14

• Platform / OS: Linux

What you did

1. define migrations of the application (create users) and the TestPlugin plugin (create test_plugin_entities).
2. put in tests/bootstrap.php:

   (new Migrator())->runMany([
       ['skip' => ['test_plugin_entities']],
       ['plugin' => 'TestPlugin', 'skip' => ['[!t]*', 't[!e]*']],
   ]);
   

3. run tests.
4. add an application migration.
5. run tests.
6. run tests.
7. run tests.

Expected Behavior

All runs succeed. On second run,

• application tables are dropped and re-created by migrations.
• TestPlugin tables are not dropped.

On third and forth runs, no tables are dropped.

Actual Behavior

Second run failed:

Error in bootstrap script: PDOException:
SQLSTATE[42S01]: Base table or view already exists: 1050 Table 'test_plugin_entities' already exists

• application tables are dropped and re-created by migrations.
• TestPlugin tables are not dropped, but test_plugin_phinxlog is empty.

Third run failed:

Error in bootstrap script: PDOException:
SQLSTATE[42S01]: Base table or view already exists: 1050 Table 'users' already exists

• application tables are not dropped, but phinxlog is empty.

Forth run succeeded.

• application tables are dropped and re-created by migrations.
• TestPlugin tables are dropped and re-created by migrations.

Cause

https://github.com/cakephp/migrations/blob/792cc076556884acf33098f36e881540db612704/src/TestSuite/Migrator.php#L217-L220

This truncates phinxlog of unrelated migration sets.

Workaround

This problem doesn't occur if we don't use the skip option, but tables are truncated twice in one run.

[markstory]

I took some time to look at this tonight and I'm not sure there is a feasible solution for this other than to throw errors when skip is used multiple times in a runMany() or, or to convert the 'table exists' error into a more user friendly one.

The problem boils down to skip is able to exclude some or all tables from a migration set (plugin or application). When we're attempting to reset both table schema and phinxlog state we can only operate at the connection level. Because both your plugin and application are using the same connection the queries we need to use to find which tables need to be dropped and which phinxlogs need to be reset pollute each other and overlap.

Because we don't have enough metadata to know which tables map to which phinxlog tables, and which migrations within those migration logs we can only operate on schema and phinxlogs in a coarse manner.

You might be able to get the results you're looking for by using run() instead of runMany() or by calling truncate() before you call runMany() and then setting the truncateTables parameter of runMany() to false.

[nishimura-d]

Sorry for late response. I understand that setting different skips for same connection is not expected use.

The problem is truncating same connection multiple times. I found a bug: https://github.com/cakephp/migrations/blob/b1b1459b149b0f27374aa4ba1706c5587f964126/src/TestSuite/Migrator.php#L94-L96

$connectionsList is array of array, but checking against string.

[markstory]

@nishimura-d That does look incorrect. I can get that condition and the related one for connectionsToDrop but those changes don't seem to change the behavior in a way that will address this issue as the test added in #604 is still passing which means the migration application failed.

[nishimura-d]

Yes, the Expected Behavior is not achieved. This issue stems from my misunderstanding. The reason I tried this in the first place was that the same table was being truncated over and over again, and I thought skip would work around this. This is solved by #607, so I won't try to use skip for this purpose anymore.

Once #607 is merged, I will write:

(new Migrator())->runMany([
    [],
    ['plugin' => 'TestPlugin'],
]);

1. On the second run, users and test_plugin_entities are dropped, phinxlog and test_plugin_phinxlog are truncated, both migrations succeed and users and test_plugin_entities are truncated once.
2. On the third run, no tables are dropped, phinxlog and test_plugin_phinxlog are not truncated, both migrations succeed and users and test_plugin_entities are truncated once.

This behavior is fine.

Since my misunderstanding has been resolved, I am fine with closing this issue as is. However, I still find the behavior of skip difficult to understand, and I would like to have something to help me understand it better. I think it is hard to understand that you can add skip to multiple migration sets of the same connection, but only the first one is used.

BTW, about "the test added in https://github.com/cakephp/migrations/pull/604".

     $migrator->runMany([ 
         ['plugin' => 'Migrator', 'skip' => ['migrator']], 
     ]); 

This would be the same. The Migrations source of the Migrator plugin creates the migrator table, so I would expect it to fail if you skip it. https://github.com/cakephp/migrations/blob/f262d6f8dffadefc5a986890a92c68e2fad71fa1/tests/test_app/Plugin/Migrator/config/Migrations/20211001000000_migrator.php#L9 That's why https://github.com/cakephp/migrations/blob/f262d6f8dffadefc5a986890a92c68e2fad71fa1/src/TestSuite/Migrator.php#L124-L125 I don't think this message is appropriate.

In my example, I skip tables that are not in each migration set. Such a specification didn't make sense, though.

[markstory]

I don't think this message is appropriate.

What would make more sense to you?

Since my misunderstanding has been resolved, I am fine with closing this issue as is.

Sounds good. I agree that skip with multiple sources on the same connection has a poor interaction. The original intention behind skip was to avoid tables that might be in the test database but don't have their schema and data managed by CakePHP.

[nishimura-d]

How about this?

If you are using the skip option and running multiple sets of migrations on the same connection, you can't skip tables managed by CakePHP in the connection.

[markstory]

That works for me too. Did you want to put a pull request together for that? If not I'll get to this eventually.

[nishimura-d]

No. I don't have enough time to make a pull request, sorry.