Opened 12 days ago

Last modified 10 days ago

#36259 assigned Bug

`_is_pk_set()` is incorrectly used to check for unsaved models. — at Initial Version

Reported by: Clifford Gama Owned by: Clifford Gama
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Csirmaz Bendegúz, Simon Charette Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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:

  • TabularUnified tests/one_to_one/tests.py

    diff --git a/tests/one_to_one/tests.py b/tests/one_to_one/tests.py
    index 451e97c274..67b1c32e90 100644
    a b class OneToOneTests(TestCase):  
    170170        with self.assertRaises(Restaurant.DoesNotExist):
    171171            place.restaurant
    172172
     173    def test_unsaved_object_with_supplied_primary_key(self):
     174        link1 = Place.objects.create(name="Ushaka", address="Ethekwini")
     175        link2 = ManualPrimaryKey(primary_key="YTREWQ", name="moonwalk")
     176        msg = (
     177            "save() prohibited to prevent data loss due to unsaved related object "
     178            "'link2'."
     179        )
     180        with self.assertRaisesMessage(ValueError, msg):
     181            MultiModel.objects.create(link1=link1, link2=link2, name="Unsavable")
     182
    173183    def test_reverse_relationship_cache_cascade(self):
    174184        """
    175185        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.added checks.

According to the ticket's flags, the next step(s) to move this issue forward are:

  • For anyone except the patch author to review the patch using the patch review checklist and either mark the ticket as "Ready for checkin" if everything looks good, or leave comments for improvement and mark the ticket as "Patch needs improvement".

Change History (0)

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