New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in migration when adding a foreign key with reference option = RESTRICT #556

Closed
pavlospap opened this Issue Nov 2, 2015 · 14 comments

Comments

Projects
None yet
4 participants
@pavlospap

hello I am using the 2.0.8 version and I have this problem. The following code

self::$_connection->addForeignKey('table_name', self::getDbName(), new Reference('fk_table_name_01', array(
            'referencedTable' => 'ref_table',
            'columns' => array('col_name'),
            'referencedColumns' => array('id'),
            'onUpdate' => 'RESTRICT',
            'onDelete' => 'RESTRICT')
        ));

throught the phalcon migration run command will produce a foreign key with reference option = NO ACTION for both update and delete.

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Jun 30, 2017

Member

Fixed in the 3.2.x branch. Feel free to open new issue if the problem appears again. Thank you for contributing.

Member

sergeyklay commented Jun 30, 2017

Fixed in the 3.2.x branch. Feel free to open new issue if the problem appears again. Thank you for contributing.

@topotru

This comment has been minimized.

Show comment
Hide comment
@topotru

topotru Aug 24, 2017

It seems that removing

self::$_connection->query('SET FOREIGN_KEY_CHECKS=0'); 

crashes migrations.

For the first table created, we need a foreign key to the table that has not yet been created.

And we get an error:

 SQLSTATE[HY000]: General error: 1215 Cannot add foreign key constraint

Migrations, which previously worked well now fall with an error.

topotru commented Aug 24, 2017

It seems that removing

self::$_connection->query('SET FOREIGN_KEY_CHECKS=0'); 

crashes migrations.

For the first table created, we need a foreign key to the table that has not yet been created.

And we get an error:

 SQLSTATE[HY000]: General error: 1215 Cannot add foreign key constraint

Migrations, which previously worked well now fall with an error.

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
Member

sergeyklay commented Aug 24, 2017

@sergeyklay sergeyklay reopened this Aug 24, 2017

@sergeysviridenko

This comment has been minimized.

Show comment
Hide comment
@sergeysviridenko

sergeysviridenko Aug 28, 2017

Contributor

@topotru Could you provide more information and steps for reproduce?

Contributor

sergeysviridenko commented Aug 28, 2017

@topotru Could you provide more information and steps for reproduce?

@topotru

This comment has been minimized.

Show comment
Hide comment
@topotru

topotru Aug 28, 2017

@sergeysviridenko
Oh sure.
Just two tables for example.

CREATE TABLE `notes` (
	`id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT,
	`text` TEXT NOT NULL,
	`created_by` INT(10) UNSIGNED NOT NULL,
	PRIMARY KEY (`id`),
	INDEX `FK_notes_users` (`created_by`),
	CONSTRAINT `FK_notes_users` FOREIGN KEY (`created_by`) REFERENCES `users` (`id`)
)
COLLATE='utf8_general_ci' ENGINE=InnoDB;

CREATE TABLE `users` (
	`id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT,
	`name` VARCHAR(50) NOT NULL,
	PRIMARY KEY (`id`)
)
COLLATE='utf8_general_ci' ENGINE=InnoDB;

And their migrations:

class NotesMigration_100 extends Migration
{
    public function morph()
    {
        $this->morphTable('notes', [
                'columns' => [
                    new Column(
                        'id',
                        [
                            'type' => Column::TYPE_INTEGER,
                            'unsigned' => true,
                            'notNull' => true,
                            'autoIncrement' => true,
                            'size' => 10,
                            'first' => true
                        ]
                    ),
                    new Column(
                        'text',
                        [
                            'type' => Column::TYPE_TEXT,
                            'notNull' => true,
                            'size' => 1,
                            'after' => 'id'
                        ]
                    ),
                    new Column(
                        'created_by',
                        [
                            'type' => Column::TYPE_INTEGER,
                            'unsigned' => true,
                            'notNull' => true,
                            'size' => 10,
                            'after' => 'text'
                        ]
                    )
                ],
                'indexes' => [
                    new Index('PRIMARY', ['id'], 'PRIMARY'),
                    new Index('FK_notes_users', ['created_by'], null)
                ],
                'references' => [
                    new Reference(
                        'FK_notes_users',
                        [
                            'referencedTable' => 'users',
                            'referencedSchema' => 'test',
                            'columns' => ['created_by'],
                            'referencedColumns' => ['id'],
                            'onUpdate' => 'RESTRICT',
                            'onDelete' => 'RESTRICT'
                        ]
                    )
                ],
                'options' => [
                    'TABLE_TYPE' => 'BASE TABLE',
                    'AUTO_INCREMENT' => '1',
                    'ENGINE' => 'InnoDB',
                    'TABLE_COLLATION' => 'utf8_general_ci'
                ],
            ]
        );
    }
}


class UsersMigration_100 extends Migration
{
    public function morph()
    {
        $this->morphTable('users', [
                'columns' => [
                    new Column(
                        'id',
                        [
                            'type' => Column::TYPE_INTEGER,
                            'unsigned' => true,
                            'notNull' => true,
                            'autoIncrement' => true,
                            'size' => 10,
                            'first' => true
                        ]
                    ),
                    new Column(
                        'name',
                        [
                            'type' => Column::TYPE_VARCHAR,
                            'notNull' => true,
                            'size' => 50,
                            'after' => 'id'
                        ]
                    )
                ],
                'indexes' => [
                    new Index('PRIMARY', ['id'], 'PRIMARY')
                ],
                'options' => [
                    'TABLE_TYPE' => 'BASE TABLE',
                    'AUTO_INCREMENT' => '1',
                    'ENGINE' => 'InnoDB',
                    'TABLE_COLLATION' => 'utf8_general_ci'
                ],
            ]
        );
    }
}

The reason is that migration files are processed in alphabetical order.
And the first file (NotesMigration_100) can't set the foreign key (FK_notes_users) on a non-existent table (users).

You can take code for reproduce here https://github.com/topotru/phalcon-migrations-test.git

Thank you for your contributing!

topotru commented Aug 28, 2017

@sergeysviridenko
Oh sure.
Just two tables for example.

CREATE TABLE `notes` (
	`id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT,
	`text` TEXT NOT NULL,
	`created_by` INT(10) UNSIGNED NOT NULL,
	PRIMARY KEY (`id`),
	INDEX `FK_notes_users` (`created_by`),
	CONSTRAINT `FK_notes_users` FOREIGN KEY (`created_by`) REFERENCES `users` (`id`)
)
COLLATE='utf8_general_ci' ENGINE=InnoDB;

CREATE TABLE `users` (
	`id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT,
	`name` VARCHAR(50) NOT NULL,
	PRIMARY KEY (`id`)
)
COLLATE='utf8_general_ci' ENGINE=InnoDB;

And their migrations:

class NotesMigration_100 extends Migration
{
    public function morph()
    {
        $this->morphTable('notes', [
                'columns' => [
                    new Column(
                        'id',
                        [
                            'type' => Column::TYPE_INTEGER,
                            'unsigned' => true,
                            'notNull' => true,
                            'autoIncrement' => true,
                            'size' => 10,
                            'first' => true
                        ]
                    ),
                    new Column(
                        'text',
                        [
                            'type' => Column::TYPE_TEXT,
                            'notNull' => true,
                            'size' => 1,
                            'after' => 'id'
                        ]
                    ),
                    new Column(
                        'created_by',
                        [
                            'type' => Column::TYPE_INTEGER,
                            'unsigned' => true,
                            'notNull' => true,
                            'size' => 10,
                            'after' => 'text'
                        ]
                    )
                ],
                'indexes' => [
                    new Index('PRIMARY', ['id'], 'PRIMARY'),
                    new Index('FK_notes_users', ['created_by'], null)
                ],
                'references' => [
                    new Reference(
                        'FK_notes_users',
                        [
                            'referencedTable' => 'users',
                            'referencedSchema' => 'test',
                            'columns' => ['created_by'],
                            'referencedColumns' => ['id'],
                            'onUpdate' => 'RESTRICT',
                            'onDelete' => 'RESTRICT'
                        ]
                    )
                ],
                'options' => [
                    'TABLE_TYPE' => 'BASE TABLE',
                    'AUTO_INCREMENT' => '1',
                    'ENGINE' => 'InnoDB',
                    'TABLE_COLLATION' => 'utf8_general_ci'
                ],
            ]
        );
    }
}


class UsersMigration_100 extends Migration
{
    public function morph()
    {
        $this->morphTable('users', [
                'columns' => [
                    new Column(
                        'id',
                        [
                            'type' => Column::TYPE_INTEGER,
                            'unsigned' => true,
                            'notNull' => true,
                            'autoIncrement' => true,
                            'size' => 10,
                            'first' => true
                        ]
                    ),
                    new Column(
                        'name',
                        [
                            'type' => Column::TYPE_VARCHAR,
                            'notNull' => true,
                            'size' => 50,
                            'after' => 'id'
                        ]
                    )
                ],
                'indexes' => [
                    new Index('PRIMARY', ['id'], 'PRIMARY')
                ],
                'options' => [
                    'TABLE_TYPE' => 'BASE TABLE',
                    'AUTO_INCREMENT' => '1',
                    'ENGINE' => 'InnoDB',
                    'TABLE_COLLATION' => 'utf8_general_ci'
                ],
            ]
        );
    }
}

The reason is that migration files are processed in alphabetical order.
And the first file (NotesMigration_100) can't set the foreign key (FK_notes_users) on a non-existent table (users).

You can take code for reproduce here https://github.com/topotru/phalcon-migrations-test.git

Thank you for your contributing!

@sergeysviridenko

This comment has been minimized.

Show comment
Hide comment
@sergeysviridenko

sergeysviridenko Aug 30, 2017

Contributor

@topotru This problem because referenced table isn't exist. You can add foreign key after creating second table.

Contributor

sergeysviridenko commented Aug 30, 2017

@topotru This problem because referenced table isn't exist. You can add foreign key after creating second table.

@topotru

This comment has been minimized.

Show comment
Hide comment
@topotru

topotru Aug 30, 2017

Do you suggest creating another migration file to add foreign keys?
And what about the existing versioned migration?
I have versions from 1.0.0 to 1.9.9
Each of them contains the installation of foreign keys.
And I can't do it - I haven't versions between existing nums.
And in general it is not correct. The keys must be directly applied in the migration file for a particular table.
My backward campability is broken. And I think that not only my.

May be it's more correct to make a collection of keys when executing the migration version folder and apply it at once for all tables after completion for each version?

topotru commented Aug 30, 2017

Do you suggest creating another migration file to add foreign keys?
And what about the existing versioned migration?
I have versions from 1.0.0 to 1.9.9
Each of them contains the installation of foreign keys.
And I can't do it - I haven't versions between existing nums.
And in general it is not correct. The keys must be directly applied in the migration file for a particular table.
My backward campability is broken. And I think that not only my.

May be it's more correct to make a collection of keys when executing the migration version folder and apply it at once for all tables after completion for each version?

@topotru

This comment has been minimized.

Show comment
Hide comment
@topotru

topotru Aug 30, 2017

Or maybe will be better to realise command line option like --foreign-key-check=false (by default - true) on migration run command.

topotru commented Aug 30, 2017

Or maybe will be better to realise command line option like --foreign-key-check=false (by default - true) on migration run command.

@topotru

This comment has been minimized.

Show comment
Hide comment
@topotru

topotru Aug 30, 2017

Also I want to note that the migrations generated by the "generate" command will also be invalid.
The migration files listed above are created by the "migration generate" command, but they can not be executed by the "migratory run" command.

topotru commented Aug 30, 2017

Also I want to note that the migrations generated by the "generate" command will also be invalid.
The migration files listed above are created by the "migration generate" command, but they can not be executed by the "migratory run" command.

@topotru

This comment has been minimized.

Show comment
Hide comment
@topotru

topotru Aug 30, 2017

@sergeyklay what do you think about it?

topotru commented Aug 30, 2017

@sergeyklay what do you think about it?

@sergeysviridenko

This comment has been minimized.

Show comment
Hide comment
@sergeysviridenko

sergeysviridenko Aug 31, 2017

Contributor

@topotru At the moment migration generate foreign key in morph method. After generating migration you can change code.

Contributor

sergeysviridenko commented Aug 31, 2017

@topotru At the moment migration generate foreign key in morph method. After generating migration you can change code.

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Aug 31, 2017

Member

We only accept bug reports, new feature requests and pull requests in GitHub.

For questions regarding the usage of the DevTools, support requests, and questions like this please visit the official forums.

Member

sergeyklay commented Aug 31, 2017

We only accept bug reports, new feature requests and pull requests in GitHub.

For questions regarding the usage of the DevTools, support requests, and questions like this please visit the official forums.

@sergeyklay sergeyklay closed this Aug 31, 2017

@topotru

This comment has been minimized.

Show comment
Hide comment
@topotru

topotru Aug 31, 2017

What are you talking about?!
Now dev-tools generate invalid migrations!

I expect that the migration files generated by the "migration generate" command can be executed by the command "migration run" without any changes, and execute without any errors.
It's not NFR!
It's broken migrations module.

topotru commented Aug 31, 2017

What are you talking about?!
Now dev-tools generate invalid migrations!

I expect that the migration files generated by the "migration generate" command can be executed by the command "migration run" without any changes, and execute without any errors.
It's not NFR!
It's broken migrations module.

@sergeyklay sergeyklay reopened this Aug 31, 2017

@sergeysviridenko

This comment has been minimized.

Show comment
Hide comment
@sergeysviridenko

sergeysviridenko Sep 1, 2017

Contributor

@topotru Actually migration works fine. Foreign key is very specific object. You have to take care about integrity of keys by yourself. There're many way to solve your problem: for example - you can execute SQL code for creating foreign key after running migration, or you can execute SET FOREIGN_KEY_CHECKS=0 first.

Contributor

sergeysviridenko commented Sep 1, 2017

@topotru Actually migration works fine. Foreign key is very specific object. You have to take care about integrity of keys by yourself. There're many way to solve your problem: for example - you can execute SQL code for creating foreign key after running migration, or you can execute SET FOREIGN_KEY_CHECKS=0 first.

@sergeyklay sergeyklay closed this Sep 1, 2017

@phalcon phalcon locked and limited conversation to collaborators Sep 1, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.