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

[8.x] Throw an exception when trying to add a foreign key to an existing SQLite database #32583

Closed
wants to merge 1 commit into from
Closed

[8.x] Throw an exception when trying to add a foreign key to an existing SQLite database #32583

wants to merge 1 commit into from

Conversation

SjorsO
Copy link
Contributor

@SjorsO SjorsO commented Apr 28, 2020

I just spent an hour trying to figure out why a foreign key constraint wasn't working in one of my tests. As it turns out, SQLite doesn't support adding foreign keys to existing databases. Doing this in a migration doesn't throw an exception, you'll only notice your foreign key constraints aren't working when you start seeing weird behavior in your code (such as ON DELETE CASCADE not working).

What makes this issue even harder to debug, is that when you look up "SQLite foreign key constraints not working", all the answers you find will say "foreign keys are disabled by default, you should enable them manually". Laravel has been enabling foreign keys for SQLite since #26298, so all these answers aren't useful. You have to specifically look up "add foreign key to SQLite table" to figure out why it isn't working.

This PR makes it so that an exception is thrown if you try to add a foreign key to an existing SQLite database. It also adds tests for the exceptions that are thrown when you try to run commands that aren't available for SQLite.

I'm targeting 8.x because this change will likely break some users their migrations.

@taylorotwell
Copy link
Member

I think this is maybe better left as documentation. This will prevent people from running some basic tests on SQLite when their application uses foreign keys.

@SjorsO
Copy link
Contributor Author

SjorsO commented Apr 29, 2020

@taylorotwell What do you think about a prevent_using_unsupported_commands config option for SQLite connections? If it defaults to false, it won't bother people doing basic tests. If you want to make sure you don't get bit by weird problems like this you can manually set it to true. A config option like that would also nudge people to read the documentation.

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.

2 participants