Opened 13 years ago

Closed 11 years ago

#15146 closed Bug (duplicate)

Reverse relations for unsaved objects include objects with NULL foreign key

Reported by: treyh Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Model with parent relation and children reverse relation:

class TestModel(models.Model):
    parent = models.ForeignKey('self', blank=True, null=True, related_name='children')

Bug found while using model:

t = TestModel()
t.save()
u = TestModel()
u.children.all() # Incorrectly returns: [<TestModel: TestModel object>]
u.save()
u.children.all() # Correctly returns: []

Attachments (3)

tests.diff (1.6 KB ) - added by elbarto 13 years ago.
Tests added.
tests.2.diff (1.6 KB ) - added by elbarto 13 years ago.
New tests added.
patch.diff (926 bytes ) - added by elbarto 13 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 by treyh, 13 years ago

Summary: Reverse relations include unsaved objects (NULL id)Reverse relations include objects with NULL foreign key

comment:2 by treyh, 13 years ago

Summary: Reverse relations include objects with NULL foreign keyReverse relations for unsaved objects include objects with NULL foreign key

comment:3 by Russell Keith-Magee, 13 years ago

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted

This is an edge case that exists when an object has been created, but not saved; at that point, the primary key of the object is NULL, so the reverse query does a lookup with that value.

The fix here will be to prevent related object lookups when pk=None.

comment:4 by elbarto, 13 years ago

Owner: changed from nobody to elbarto

by elbarto, 13 years ago

Attachment: tests.diff added

Tests added.

by elbarto, 13 years ago

Attachment: tests.2.diff added

New tests added.

by elbarto, 13 years ago

Attachment: patch.diff added

comment:5 by elbarto, 13 years ago

Has patch: set

comment:6 by Łukasz Rekucki, 13 years ago

Severity: Normal
Type: Bug

comment:7 by Julien Phalip, 13 years ago

Patch needs improvement: set

Could you combine the fix and tests into one single diff patch?

comment:8 by elbarto, 13 years ago

Easy pickings: unset
Owner: changed from elbarto to nobody

comment:9 by Luke Plant, 12 years ago

UI/UX: unset

Ticket #17541 is a duplicate. I don't know which one to close at this point.

comment:10 by Stavros Korokithakis, 11 years ago

As I said on the other ticket, please upgrade the severity of this. It has just caused entirely sane-looking code to delete over half our database rows. This needs to be fixed ASAP, the patch has been submitted over two years ago.

comment:11 by Russell Keith-Magee, 11 years ago

Resolution: duplicate
Status: newclosed

Closing this one as a duplicate of #17541. It doesn't really matter which one we leave open, as long as there's only one ticket per problem.

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