Code

Opened 2 years ago

Closed 8 months ago

Last modified 8 months ago

#18250 closed Bug (fixed)

Comparing two unsaved model objects

Reported by: yrdnsa@… Owned by: Yarden
Component: Database layer (models, ORM) Version: master
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

Attachments (0)

Change History (10)

comment:1 Changed 2 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Type changed from Uncategorized to Bug

comment:2 Changed 2 years ago by moritzs

  • Cc moritzs added
  • Triage Stage changed from Unreviewed to Design decision needed
  • Version changed from 1.3 to master

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 Changed 2 years ago by Alex

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

comment:4 Changed 2 years ago by claudep

  • Triage Stage changed from Design decision needed to Accepted

comment:5 Changed 2 years ago by moritzs

  • Cc moritzs removed

comment:6 Changed 2 years ago by moritzs

  • Cc moritz.sichert@… added

comment:7 Changed 2 years ago by akaariai

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 2 years ago by akaariai (previous) (diff)

comment:8 Changed 2 years ago by jcspray@…

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

comment:9 Changed 8 months ago by akaariai

  • Resolution set to duplicate
  • Status changed from new to closed

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

comment:10 Changed 8 months ago by Anssi Kääriäinen <akaariai@…>

  • Resolution changed from duplicate to fixed

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.