Opened 9 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)

24636-test.diff (1.1 KB ) - added by Tim Graham 9 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 by Tim Graham, 9 years ago

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?

comment:2 by Marc Aymerich, 9 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().

Last edited 9 years ago by Marc Aymerich (previous) (diff)

comment:3 by iuliachiriac, 9 years ago

Resolution: invalid
Status: newclosed

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 Tim Graham, 9 years ago

Resolution: invalid
Status: closednew
Triage Stage: UnreviewedAccepted

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 Tim Graham, 9 years ago

Attachment: 24636-test.diff added

comment:5 by iuliachiriac, 9 years ago

Owner: changed from nobody to iuliachiriac
Status: newassigned

comment:6 by iuliachiriac, 9 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 Claude Paroz, 9 years ago

Has patch: set
Patch needs improvement: set

comment:8 by Simon Charette, 9 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin
Version: 1.8master

comment:9 by Tim Graham, 9 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

comment:10 by Tim Graham, 9 years ago

Summary: InvalidOperation saving decimal.DecimalAdd validator to DecimalField to ensure that it respects max_digits/decimal_places

comment:11 by Simon Charette, 9 years ago

Patch needs improvement: unset

Here's a PR with an up-to-date patch.

comment:12 by Tim Graham, 9 years ago

Patch needs improvement: set

Left some comments for improvement.

comment:13 by Simon Charette, 9 years ago

Owner: changed from iuliachiriac to Simon Charette
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 Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:15 by Simon Charette <charette.s@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In 75ed590:

Fixed #24636 -- Added model field validation for decimal places and max digits.

Note: See TracTickets for help on using tickets.
Back to Top