diff --git a/django/db/models/base.py b/django/db/models/base.py index 3e2ae8d4254..a5b0f188b4d 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -459,14 +459,21 @@ class Model(six.with_metaclass(ModelBase)): return '%s object' % self.__class__.__name__ def __eq__(self, other): - return (isinstance(other, Model) and - self._meta.concrete_model == other._meta.concrete_model and - self._get_pk_val() == other._get_pk_val()) + if not isinstance(other, Model): + return False + if self._meta.concrete_model != other._meta.concrete_model: + return False + my_pk = self._get_pk_val() + if my_pk is None: + return self is other + return my_pk == other._get_pk_val() def __ne__(self, other): return not self.__eq__(other) def __hash__(self): + if self._get_pk_val() is None: + raise TypeError("Model instances without primary key value are unhashable") return hash(self._get_pk_val()) def __reduce__(self): diff --git a/django/forms/models.py b/django/forms/models.py index a5b82e521d7..4c6ee9c6ed6 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -631,7 +631,11 @@ class BaseModelFormSet(BaseFormSet): seen_data = set() for form in valid_forms: # get data for each field of each of unique_check - row_data = tuple([form.cleaned_data[field] for field in unique_check if field in form.cleaned_data]) + row_data = (form.cleaned_data[field] + for field in unique_check if field in form.cleaned_data) + # Reduce Model instances to their primary key values + row_data = tuple(d._get_pk_val() if hasattr(d, '_get_pk_val') else d + for d in row_data) if row_data and not None in row_data: # if we've already seen it then we have a uniqueness failure if row_data in seen_data: diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt index 6295eb04073..da657a9a01b 100644 --- a/docs/ref/models/instances.txt +++ b/docs/ref/models/instances.txt @@ -521,6 +521,25 @@ For example:: In previous versions only instances of the exact same class and same primary key value were considered equal. +``__hash__`` +------------ + +.. method:: Model.__hash__() + +The ``__hash__`` method is based on the instance's primary key value. It +is effectively hash(obj.pk). If the instance doesn't have a primary key +value then a ``TypeError`` will be raised (otherwise the ``__hash__`` +method would return different values before and after the instance is +saved, but changing the ``__hash__`` value of an instance `is forbidden +in Python`_). + +.. versionchanged:: 1.7 + + In previous versions instance's without primary key value were + hashable. + +.. _is forbidden in Python: http://docs.python.org/reference/datamodel.html#object.__hash__ + ``get_absolute_url`` -------------------- diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index 774d9d31611..6480a2505f2 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -199,6 +199,14 @@ Miscellaneous equal when primary keys match. Previously only instances of exact same class were considered equal on primary key match. +* The :meth:`django.db.models.Model.__eq__` method has changed such that + two ``Model`` instances without primary key values won't be considered + equal (unless they are the same instance). + +* The :meth:`django.db.models.Model.__hash__` will now raise ``TypeError`` + when called on an instance without a primary key value. This is done to + avoid mutable ``__hash__`` values in containers. + Features deprecated in 1.7 ========================== diff --git a/tests/basic/tests.py b/tests/basic/tests.py index 2b051621ef4..611944902ac 100644 --- a/tests/basic/tests.py +++ b/tests/basic/tests.py @@ -708,9 +708,20 @@ class ModelTest(TestCase): SelfRef.objects.get(selfref=sr) def test_eq(self): + self.assertEqual(Article(id=1), Article(id=1)) self.assertNotEqual(Article(id=1), object()) self.assertNotEqual(object(), Article(id=1)) + a = Article() + self.assertEqual(a, a) + self.assertNotEqual(Article(), a) + def test_hash(self): + # Value based on PK + self.assertEqual(hash(Article(id=1)), hash(1)) + with self.assertRaises(TypeError): + # No PK value -> unhashable (because save() would then change + # hash) + hash(Article()) class ConcurrentSaveTests(TransactionTestCase):