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

[5.7] Allow disabling sqlite foreign keys #26306

Merged
merged 3 commits into from
Oct 30, 2018
Merged

[5.7] Allow disabling sqlite foreign keys #26306

merged 3 commits into from
Oct 30, 2018

Conversation

SjorsO
Copy link
Contributor

@SjorsO SjorsO commented Oct 30, 2018

#26298 added the ability to enable foreign key constraints for sqlite datbase connections using a config value. This PR allows you to disable foreign key constraints by setting the config value to false.

@dakira
Copy link
Contributor

dakira commented Oct 30, 2018

Hey. In what case would this be useful? Foreign key constraints are off by default. For the next Laravel release I would change this anyway because it should be on my default (as all other databases) and the Sqlite docs state this:

Note, however, that future releases of SQLite might change so that foreign key constraints enabled by default. Careful developers will not make any assumptions about whether or not foreign keys are enabled by default but will instead enable or disable them as necessary.

So I would propose a PR to master checking for false and enabling in all other cases.

@SjorsO
Copy link
Contributor Author

SjorsO commented Oct 30, 2018

The SQLite docs say that foreign key constraints can be enabled by default by passing a flag when compiling the library, so we can't assume that foreign keys are always disabled by default for all users.

It makes sense to be that if passing true enables the constraints, that passing false would disable them.

@dakira
Copy link
Contributor

dakira commented Oct 30, 2018

You're right, forget what I said. I didn't think this through. :) This should definitely be merged.

@dakira
Copy link
Contributor

dakira commented Oct 30, 2018

@SjorsO You should also expand the test that I submitted with my PR to make sure false actually disables the FKs. Not quite sure how to test this, though. Edit: I think this is actually hard to test without an actual sqlite binary that enables FKs by default.

@taylorotwell taylorotwell merged commit 15b8f29 into laravel:5.7 Oct 30, 2018
@SjorsO SjorsO deleted the patch-1 branch October 31, 2018 12:28
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

3 participants