#17332: Doing a save after primary key has changed should raise an error
-------------------------------------+-------------------------------------
Reporter: akaariai | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.3
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by akaariai):
* cc: akaariai (added)
* needs_docs: => 1
* has_patch: 0 => 1
* needs_tests: => 1
* needs_better_patch: => 0
Comment:
A proof of concept patch attached.
Some notes about the implementation:
- The implementation is based on field.get_immutable_value(obj). By
default this returns the fields value as is. This is set to
obj._state.old_pk_values, and then in save we compare the new pk value to
that. If changes -> deprecation warning.
- I did some cleanup to how model._state is updated, this was required
as there was a lot of non-DRYness after my changes.
- I also did some cleanups to model.save(). I think I also found a bug
related to transaction handling, I will create another ticket about that.
- There is no compatibility flag, you can get the old behavior back by
explicitly rotating the PK value to None and then to new value. That is:
{{{
obj = qs.get(pk=1); obj.pk = 2; obj.save() -> deprecation warning
obj = qs.get(pk=1); obj.pk = None; obj.pk = 2; obj.save() -> no warning,
and will work later on.
}}}
There is no safe get_immutable_value for 3rd party fields. The idea is
that custom fields have two releases to implement this if needed. If they
fail to do this and are mutable based, then the tracking of changes will
not work. I think this approach is now pretty good, although the current
patch does not have tests. How to test `PendingDeprecationWarnings`?
The bad news about this approach is that model `__init__` is now 10-20%
slower for somewhat simple models (20% for just integer pk, 10% when there
are additional 10 integer fields). The speed difference would likely be
much smaller if the fields were large strings, but this still feels a bit
bad. I don't know how to make this any faster, except if we track just the
"main" primary key, that is, we skip tracking of parent class primary
keys.
The next problem is what to do to `ModelForms`. They would need to have an
editable field when inserting, and non-editable field when editing an
already existing model. Bad news is that `ModelForms` do not support non-
editable fields. A html attribute editable="false" + validation might
work, but isn't too pretty approach...
--
Ticket URL: <https://code.djangoproject.com/ticket/17332#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
