cakephp / migrations

CakePHP database migrations plugin
Other
138 stars 116 forks source link

Adding SET_NULL foreign keys fails #667

Closed dereuromark closed 6 months ago

dereuromark commented 10 months ago

This is a (multiple allowed):

What you did

$this->table('events')
            ->addForeignKey('city_id', 'cities', ['id'], ['delete' => 'SET_NULL'])
            ->addForeignKey('user_id', 'users', ['id'], ['delete' => 'SET_NULL'])
            ->addForeignKey('next_id', 'events', ['id'], ['delete' => 'SET_NULL'])
            ->update();

Expected Behavior

Adding those keys to preserve DB integrity.

Actual Behavior

Trying to add a SET_NULL key is not working anymore.

DOException: SQLSTATE[HY000]: General error: 1005 Can't create table xyz_local.events (errno: 150 "Foreign key constraint is incorrectly formed") in /shared/httpd/xyz/vendor/robmorgan/phinx/src/Phinx/Db/Adapter/PdoAdapter.php:198

Even if one tries to only set a single key.

This used to work as such (in 4.x), now not anymore. I also double checked that there werent any such keys defined yet.

dereuromark commented 10 months ago

I can confirm that this is based on the unsigned vs signed topic. The length (11 vs 10) seems not relevant.

I am not sure there is much this plugin can fix now for existing migrations. For freshly baked ones we might want to see how we can improve it.

umer936 commented 10 months ago

I'm honestly pretty stuck on this. I'm in the middle of writing a migration to run in between "old migrations" and "new migrations" that reads in every foreign key to an array, drops them, changes all 'user_id' fields to unsigned (11) int, then readds the foreign keys. I've tried many other things including setting fk check to 0 with no success.

dereuromark commented 10 months ago

Jep, also spend already days on this topic But I now have a solution - whipped up some helpful commands:

https://github.com/dereuromark/cakephp-setup/blob/master/docs/Console/Commands.md#dbunsigned

First I ran

to give me all problematic fields and migration code to execute

In some cases with already existing constraints between signed fields I have to tmp remove them and re add them afterwards, e.g.

        // manually added this line
        $this->table('cities')
            ->dropForeignKey('state_id')
            ->update();

        $this->table('cities')
            ->changeColumn('state_id', 'integer', [
                'default' => null,
                'null' => true,
                'signed' => false,
            ])
            ->update();

        $this->table('states')
            ->changeColumn('id', 'integer', [
                'autoIncrement' => true,
                'default' => null,
                'null' => false,
                'signed' => false,
            ])
            ->update();

        // manually added this line
        $this->table('cities')
            ->addForeignKey('state_id', 'states', ['id'], ['delete' => 'SET_NULL'])
            ->update();

This the script currently doesnt do for me, but I only had 2-3 cases, so fair enough. If someone wants to add this, should be doable actually (detecting the existing constraint and tmp removing+adding).

Then I ran

to give me all missing NOT NULL constraints for data integrity With the first command finished, this worked out of the box now.

umer936 commented 10 months ago

Here's mine to fix the unsigned.

https://gist.github.com/umer936/2b5bec1a4cdd3def9c50e5ab25f81d45

Working on making it more generic. Feel free to take from this if you find parts useful.

umer936 commented 10 months ago

I do think your idea of having it in a command is correct rather than a Migration as it could need to be run multiple times and one may not know the specific order.

dereuromark commented 6 months ago

I documented the commands at https://github.com/dereuromark/cakephp-setup/blob/master/docs/Console/Commands.md#db-integrity