Equality operator on django.db.models.Model not commutative
|Reported by:||freek.wiekmeijer@…||Owned by:||anonymous|
|Component:||Database layer (models, ORM)||Version:||1.3|
|Cc:||anssi.kaariainen@…||Triage Stage:||Ready for checkin|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
Description (last modified by lukeplant)
In math is is a common assumption that the equality operator should be commutative, i.e. A==B would always correspond to B==A.
In django's Model class I found non-commutative behaviour.
class Model(object): ... def __eq__(self, other): return isinstance(other, self.__class__) and self._get_pk_val() == other._get_pk_val() ...
This implementation of eq() will cause a different outcome between (A==B) and (B==A) in cases where one of the two compared objects is a derived class of the other.
class MyModel(models.Model): pass class MyDerivedModel(MyModel): pass A = MyModel() B = MyDerivedModel() (A==B) # --> A.__eq__ returns True if (a.pk == b.pk) (B==A) # --> B.__eq__ returns False
I checked how derived models are created in the database (MySQL 5.0 backend). The table for MyDerivedModel does not contain its own primary key, but a reference to the PK in MyModel instead. This means that there can never be a collision between the primary key of MyModel and the primary key of MyDerivedModel.
So it is safe to compare PKs across base and derived classes, as is currently done.
Make the type comparison between self and other symmetrical.
class Model(object): def __eq__(self, other): return (isinstance(other, self.__class__) or isinstance(self, other.__class__)) and \ (self._get_pk_val() == other._get_pk_val())
Change History (27)
comment:1 Changed 3 years ago by lukeplant
- Description modified (diff)
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Accepted
comment:2 Changed 3 years ago by teraom
- Owner changed from nobody to teraom
- Status changed from new to assigned
Changed 3 years ago by teraom
comment:16 Changed 3 years ago by anonymous
- Owner changed from teraom to anonymous
- Status changed from assigned to new