Skip to content

Conversation

JanJakes
Copy link
Member

@JanJakes JanJakes commented Sep 8, 2025

Implemented in this PR:

  • Add support for FOREIGN KEY constraints in CREATE TABLE.
  • Add support for REFECENCES clause in CREATE TABLE column definitions.
  • Add support for ADD FOREIGN KEY clause in ALTER TABLE.
  • Add support for DROP FOREIGN KEY clause in ALTER TABLE.
  • Add support for DROP CONSTRAINT clause in ALTER TABLE.
  • Add support for DROP PRIMARY KEY clause in ALTER TABLE.

Closes #236.
Fixes #229.

@JanJakes JanJakes force-pushed the foreign-keys branch 2 times, most recently from f2ad6dd to a61700a Compare September 10, 2025 14:35
@JanJakes JanJakes marked this pull request as ready for review September 10, 2025 15:18
@JanJakes JanJakes requested a review from adamziel September 10, 2025 15:18
'CREATE TABLE `t2` (',
' `id` int DEFAULT NULL,',
' `t1_id` int DEFAULT NULL,',
' CONSTRAINT `fk1` FOREIGN KEY (`t1_id`) REFERENCES `t1` (`id`) ON DELETE CASCADE',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I was looking for this and I'm glad to see we're testing for SHOW CREATE TABLE

@adamziel
Copy link
Collaborator

Do we support the following FOREIGN KEY options now?

RESTRICT | CASCADE | SET NULL | NO ACTION | SET DEFAULT

If not, let's throw when they're used.

@adamziel
Copy link
Collaborator

Of course the moment I submitted the comment I found unit tests for those. Lovely!

$this->quote_sqlite_identifier( $referential_constraint['REFERENCED_TABLE_NAME'] ),
implode( ', ', $referenced_column_names )
);
if ( 'NO ACTION' !== $referential_constraint['DELETE_RULE'] ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's covered by the tests but I still want to ask – NO ACTION is the default value of DELETE_RULE even if there is no explicit ON DELETE %s, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamziel Yes, in both MySQL and SQLite. And it never appears explicitly in the SHOW CREATE TABLE statement. It seems that it's pretty much equivalent to RESTRICT:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe to be even more precise, we could consider translating MySQL's NO ACTION to SQLite's RESTRICT.

From MySQL docs:

A keyword from standard SQL. For InnoDB, this is equivalent to RESTRICT; the delete or update operation for the parent table is immediately rejected if there is a related foreign key value in the referenced table.

From SQLite docs:

The "RESTRICT" action means that the application is prohibited from deleting (for ON DELETE RESTRICT) or modifying (for ON UPDATE RESTRICT) a parent key when there exists one or more child keys mapped to it. The difference between the effect of a RESTRICT action and normal foreign key constraint enforcement is that the RESTRICT action processing happens as soon as the field is updated - not at the end of the current statement as it would with an immediate constraint, or at the end of the current transaction as it would with a deferred constraint. Even if the foreign key constraint it is attached to is deferred, configuring a RESTRICT action causes SQLite to return an error immediately if a parent key with dependent child keys is deleted or modified.

Sounds like it would be a tiny bit more correct, maybe.

@adamziel
Copy link
Collaborator

adamziel commented Sep 10, 2025

This looks great, thank you @JanJakes! Feel free to merge.

Tangentially, would it be easy to add support for ALTER TABLE x DROP INDEX y and ALTER TABLE x DROP KEY y from here? Or are these fundamentally different?

@JanJakes
Copy link
Member Author

@adamziel

Tangentially, would it be easy to add support for ALTER TABLE x DROP INDEX y and ALTER TABLE x DROP KEY y from here? Or are these fundamentally different?

I had to look it up, but great news—it's already supported and tested.

@JanJakes JanJakes merged commit ba84357 into develop Sep 11, 2025
14 checks passed
@JanJakes JanJakes deleted the foreign-keys branch September 11, 2025 15:01
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.

Can't import database that contains Gravity Forms dump
2 participants