fix(mass_model_updater): read tracked fields via attribute access for skip_unchanged#15114
Open
valentijnscholten wants to merge 1 commit into
Open
Conversation
… skip_unchanged The skip-unchanged no-op check snapshotted each tracked field with model.__dict__.get(f) to avoid triggering a deferred-field load. But when a tracked field is deferred (the caller's queryset used .only()/.defer() and omitted it), __dict__ has no entry and .get() returns None. If function then recomputes that field to None, None == None marks the row unchanged and the write is silently dropped, persisting a stale value. Read the pre-function value with normal attribute access (getattr) instead. Deferred tracked fields are loaded and compared against their real persisted value, so a real change can never be mistaken for a no-op. The cost of a deferred tracked field is now a visible per-row load rather than silent data loss; callers needing the fast path must load the tracked fields in their queryset (documented). Adds regression tests: a deferred tracked field recomputed to None must issue an UPDATE and persist; one left unchanged must still skip the write.
Maffooch
approved these changes
Jun 30, 2026
dogboat
approved these changes
Jul 1, 2026
Jino-T
approved these changes
Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mass_model_updater(..., skip_unchanged=True)(added in perf(dedupe): skip unchanged rows, VALUES fast-write, prefetch vulnerability_ids #15046) snapshotted each tracked field withmodel.__dict__.get(f)before callingfunction, to avoid triggering a deferred-field load when deciding whether a row changed..only()/.defer()and omitted it —__dict__has no entry, so the snapshot readsNone. Iffunctionrecomputes that field toNone, the comparisonNone == Nonemarks the row "unchanged" and the write is silently dropped, leaving a stale persisted value..only(...)that omits the very field passed infields=). The previous__dict__approach turned that caller mistake into silent data loss instead of a visible cost.functionvalue of each tracked field with normal attribute access (getattr) instead of__dict__. A deferred tracked field is loaded and compared against its real persisted value, so a real change can never be mistaken for a no-op. The skip-unchanged optimization keeps working for correctly-loaded querysets; a deferred tracked field now degrades to a visible per-row load rather than silent incorrectness. Documented that callers needing the fast path must load the tracked fields.unittests/test_mass_model_updater.py: a deferred tracked field recomputed toNonemust issue an UPDATE and persist; a deferred tracked field left unchanged must still skip the write.skip_unchangedexists only ondev(not in released versions), so released versions are unaffected.Alternative to #15112, which guards the no-op check with
get_deferred_fields()and always writes deferred rows. This PR instead removes the__dict__indirection that caused the bug, so the no-op check stays correct (and the optimization keeps working) for deferred-but-loaded rows.