Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#30330 closed Bug (fixed)

delete() on instances of models without any dependencies doesn't clear PKs.

Reported by: Artur Owned by: nobody
Component: Database layer (models, ORM) Version: 2.2
Severity: Release blocker Keywords: delete, pk
Cc: Florian Apolloner Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Deleting any model with no dependencies not updates the PK on the model. It should be set to None after .delete() call.

See Django.db.models.deletion:276-281. Should update the model line 280.

Attachments (2)

test-30330.diff (1.1 KB ) - added by Mariusz Felisiak 6 years ago.
Regression test.
fix-30330.diff (657 bytes ) - added by Florian Apolloner 6 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 by Mariusz Felisiak, 6 years ago

Cc: Florian Apolloner added
Severity: NormalRelease blocker
Summary: Deletion isn't updating modeldelete() on instances of models without any dependencies doesn't clear PKs.
Triage Stage: UnreviewedAccepted

Reproduced at 1ffddfc233e2d5139cc6ec31a4ec6ef70b10f87f.

Regression in bc7dd8490b882b2cefdc7faf431dc64c532b79c9.

Thanks for the report.

by Mariusz Felisiak, 6 years ago

Attachment: test-30330.diff added

Regression test.

by Florian Apolloner, 6 years ago

Attachment: fix-30330.diff added

comment:3 by Florian Apolloner, 6 years ago

I have attached a simple fix which mimics what https://github.com/django/django/blob/master/django/db/models/deletion.py#L324-L326 does for multiple objects. I am not sure if we need https://github.com/django/django/blob/master/django/db/models/deletion.py#L320-L323 (the block above) because I think field_updates is only ever filled if the objects are not fast-deletable -- ie https://github.com/django/django/blob/master/django/db/models/deletion.py#L224 is not called due to the can_fast_delete check at the beginning of the collect function.

That said, if we want to be extra "safe" we can just move lines 320 - 326 into an extra function and call that from the old and new location (though I do not think it is needed).

comment:5 by Florian Apolloner, 5 years ago

Has patch: set

comment:6 by Simon Charette, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: newclosed

In afc708c:

Fixed #30330 -- Fixed setting of primary key to None during fast-delete.

Regression in bc7dd8490b882b2cefdc7faf431dc64c532b79c9.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In b88ffef7:

[2.2.x] Fixed #30330 -- Fixed setting of primary key to None during fast-delete.

Regression in bc7dd8490b882b2cefdc7faf431dc64c532b79c9.

Backport of afc708cf6d047b35db57bd7c55baeffef459d279 from master

Note: See TracTickets for help on using tickets.
Back to Top