Changes between Version 2 and Version 3 of Ticket #36259, comment 14


Ignore:
Timestamp:
Oct 20, 2026, 1:42:37 PM (3 weeks ago)
Author:
Alin Morosanu

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #36259, comment 14

    v2 v3  
    1 I’m investigating #36259, which reports that "''**Django’s unsaved-object checks can be bypassed when an unsaved instance already has a primary key**''".
     1I’m investigating #36259 and would appreciate clarification of the intended behavior before preparing a new PR.
    22
    3 The original proposed solution was to use `_state.adding` to detect unsaved instances. However, in [https://github.com/django/django/pull/19275 PR #19275], Jacob Walls identified existing workflows that stricter checks would reject:
     3The ticket description and comment:6 identify four cases involving an unsaved instance that already has a manually assigned or default-generated primary key. Against unchanged Django at commit {{{d430084791}}}, using Python 3.15 and SQLite, I observed:
    44
    5 - Querying related objects through a new UUID-backed instance, such as `pizza.toppings.all()`.
    6 - Saving interdependent objects within a transaction where foreign-key constraints are checked after both objects have been saved.
     5 1. '''Saving a relationship:''' the report expects an early {{{ValueError}}} when saving an object that references the unsaved instance. The reproduction instead reaches an {{{IntegrityError}}} when the foreign-key constraint is checked.
     6 2. '''Reading a generated field:''' the report expects the unsaved-field {{{AttributeError}}}. Reading an unloaded generated field instead raises {{{DoesNotExist}}} when the assigned primary key has no matching row.
     7 3. '''QuerySet membership:''' the report expects {{{contains()}}} to raise {{{ValueError}}}. It currently checks membership by primary key, returning {{{False}}} when no matching row exists.
     8 4. '''Related filtering:''' the report expects {{{filter()}}} and {{{exclude()}}} to reject the unsaved instance with {{{ValueError}}}. They currently accept the primary-key-based lookup.
    79
     10In [https://github.com/django/django/pull/19275 PR #19275], Jacob Walls identified valid workflows that stricter {{{_state.adding}}} checks would reject: querying a new UUID-backed instance’s related manager, and saving interdependent objects within a transaction whose foreign-key constraints are checked after both rows exist.
    811
    9 I’ve investigated the four cases described in this ticket. On the tested checkout, with `SQLite`:
     12I have prepared an uncommitted local prototype addressing only case 2. It allows the normal generated-field fetch, preserving successful retrieval when the primary key identifies an existing row. If the fetch raises the instance model’s {{{DoesNotExist}}} and the instance is marked as adding, it raises the existing unsaved-field {{{AttributeError}}} instead. Cases 1, 3, and 4 remain unchanged.
    1013
    11 - Saving with a missing referenced row raises `IntegrityError` when the constraint is checked.
    12 - `contains()` returns `False` for an assigned PK with no matching row.
    13 - Related `filter()` and `exclude()` queries execute without an unsaved-object error.
    14 - Reading an unloaded generated field with an assigned PK but no matching row raises `DoesNotExist`.
     14Seven regression cases fail against the unchanged runtime and pass with this change. All 71 generated-field tests pass with the prototype on SQLite.
    1515
    16 I have an uncommitted prototype addressing only the generated-field case. It allows the normal lookup, preserving successful retrieval when the PK identifies an existing row. If that lookup raises the model’s `DoesNotExist` and the instance is marked as adding, it raises the existing unsaved-field `AttributeError` instead. It does not change saves, membership checks, or related filters.
     16Could a maintainer clarify whether the existing primary-key-based behavior should be preserved for cases 1, 3, and 4, given those compatibility examples?
    1717
    18 Seven regression cases fail against the unchanged runtime and pass with this change.
    19 
    20 Could a maintainer clarify whether the existing save and query behaviors should be preserved? Would the generated-field exception change be suitable for a separate, narrowly scoped PR, or should a broader behaviour change and possible deprecation be discussed first?
     18Would a separate PR addressing case 2 be an appropriate scope, or should the broader behavior and any required deprecation path be settled first? The prototype also changes an observable exception type, so I would appreciate guidance on that distinction.
Back to Top