#22210 closed Bug (fixed)
Saving Model instance to Float Field
Reported by: | Owned by: | anonymous | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.6 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I came across what I believe to be an undesired behavior.
If we have a model such as:
class Person(models.Model): first_name = models.CharField(max_length=30) last_name = models.CharField(max_length=30) age = models.FloatField()
Then try to do this:
obj = Person.objects.get(pk=1) obj.age = obj obj.save()
It will succeed with out throwing any errors. If we then get that same instance back from the database the age of that instance will be one which is the primary key of the object.
This is clearly a corner case but I think it would be worth hardening django to not allow this behavior to happen. I thought that the get_prep_value() method would have been called which would have thrown a TypeError since you can not cast a model instance to a float. It turns out that this does not happen because django.db.models.sql.compiler.SQLUpdateCompiler.as_sql checks if the value of a field has the attribute 'prepare_database_save' which a model instance does it uses that instead of field.get_db_prep_save.
Change History (7)
comment:1 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Hi,
Thanks for the information.
I have managed to replicate the bug on a unit test and I have written a patch for it. The patch will be linked here soon. As it's my first patch, please let me know if anything is incorrect.
Regards,
Dan
comment:3 by , 11 years ago
Hi,
This is my initial pull request, please let me know if there is anything I need to adjust.
https://github.com/django/django/pull/2404
Regards,
Dan
comment:4 by , 11 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
comment:5 by , 11 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:6 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Agreed that validation here could be tightened; it's an edge case, but a potentially confusing one if you do it by accident.