Opened 11 years ago
Closed 10 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 , 11 years ago
comment:2 by , 11 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 , 11 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 , 11 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 , 11 years ago
| Attachment: | 24636-test.diff added |
|---|
comment:5 by , 11 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:6 by , 11 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 , 11 years ago
| Has patch: | set |
|---|---|
| Patch needs improvement: | set |
comment:8 by , 11 years ago
| Patch needs improvement: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
| Version: | 1.8 → master |
comment:9 by , 11 years ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
comment:10 by , 10 years ago
| Summary: | InvalidOperation saving decimal.Decimal → Add validator to DecimalField to ensure that it respects max_digits/decimal_places |
|---|
comment:13 by , 10 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 , 10 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?