Issue: Foreign key enforcement in SQLite
Author: wkschwartz
State: closed
Created: 2015-03-16 17:22:47

Foreign keys are disabled by [default in SQLite3](http://www.sqlite.org/foreignkeys.html#fk_enable). It would be nice to have an optional `enable_foreign_keys` boolean argument to the `SqliteDatabase` constructor. It would have two effects. Suppose `db = SqliteDatabase(filename, enable_foreign_keys=True)`. Then
1. `db.foreign_keys is True`; and
2. `db.create_tables` would issue

``` SQL
PRAGMA foreign_keys = ON;
```

Issuing that `PRAGMA` in the middle of a multi-statement transaction has no effect and swallows the error. It is only possible to turn on `foreign_keys` in SQLite versions greater than 3.6.19 when SQLite is compiled with `SQLITE_OMIT_TRIGGER` and `SQLITE_OMIT_FOREIGN_KEY` turned _off_. The versions of SQLite compiled into Python versions 2.7.9 and 3.4.3 on Windows appear to support `foreign_keys`.


You can subclass `SqliteDatabase` and override the `_add_conn_hooks` method. I don't think I will add this as a database option, but it should be easy to add yourself.

**Edit**: this is out-dated, see bottom of page for more info.

Using `_add_conn_hooks` doesn't work. In fact, it doesn't even work for `journal_mode`. See #554.

I created a wrapper connection function for my global database object `_db` that looks something like:

``` python
def connect():
    _db.connect()
    if isinstance(_db, peewee.SqliteDatabase):
       if _db.execute_sql('PRAGMA foreign_keys).fetchone() is None:
          _db.close()
          raise RuntimeError('Your installed version of SQLite does not support foreign keys.')
       _db.execute_sql('PRAGMA foreign_keys = ON')
```


I've gone ahead and addressed the issue in #554. I'm not sure why `_add_conn_hooks` won't work. Can you not write:

``` python
cursor = conn.cursor()
cursor.execute('pragma foreign_keys=ON')
```


I'm sure that would work. I just gave up too quickly.

Does the fact that you recommend `_add_conn_hooks` mean you intend on keeping its API stable? I assumed from the fact that it's a private method client code (such as mine) shouldn't use it.


I think it'd be good to add a public API for interacting with a new connection. I will add that, add docs, and update you with my progress.


Added 1040736. Added bonus, you can call `execute_sql()` from this hook.

Thanks for working with me on this. Good feature for sure.


Very cool.


I have a bit of feedback on this one, which I hope is helpful:

It strikes me that having to subclass SqliteDatabase is a little heavyweight for something that I would've expected to "just work". I just ran into this issue, and was quite surprised to find that Sqlite turns foreign-key behaviour off by default. Worse, it depends on compile-time options, etc, and even the Sqlite docs say not to depend on either behaviour as default.

I think it's fairly important for users to be aware of this and make it easy to turn it on. It's simple (and understandable) to assume that if you set on_delete='CASCADE' then that's exactly what will happen. 

In fact, this is a bigger problem than it first appears: because peewee does not specify AUTOINCREMENT on primary key fields, SQLite will feel free to re-use deleted ids. In combination with foreign keys not working, you can easily get nasty data bugs where an orphaned child will get re-linked to the next parent that comes along. http://sqlite.org/autoinc.html

A couple suggestions that I think would help a lot:
1. Mentioning this issue near the top of the Sqlite section of the docs, instead of being tucked away under "Additional connection initialization", would bring it to people's attention.
2. Specifying AUTOINCREMENT on id fields seems like a good idea.
3. Simpler code that doesn't require subclassing SqliteDatabase would be nice.
4. In fact, you can already do this simply with `SqliteDatabase(..., pragmas=[('foreign_keys', 'ON')])`. I propose that should be the "officially recommended" solution at least if the code were to remain status quo.
5. I think a `foreign_keys` parameter to SqliteDatabase() is a great idea! I'd be interested to know your rationale for not adding it.

Hope I'm not coming across as overly critical! Peewee's great, and I respect your work.

Is it ok to comment on closed issues like this, or should I raise a new issue?

SqliteDatabase can now be instantiated with pragmas, so:

```python
db = SqliteDatabase('filename.db', pragmas=(('foreign_keys', 'on'),))
```

> Specifying AUTOINCREMENT on id fields seems like a good idea.

This has performance implications. Hence, we use the default SQLite behavior but allow you to use auto-increment if you really want that. http://docs.peewee-orm.com/en/latest/peewee/playhouse.html#PrimaryKeyAutoIncrementField