Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#18250 closed Bug (fixed)

Comparing two unsaved model objects

Reported by: yrdnsa@… Owned by: Yarden
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: compare model
Cc: moritz.sichert@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

if i do this:

User(email="exmple@…") == User(email="exmple22@…")

it will return True!

but if i save them- it will return False.
this is because the eq magic method tries to compare their PK's. if no pk it will return None. so None == None...

django.db.models.base line 383
def eq

Change History (10)

comment:1 by anonymous, 12 years ago

Type: UncategorizedBug

comment:2 by Moritz Sichert, 12 years ago

Cc: Moritz Sichert added
Triage Stage: UnreviewedDesign decision needed
Version: 1.3master

Changing the __eq__ method to check if the pk value is None would cause that all non-saved model objects would return False when being compared:

>>> o = MyModel(text='foo')
>>> o == o
False

comment:3 by Alex Gaynor, 12 years ago

It doesn't have to, you can have it fall back to an identity comparison if the pks are None.

comment:4 by Claude Paroz, 12 years ago

Triage Stage: Design decision neededAccepted

comment:5 by Moritz Sichert, 12 years ago

Cc: Moritz Sichert removed

comment:6 by Moritz Sichert, 12 years ago

Cc: moritz.sichert@… added

comment:7 by Anssi Kääriäinen, 12 years ago

See also #11892, #16458

How about something like:

def __eq__(self, other):
    if not isinstance(other, Model):
        return False
    if self._get_pk_val() is None:
        return id(self) == id(other)
    if self._meta.concrete_class != other._meta.concrete_class:
        return False
    return self._get_pk_val() == other._get_pk_val()

Not tested...

Last edited 12 years ago by Anssi Kääriäinen (previous) (diff)

comment:8 by jcspray@…, 12 years ago

aakaariai's suggestion worked for me when I encountered this issue.

comment:9 by Anssi Kääriäinen, 11 years ago

Resolution: duplicate
Status: newclosed

I think this one is duplicate of #18864 (it has a slightly bigger scope as it also deals with __hash__).

comment:10 by Anssi Kääriäinen <akaariai@…>, 11 years ago

Resolution: duplicatefixed

In 6af05e7a0f0e4604d6a67899acaa99d73ec0dfaa:

Fixed model.eq and hash for no pk value cases

The eq method now considers two instances without primary key value
equal only when they have same id(). The hash method raises
TypeError for no primary key case.

Fixed #18864, fixed #18250

Thanks to Tim Graham for docs review.

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