Unsaved related object with primary_key=True field does not raise unsaved object error

[Clifford Gama]

Since models defined with custom primary keys can have pk set before being saved to the database, this means they bypass this as if they were saved. Here is an example:

tests/one_to_one/tests.py

@@ -170 +170 @@
         with self.assertRaises(Restaurant.DoesNotExist):
             place.restaurant
 
+    def test_unsaved_object_with_supplied_primary_key(self):
+        link1 = Place.objects.create(name="Ushaka", address="Ethekwini")
+        link2 = ManualPrimaryKey(primary_key="YTREWQ", name="moonwalk")
+        msg = (
+            "save() prohibited to prevent data loss due to unsaved related object "
+            "'link2'."
+        )
+        with self.assertRaisesMessage(ValueError, msg):
+            MultiModel.objects.create(link1=link1, link2=link2, name="Unsavable")
+
     def test_reverse_relationship_cache_cascade(self):
         """
         Regression test for #9023: accessing the reverse relationship shouldn't

which results in

FE
======================================================================
ERROR: test_unsaved_object_with_supplied_primary_key (one_to_one.tests.OneToOneTests.test_unsaved_object_with_supplied_primary_key)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/clifford/code/django/django/db/backends/sqlite3/base.py", line 304, in check_constraints
    raise IntegrityError(
django.db.utils.IntegrityError: The row in table 'one_to_one_multimodel' with primary key '1' has an invalid foreign key: one_to_one_multimodel.link2_id contains a value 'YTREWQ' that does not have a corresponding value in one_to_one_manualprimarykey.primary_key.

======================================================================
FAIL: test_unsaved_object_with_supplied_primary_key (one_to_one.tests.OneToOneTests.test_unsaved_object_with_supplied_primary_key)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/clifford/code/django/tests/one_to_one/tests.py", line 180, in test_unsaved_object_with_supplied_primary_key
    with self.assertRaisesMessage(ValueError, msg):
  File "/usr/lib/python3.12/contextlib.py", line 144, in __exit__
    next(self.gen)
AssertionError: ValueError not raised

----------------------------------------------------------------------
Ran 1 test in 0.004s

FAILED (failures=1, errors=1)

And here is the culprit ​code:
This is done in a few other places. My proposal is to replace such uses cases of _is_pk_set with instance._state.adding checks.

[Clifford Gama]

​PR

[NandiniPahuja]

Hi, I’d like to work on this issue. Is it still available?

[Jacob Walls]

Take a look at the last comment: notice the ticket owner already submitted a pull request.

[Sarah Boyce]

Hi Clifford, thank you for the report!
This is also failing on 5.1, so this is not a regression introduced by 5.2.

[Clifford Gama]

Thank you, Sarah!

I should have mentioned in the report that this incorrect use of _is_pk_set() affects also three other cases:

1. Access of a generated field on an unsaved instance with a specified primary key.

   tests/model_fields/test_generatedfield.py

   @@ -180 +180 @@
    
        def test_unsaved_error(self):
            m = self.base_model(a=1, b=2)
   +        manual_pk_obj = GeneratedModelNonAutoPk(id=101, a="a")
            msg = "Cannot read a generated field from an unsaved model."
            with self.assertRaisesMessage(AttributeError, msg):
                m.field
   +        with self.assertRaisesMessage(AttributeError, msg):  # Raises integrity error instead
   +            manual_pk_obj.b
    
        def test_full_clean(self):
            m = self.base_model(a=1, b=2)

2. QuerySet.contains(unsaved_with_manual_pk) raises integrity error.

   tests/queries/test_contains.py

   @@ -1 +1 @@
    from django.test import TestCase
    
   -from .models import DumbCategory, NamedCategory, ProxyCategory
   +from .models import DateTimePK, DumbCategory, NamedCategory, ProxyCategory
    
    
    class ContainsTests(TestCase):
   @@ -13 +13 @@
            msg = "QuerySet.contains() cannot be used on unsaved objects."
            with self.assertRaisesMessage(ValueError, msg):
                DumbCategory.objects.contains(DumbCategory())
   +        with self.assertRaisesMessage(ValueError, msg):
   +            DateTimePK.objects.contains(DateTimePK())
    
        def test_obj_type(self):
            msg = "'obj' must be a model instance."

3. filter|exclude(obj=unsaved_obj_with_manual_pk)

   tests/queries/tests.py

   @@ -1982 +1982 @@
                Employment.objects.filter(employer__in=[company, Company(name="unsaved")])
            with self.assertRaisesMessage(ValueError, msg):
                StaffUser.objects.filter(staff=Staff(name="unsaved"))
   +        with self.assertRaisesMessage(ValueError, msg):
   +            ExtraInfo.objects.filter(date=DateTimePK())
    
    
    class SelectRelatedTests(TestCase):
   @@ -3360 +3362 @@
                Employment.objects.exclude(employer__in=[company, Company(name="unsaved")])
            with self.assertRaisesMessage(ValueError, msg):
                StaffUser.objects.exclude(staff=Staff(name="unsaved"))
   +        with self.assertRaisesMessage(ValueError, msg):
   +            ExtraInfo.objects.exclude(date=DateTimePK())
    
    
    class ExcludeTest17600(TestCase):

[Sarah Boyce]

Thank you for the extra details. I've checked and all of these are still failing on 5.1 so are not regressions introduced by 5.2
I also think each case is roughly covered by the title, with the GeneratedField being a special case