Opened 10 years ago
Closed 9 years ago
#24636 closed Bug (fixed)
Add validator to DecimalField to ensure that it respects max_digits/decimal_places
Reported by: | Marc Aymerich | Owned by: | Simon Charette |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | InvalidOperation decimal Decimal |
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 get an InvalidOperation exception when trying to save a particular decimal field.
Traceback: File "/usr/local/lib/python3.4/dist-packages/django/core/handlers/base.py" in get_response 132. response = wrapped_callback(request, *callback_args, **callback_kwargs) File "/usr/local/lib/python3.4/dist-packages/django/contrib/admin/options.py" in wrapper 616. return self.admin_site.admin_view(view)(*args, **kwargs) File "/usr/local/lib/python3.4/dist-packages/django/utils/decorators.py" in _wrapped_view 110. response = view_func(request, *args, **kwargs) File "/usr/local/lib/python3.4/dist-packages/django/views/decorators/cache.py" in _wrapped_view_func 57. response = view_func(request, *args, **kwargs) File "/usr/local/lib/python3.4/dist-packages/django/contrib/admin/sites.py" in inner 233. return view(request, *args, **kwargs) File "/home/orchestra/django-orchestra/orchestra/contrib/accounts/admin.py" in changelist_view 286. extra_context=context) File "/usr/local/lib/python3.4/dist-packages/django/utils/decorators.py" in _wrapper 34. return bound_func(*args, **kwargs) File "/usr/local/lib/python3.4/dist-packages/django/utils/decorators.py" in _wrapped_view 110. response = view_func(request, *args, **kwargs) File "/usr/local/lib/python3.4/dist-packages/django/utils/decorators.py" in bound_func 30. return func.__get__(self, type(self))(*args2, **kwargs2) File "/usr/local/lib/python3.4/dist-packages/django/contrib/admin/options.py" in changelist_view 1590. response = self.response_action(request, queryset=cl.get_queryset(request)) File "/usr/local/lib/python3.4/dist-packages/django/contrib/admin/options.py" in response_action 1333. response = func(self, request, queryset) File "/home/orchestra/django-orchestra/orchestra/contrib/orders/actions.py" in __call__ 32. return self.set_options(request) File "/home/orchestra/django-orchestra/orchestra/contrib/orders/actions.py" in set_options 48. return self.confirmation(request) File "/usr/lib/python3.4/contextlib.py" in inner 30. return func(*args, **kwds) File "/home/orchestra/django-orchestra/orchestra/contrib/orders/actions.py" in confirmation 80. bills = self.queryset.bill(commit=True, **self.options) File "/home/orchestra/django-orchestra/orchestra/contrib/orders/models.py" in bill 44. bills += bill_backend.create_bills(account, bill_lines, **options) File "/home/orchestra/django-orchestra/orchestra/contrib/orders/billing.py" in create_bills 45. order_billed_until=line.order.old_billed_until File "/usr/local/lib/python3.4/dist-packages/django/db/models/fields/related.py" in create 750. return super(RelatedManager, self.db_manager(db)).create(**kwargs) File "/usr/local/lib/python3.4/dist-packages/django/db/models/manager.py" in manager_method 127. return getattr(self.get_queryset(), name)(*args, **kwargs) File "/usr/local/lib/python3.4/dist-packages/django/db/models/query.py" in create 348. obj.save(force_insert=True, using=self.db) File "/home/orchestra/django-orchestra/orchestra/contrib/bills/models.py" in save 321. super(BillLine, self).save(*args, **kwargs) File "/usr/local/lib/python3.4/dist-packages/django/db/models/base.py" in save 710. force_update=force_update, update_fields=update_fields) File "/usr/local/lib/python3.4/dist-packages/django/db/models/base.py" in save_base 738. updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields) File "/usr/local/lib/python3.4/dist-packages/django/db/models/base.py" in _save_table 822. result = self._do_insert(cls._base_manager, using, fields, update_pk, raw) File "/usr/local/lib/python3.4/dist-packages/django/db/models/base.py" in _do_insert 861. using=using, raw=raw) File "/usr/local/lib/python3.4/dist-packages/django/db/models/manager.py" in manager_method 127. return getattr(self.get_queryset(), name)(*args, **kwargs) File "/usr/local/lib/python3.4/dist-packages/django/db/models/query.py" in _insert 920. return query.get_compiler(using=using).execute_sql(return_id) File "/usr/local/lib/python3.4/dist-packages/django/db/models/sql/compiler.py" in execute_sql 962. for sql, params in self.as_sql(): File "/usr/local/lib/python3.4/dist-packages/django/db/models/sql/compiler.py" in as_sql 920. for obj in self.query.objs File "/usr/local/lib/python3.4/dist-packages/django/db/models/sql/compiler.py" in <listcomp> 920. for obj in self.query.objs File "/usr/local/lib/python3.4/dist-packages/django/db/models/sql/compiler.py" in <listcomp> 918. ) for f in fields File "/usr/local/lib/python3.4/dist-packages/django/db/models/fields/__init__.py" in get_db_prep_save 1627. self.max_digits, self.decimal_places) File "/usr/local/lib/python3.4/dist-packages/django/db/backends/base/operations.py" in value_to_db_decimal 477. return utils.format_number(value, max_digits, decimal_places) File "/usr/local/lib/python3.4/dist-packages/django/db/backends/utils.py" in format_number 200. value = value.quantize(decimal.Decimal(".1") ** decimal_places, context=context) Exception Type: InvalidOperation at /admin/orders/order/ Exception Value: [<class 'decimal.InvalidOperation'>]
decimal_places 2 context Context(prec=2, rounding=ROUND_HALF_EVEN, Emin=-999999, Emax=999999, capitals=1, clamp=0, flags=[Inexact, InvalidOperation, Rounded], traps=[InvalidOperation, DivisionByZero, Overflow]) value Decimal('21') max_digits 2
Django Version: 1.8.1 Python Version: 3.4.2
Attachments (1)
Change History (16)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
class InvalidOperationModel(models.Model): value = models.DecimalField(max_digits=2, decimal_places=2)
>>> a = InvalidOperationModel(value=1) >>> a.full_clean() >>> a.save() Traceback (most recent call last): File "<console>", line 1, in <module> File "/usr/local/lib/python3.4/dist-packages/django/db/models/base.py", line 710, in save force_update=force_update, update_fields=update_fields) File "/usr/local/lib/python3.4/dist-packages/django/db/models/base.py", line 738, in save_base updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields) File "/usr/local/lib/python3.4/dist-packages/django/db/models/base.py", line 822, in _save_table result = self._do_insert(cls._base_manager, using, fields, update_pk, raw) File "/usr/local/lib/python3.4/dist-packages/django/db/models/base.py", line 861, in _do_insert using=using, raw=raw) File "/usr/local/lib/python3.4/dist-packages/django/db/models/manager.py", line 127, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) File "/usr/local/lib/python3.4/dist-packages/django/db/models/query.py", line 920, in _insert return query.get_compiler(using=using).execute_sql(return_id) File "/usr/local/lib/python3.4/dist-packages/django/db/models/sql/compiler.py", line 962, in execute_sql for sql, params in self.as_sql(): File "/usr/local/lib/python3.4/dist-packages/django/db/models/sql/compiler.py", line 920, in as_sql for obj in self.query.objs File "/usr/local/lib/python3.4/dist-packages/django/db/models/sql/compiler.py", line 920, in <listcomp> for obj in self.query.objs File "/usr/local/lib/python3.4/dist-packages/django/db/models/sql/compiler.py", line 918, in <listcomp> ) for f in fields File "/usr/local/lib/python3.4/dist-packages/django/db/models/fields/__init__.py", line 1627, in get_db_prep_save self.max_digits, self.decimal_places) File "/usr/local/lib/python3.4/dist-packages/django/db/backends/base/operations.py", line 477, in value_to_db_decimal return utils.format_number(value, max_digits, decimal_places) File "/usr/local/lib/python3.4/dist-packages/django/db/backends/utils.py", line 200, in format_number value = value.quantize(decimal.Decimal(".1") ** decimal_places, context=context) decimal.InvalidOperation: [<class 'decimal.InvalidOperation'>]
with max_digits=3
works for value=1
, but fails for value=11
.
the documentation says that max_digits
must be greater than or equal to decimal_places.
https://docs.djangoproject.com/en/dev/ref/models/fields/#django.db.models.DecimalField.max_digits
I believe this could be a regression introduced in 1.8, since my code appeared to work in 1.7.
EDIT: At least a ValidationError should be raised when doing a field clean().
comment:3 by , 10 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
This is not actually a bug.
value=1
with 2 decimals will be saved as 1.00
, meaning it exceeds the maximum number of digits (you actually have 3 digits). In general, if max_digits == decimal_places
you can only save subunitary numbers, with a precision of decimal_places
digits.
Similarly, max_digits=3
and decimal_places=1
will work for numbers up to 9.99
, and will fail for value=11
.
comment:4 by , 10 years ago
Resolution: | invalid |
---|---|
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
I didn't see the any reasoning why a ValidationError
shouldn't be raised? Attached is the patch I used to reproduce the error.
by , 10 years ago
Attachment: | 24636-test.diff added |
---|
comment:5 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 10 years ago
I have added a validator to check that the input is less than 10 ** (max_digits - decimal_places)
Here is the pull request: https://github.com/django/django/pull/4514
comment:7 by , 10 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
comment:8 by , 10 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Version: | 1.8 → master |
comment:9 by , 10 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
comment:10 by , 9 years ago
Summary: | InvalidOperation saving decimal.Decimal → Add validator to DecimalField to ensure that it respects max_digits/decimal_places |
---|
comment:13 by , 9 years ago
Owner: | changed from | to
---|---|
Patch needs improvement: | unset |
I made the changes required based on your feedback. Unfortunately the comments were lost since they were attached to a fixed up commit. They can be found here.
comment:14 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
I think we will need some more details. In particular, it's not clear if the problem is in Django or in your own code. Could you please write a test for Django's test suite that reproduces the error or tell us how we can reproduce it?