#9308 closed (fixed)
Django fails to set nullable foreign key field to NULL before deleting the referenced record
Reported by: | egenix_viktor | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.0 |
Severity: | Keywords: | delete record foreign key constraint check NULL nullable field | |
Cc: | bthomas@…, liangent@…, jdunck@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.)
Attachments (3)
Change History (26)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
Cc: | added |
---|
comment:3 by , 16 years ago
Has patch: | set |
---|
follow-up: 5 comment:4 by , 16 years ago
comment:5 by , 16 years ago
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.
comment:6 by , 16 years ago
milestone: | → 1.1 |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:8 by , 16 years ago
I'm sorry to hear that, but some more information would be useful. What doesn't work?
follow-up: 12 comment:9 by , 16 years ago
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?
comment:11 by , 16 years ago
Needs tests: | set |
---|
comment:12 by , 16 years ago
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
comment:13 by , 16 years ago
Needs tests: | unset |
---|
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.
follow-up: 15 comment:14 by , 16 years ago
i still have the question that why do we need this filter?
comment:15 by , 16 years ago
comment:16 by , 16 years ago
Cc: | added |
---|
comment:17 by , 16 years ago
Cc: | added |
---|
comment:19 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:20 by , 15 years ago
comment:21 by , 15 years ago
comment:22 by , 14 years ago
Cc: | removed |
---|
This is essentially the problem that caused me to re-open ticket #7778. The patch I attached should resolve this.