Skip to content
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

Fix dropForeignKey for SQLite #641

Merged
merged 3 commits into from
Sep 25, 2015

Conversation

HavokInspiration
Copy link
Member

Currently, if you define a foreign key constraint with ON UPDATE and ON DELETE statements in SQLite, under specific circumstances, the constraint drop results in an error as the new table is not created.

Take the example I added in the SQLiteAdapterTest file.
Given how the constraints are added, the create script is :

CREATE TABLE `table` (
    `id` INTEGER NULL PRIMARY KEY AUTOINCREMENT,
    `ref_table_id` INTEGER,
    `ref_table_field` VARCHAR(255),
    FOREIGN KEY (`ref_table_field`) REFERENCES `ref_table` (`field1`) ON DELETE CASCADE ON UPDATE CASCADE,
    FOREIGN KEY (`ref_table_id`) REFERENCES `ref_table` (`id`)
)

When this query is fed to the preg_replace() call that was modified in the SQLiteAdapter if a dropForeignKey('table', array('ref_table_field')) is done, it will result in :

CREATE TABLE `table` (
    `id` INTEGER NULL PRIMARY KEY AUTOINCREMENT,
    `ref_table_id` INTEGER,
    `ref_table_field` VARCHAR(255) ON DELETE CASCADE ON UPDATE CASCADE,
    FOREIGN KEY (`ref_table_id`) REFERENCES `ref_table` (`id`)
)

Which is invalid and will not "re-create" the table, having the consequence to simply dropping the table.
This PR aims to fix this by modifying the regex in two parts :

  • after the REFERENCES $reftable ($refcolumn) part by making the regex go down to a , or a ) which makes the regex eat up the ON DELETE and ON UPDATE part as well.
  • after the FOREIGN KEY ($column part by allowing other columns to be defined in case of composite foreign key constraints (which would mean that any constraints that column is bound to, whether simple or composite, will be dropped)

This covers the need I have (and fix the bug I had), so feel free to test this and give me feedback in case there are things I did not considered before merging.

…nd "ON DELETE" definitions, the SQLite dropForeignKey method will fail to properly re-generate the table
…onstraints using SQLite

The regex has had two modifications :
- it takes into account the fact that constraints can be define with "ON UPDATE" and "ON DELETE" columns definitions
- it also takes into account composite foreign key constraints. This means that if you have a constraint composed of 'fieldA' and 'fieldB' and you want to drop 'fieldA' FK, the composite constraint will be dropped
@robmorgan
Copy link
Member

thx

robmorgan added a commit that referenced this pull request Sep 25, 2015
@robmorgan robmorgan merged commit e5a7ba6 into cakephp:0.4.x-dev Sep 25, 2015
@HavokInspiration HavokInspiration deleted the sqlite-drop-fk branch November 30, 2015 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants