Django fails to set nullable foreign key field to NULL before deleting the referenced record

[egenix_viktor]

Unit test: modeltests/delete

Test case:

# Since E.f is nullable, we should delete F first (after nulling out
# the E.f field), then E.

>>> o = CollectedObjects()
>>> e1._collect_sub_objects(o)
>>> o.keys()
[<class 'modeltests.delete.models.F'>, <class 'modeltests.delete.models.E'>]

>>> e1.delete()

It should work as described in the associated comment, since f_id is a nullable column of the delete_e table:

CREATE TABLE [delete_e] (
    [id] int IDENTITY (1, 1) NOT NULL PRIMARY KEY,
    [f_id] int NULL
)

Django should set the f_id field in the E record to NULL before trying to delete the F record, but it does not do so. Instead it tries to delete the F record first, which fails the foreign key constraint check associated to the f_id field of the delete_e table (E record).

It does not work on databases where deferred constrain checking (checking constraints only COMMIT time) is not an option. For example I found this behavior while running the unit tests through pyodbc (third party, I know) and MSSQL. (Relying on such deferred constraint checking is not a good practice at all.)

[Bob Thomas]

This is essentially the problem that caused me to re-open ticket #7778. The patch I attached should resolve this.

[Bob Thomas]

Fix regression from 7778, set FK to null

[Bob Thomas]

Django used to do this properly, but the change from #7778 broke this unintentionally. I re-opened #7778 with this fix, but your ticket describes the new problem better. Attached my original patch (with the line numbers slightly offset to match 1.0.x branch).

[Peter Sagerson]

In case it's not clear, modeltests/delete currently fails under MySQL 5.0.67, not just pyodbc and MSSQL. This is pretty close to a deal-breaker for MySQL support.

[hey560@…]

I tried the patch on trunk with sqlite and it doesn't seem to work.

[Bob Thomas]

I'm sorry to hear that, but some more information would be useful. What doesn't work?

[liangent]

in the patch, lambda f: f.column == field.rel.get_related_field().name, why can we expect that a field's name can equals to a field's column name?

[liangent]

also, i cannot understand why do we need this filter

[Bob Thomas]

Replying to liangent:

in the patch, lambda f: f.column == field.rel.get_related_field().name, why can we expect that a field's name can equals to a field's column name?

You're right, it should be lambda f: f.column == field.rel.get_related_field().column, of course

[Bob Thomas]

Fix tests

[Bob Thomas]

I updated the test case mentioned in this bug's description to actually verify that E.f is set to null. It fails before the patch and passes afterwords, regardless of backend or deferred constraint checking.

[liangent]

i still have the question that why do we need this filter?

[Bob Thomas]

Replying to liangent:

i still have the question that why do we need this filter?

It was added to fix #7778

[Jacob]

The test case is *nasty* -- maybe subclass UpdateQuery?

[markshep]

use subclassing instead of monkey patching

[Russell Keith-Magee]

(In [10822]) Fixed #9308 -- Corrected the updated of nullable foreign key fields when deleting objects. Thanks to Bob Thomas for the fix, and markshep for the improvements on the test case.

[Russell Keith-Magee]

(In [10823]) [1.0.X] Fixed #9308 -- Corrected the updated of nullable foreign key fields when deleting objects. Thanks to Bob Thomas for the fix, and markshep for the improvements on the test case.

Merge of r10822 from trunk.

[libraM]

See also

​http://stackoverflow.com/questions/1006135/how-do-i-create-a-django-model-with-foreignkeys-which-does-not-cascade-deletes-to

[Jacob]

Milestone 1.1 deleted