Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

Add config option to enable foreign key constraints for the SQLite driver #1254

Closed
SjorsO opened this issue Jul 11, 2018 · 9 comments
Closed

Comments

@SjorsO
Copy link

SjorsO commented Jul 11, 2018

I often use an in-memory sqlite database when running unit tests, and a mysql database in production. Today i ran into the issue that sqlite has foreign key constraints disabled by default. Foreign keys can be enabled by calling DB::connection()->getSchemaBuilder()->enableForeignKeyConstraints();, but forgetting to do this could cause situations that pass when testing, but fail in production.

I think it would be useful to add an extra config option in config/database.php to the sqlite driver, something like:

'sqlite' => [
    'driver' => 'sqlite',
    'database' => env('DB_DATABASE', database_path('database.sqlite')),
    'prefix' => '',
    'foreign_key_constraints' =>  true | false | default
],

If this value is not set, or set to default, Laravel won't do anything. If it is true or false, Laravel will automatically call enableForeignKeyConstraints() or disableForeignKeyConstraints() when creating an sqlite database connection.

@mfn
Copy link

mfn commented Jul 25, 2018

I often use an in-memory sqlite database when running unit tests, and a mysql database in production.

In reality this is the problem: if you rely on integration tests but test against a different service, you're already putting your tests in jeopardy. Even if you enable FKs, you may find other incompatibilities and your tests might not reveal it.

To be clear: nothing is wrong with the feature you suggested, but the issue provoking it is.

@SjorsO
Copy link
Author

SjorsO commented Jul 26, 2018

I agree with you that it is a risk to use different database drivers for testing and production. But, as far as i can tell, it is a common setup for Laravel projects.

As long as you don't write raw eloquent queries (which in my opinion you should always try to avoid), and don't use exotic column types, the risk of sqlite behaving differently than mysql are small. The only big behavior difference are the missing foreign key constraints.

Laravel helping with enabling the foreign keys constraints for sqlite prevents developers from running into foreign key related issues, this could save them a lot of time and frustration.

@sisve
Copy link

sisve commented Jul 26, 2018

You could also use sqlite in production. That way your tests matches the production, and the mismatch argument is null and void for this issue.

I write this to make sure that this issue isn't closed just because there's a mismatch between unit tests and production.

@dallincoons
Copy link

There's a lot of significant differences between sqlite and mysql once you start digging into it. As one example, sqlite doesn't respect column data types. Doesn't matter if it's a varchar or an int, in sqlite you can store whatever you feel like.

It's not that hard to set up a second MySql database, and run tests using transactions so you don't have to migrate every time, and if I'm not migrating every time I've found it's fast enough for my purposes.

@sisve
Copy link

sisve commented Jul 26, 2018

So, let's assume I have a sqlite database in production, and a sqlite database in development. I want foreign keys. This issue is about that. This issue isn't about telling people what differs between sqlite and another database engines.

@dallincoons
Copy link

Then why start the discussion by saying “I often use an in-memory sqlite database when running unit tests, and a mysql database in production.”?

@SjorsO
Copy link
Author

SjorsO commented Jul 26, 2018

The emphasis on my testing setup is because that is how i ran into the issue that this feature would have prevented.

Regardless of if my testing setup is a good idea, i think this feature would benefit anyone using an sqlite database.

Edit: not really relevant to the issue, but it seems like Taylor also uses an in-memory database for testing

@SjorsO
Copy link
Author

SjorsO commented Oct 10, 2018

related: laravel/docs#4608

@SjorsO
Copy link
Author

SjorsO commented Oct 30, 2018

This was implemented in laravel/framework#26298

@SjorsO SjorsO closed this as completed Oct 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants