Error when performing a cascading delete on models with foreign keys

[njones11]

I'm using django simple history in combination with the Django CMS Mezzanine, and in that situation we have curriculum vitaes (CVs) for our users where each CV is a mezzanine page. Additionally, there are entries which appear on these CVs (publications, awards, etc.) which are models with foreign keys to these CV pages. These CV items (publications, awards, etc) have their history tracked with simple history.

When deleting a CV page, the CV items are also deleted through a cascade, which is desired. However, when doing this cascading delete, Django throws a database integrity error, since the simple history entries don't get deleted and they now reference nonexistent models (the CV page).

I understand why simple history doesn't delete historical models when the base model is deleted, but it seems like its handling of foreign keys isn't correct. I can work around the issue I'm encountering by either a) setting the foreign keys on the cv items to be nullable or by b) overriding the delete method on the CV page to also delete the historical entries before calling super().delete()

However setting the keys to be nullable seems bad, and overriding the delete method on the CV page doesn't seem like it should be necessary.

It seems like the proper solution here would be for simple history to always create foreign keys in the historical model which are nullable, even when the foreign keys in the database are not nullable. This way, a simple history object could reference a deleted item without violating database constraints.

[macro1]

What version of Django are you using? In django-simple-history 1.6.1 we changed the transformation of the ForeignKey fields on historical models to set db_constraint to False, which I believe should prevent the database engine from knowing the column is a foreign key, and avoid any constraint issues when manipulating the original model.

[hakanb]

I'm also encountering this issue using Django==1.6.5 and django-simple-history==1.6.2.

[njones11]

I saw this error on Django 1.6.11 and django-simple-history==1.6.2. However, I did just realize that I didn't apply the migration generated by django-simple-history after upgrading, so it's possible that applying that migration will relax the DB constraint.

I'm currently in the process of upgrading to 1.8, so I'll let you know if that fixes my issue.

[hakanb]

I'd really appreciate hearing your results Nick, thanks.

On Fri, Jul 17, 2015 at 9:09 PM, Nick notifications@github.com wrote:

I saw this error on Django 1.6.11 and django-simple-history==1.6.2. However, I did just realize that I didn't apply the migration generated by django-simple-history after upgrading, so it's possible that applying that migration will relax the DB constraint.

I'm currently in the process of upgrading to 1.8, so I'll let you know if that fixes my issue.

— Reply to this email directly or view it on GitHub https://github.com/treyhunner/django-simple-history/issues/185#issuecomment-122361807 .

[hakanb]

FWIW I just upgraded to Django==1.8 and the error is still occuring.

On Fri, Jul 17, 2015 at 9:14 PM, Hakan Bakkalbasi hbakkalbasi@gmail.com wrote:

I'd really appreciate hearing your results Nick, thanks.

On Fri, Jul 17, 2015 at 9:09 PM, Nick notifications@github.com wrote:

I saw this error on Django 1.6.11 and django-simple-history==1.6.2. However, I did just realize that I didn't apply the migration generated by django-simple-history after upgrading, so it's possible that applying that migration will relax the DB constraint.

I'm currently in the process of upgrading to 1.8, so I'll let you know if that fixes my issue.

— Reply to this email directly or view it on GitHub https://github.com/treyhunner/django-simple-history/issues/185#issuecomment-122361807 .

[macro1]

@hakanb, the error is raised from the database. Django migrations should be disabling the database-level FK constraint, but if the migrations are not applied, the database will signal an exception which will trickle up through Django like any other database constraint. Could you verify the historical model in your schema does not have any foreign key constraints?

[njones11]

So I upgraded to Django 1.8, and I'm still seeing this issue. I went back and investigated some more, and it appears that the FK constraint still exists even though the field is allowed to be null.

Here is an example from our PG database:

Table "public.foo_historicalhonor"
     Column      |           Type           |                                 Modifiers                                 
-----------------+--------------------------+---------------------------------------------------------------------------
 id              | integer                  | not null
 _order          | integer                  | 
 name            | text                     | not null
 person_id       | integer                  | 
 show_on_web     | boolean                  | not null
 show_on_cv      | boolean                  | not null
 history_id      | integer                  | not null default nextval('foo_historicalhonor_history_id_seq'::regclass)
 history_date    | timestamp with time zone | not null
 history_user_id | integer                  | 
 history_type    | character varying(1)     | not null
Indexes:
    "foo_historicalhonor_pkey" PRIMARY KEY, btree (history_id)
    "foo_historicalhonor_history_user_id" btree (history_user_id)
    "foo_historicalhonor_id" btree (id)
    "foo_historicalhonor_person_id" btree (person_id)
Foreign-key constraints:
    "history_user_id_refs_id_f109dac2" FOREIGN KEY (history_user_id) REFERENCES auth_user(id) DEFERRABLE INITIALLY DEFERRED
    "person_id_refs_foopage_ptr_id_41875746" FOREIGN KEY (person_id) REFERENCES foo_person(foopage_ptr_id) DEFERRABLE INITIALLY DEFERRED

I'm not a Postgres expert, but is this saying that person_id can be NULL, but that there must be a corresponding NULL person_id in the persons table? That may explain why this still isn't working for us.

[hakanb]

@macro1 Yeah, no FK constraints.

@njones11 Thanks for the additional details.

I was able to fix this by:

• Rolling back migrations to the point before django-simple-history fields were added
• Removing django-simple-history from my project
• Upgrading to Django 1.8 (including migrations)
• Adding django-simple-history and corresponding fields to the project again
• Running makemigrations

[hakanb]

I take it back. I don't know why I thought this was fixed but it's still happening.

[hakanb]

@macro1 Would we be OK if we manually dropped the FK constraints?

[macro1]

@hakanb, obviously removing them from your database would just remove the consistency guarantee (which we don't want to begin with). So running a command manually to remove the constraint is perfectly fine.

If you're asking about removing the constraint from the historical model being generated by the app, I was hoping that was already the case, so I'll need to do some tests to reproduce the issue you all are seeing.

[njones11]

@macro1 After giving this some more thought, I wonder if the issue I'm seeing is that the model the simple history object is referencing via FK is being deleted first, before the simple history object gets updated. When the FK model gets deleted, simple history's model is in an invalid state since the FK column in the history object still references an non-null ID, and thus the DB throws an integrity error. In that case, it seems like the only way to prevent this is to either 1) never have simple history create a FK or 2) ensure that the simple history object's FK gets updated to NULL before the object it references is deleted.

Any thoughts?

[macro1]

I looked at it a bit more today, and I still don't think this is a bug with django-simple-history. The initial migrations I'm seeing generated with makemigration, and the SQL I'm generating with the sql command all leave off the foreign key constraint in the SQL definitions it's generating, which is what we want. Don't worry about null or not null, those are irrelivant. We want historical models to track relationships at the time of change, and to keep those values even after the related model is gone.

So I'm thinking either you are both seeing a relationship that is somehow still expressing a database-level foreign key constraint even though nearly all of them do not. Or there is a bug with Django's handling of the constraint flag we're taking advantage of. I do think there was a bug with migrations that once the FK constraints were applied, removing them with a migration had no affect, you needed to remove them manually. But @hakanb reported seeing this with a new installation, so I'm at a loss.

As far as working around the problem, since our goal is to only have one foreign key constraint on the historical models on the database level, manually removing the others after migrations complete is completely safe, and will give the desired behavior from the application.