#33954 closed Cleanup/optimization (fixed)
NaN can be stored in DecimalField but cannot be retrieved — at Version 3
Reported by: | Xabier Bello | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 4.1 |
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 (last modified by )
Same as ticket https://code.djangoproject.com/ticket/33033, but I managed to trigger it anyway:
Steps to reproduce
- Create a brand new project using python 3.10 and django 4.1 with the default sqlite3 backend.
- Create a model with a DecimalField:
class MyModel(models.Model): value = models.DecimalField(max_digits=10, decimal_places=5)
- Programmatically create a model instance with value="nan",
obj = MyModel.objects.create(value="nan") obj.save()
- Then try to retrieve the object from the database (or refresh from database):
MyModel.objects.get(pk=1)
Traceback
Traceback (most recent call last): File "/sandbox/dj/bug/dec/views.py", line 9, in <module> MyModel.objects.get(pk=1) File "/lib64/python3.10/site-packages/django/db/models/manager.py", line 85, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) File "/lib64/python3.10/site-packages/django/db/models/query.py", line 646, in get num = len(clone) File "/lib64/python3.10/site-packages/django/db/models/query.py", line 376, in __len__ self._fetch_all() File "/lib64/python3.10/site-packages/django/db/models/query.py", line 1866, in _fetch_all self._result_cache = list(self._iterable_class(self)) File "/lib64/python3.10/site-packages/django/db/models/query.py", line 117, in __iter__ for row in compiler.results_iter(results): File "/lib64/python3.10/site-packages/django/db/models/sql/compiler.py", line 1333, in apply_converters value = converter(value, expression, connection) File "/lib64/python3.10/site-packages/django/db/backends/sqlite3/operations.py", line 344, in converter return create_decimal(value).quantize( TypeError: argument must be int or float
The value "nan" (and maybe "inf" also) skip the validation in DecimalField.to_python
, because is not None
, and is not instance of float. But decimal.Decimal("nan")
works without triggering the exception, so NaN
gets stored in the DB.
Change History (3)
follow-up: 2 comment:1 by , 2 years ago
follow-up: 3 comment:2 by , 2 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
Replying to Claude Paroz:
Django offers both model et form validation. If you insert values without using either one, you are on your own and responsible for what you insert in the database. I dont' think Django should do more here. To be confirmed.
Yes, we have both model and form validation for NaN
s. I don't think we should do more than that.
comment:3 by , 2 years ago
Description: | modified (diff) |
---|
Replying to Mariusz Felisiak:
Replying to Claude Paroz:
Django offers both model et form validation. If you insert values without using either one, you are on your own and responsible for what you insert in the database. I dont' think Django should do more here. To be confirmed.
Yes, we have both model and form validation for
NaN
s. I don't think we should do more than that.
What made me report as a bug is that in db.models.fields.DecimalField
, method to_python
, there are two shields against invalid values, one of them explicit against nan
. Both "nan" and "inf" can be weeded out early if the check is if not math.isfinite
instead of if math.isnan
, as it's actually implemented in the form validation. Currently float("inf")
fails to store in the DB as "Infinite" because it can't be quantize
'd at db/backends/utils.py::format_number
(decimal.InvalidOperation
), while "nan" can.
Notice that I'm using the Model validation, if I'm not mistaken and it refers to creating objects with MyModel.objects.create
. E.g. this works and happily stores "NaN" in the DB:
obj = MyModel.objects.create(value="nan")
But this fails with a ValidationError
raised from DecimalField.to_python
:
obj = MyModel.objects.create(value="invalid")
django.core.exceptions.ValidationError: ['“invalid” value must be a decimal number.']
And this fails with decimal.InvalidOperation
:
obj = MyModel.objects.create(value="inf")
decimal.InvalidOperation: [<class 'decimal.InvalidOperation'>]
IMHO it would be better interface to fail before saving, than to store the value and then fail on retrieving. The form validation does this, creating a valid Decimal
even if the value entered are the strings "nan" or "inf" (in to_python()
), and then check that only Decimal.is_finite()
are valid values (in validate()
).
This would be my refactor to DecimalField.to_python
, without adding any code, to consistently raise a ValidationError for all "inf", "nan" and invalid inputs, instead of one for each.
1703 def to_python(self, value): 1704 if value is None: 1705 return value 1706 if isinstance(value, float): 1707 result = self.context.create_decimal_from_float(value) 1708 try: 1709 result = decimal.Decimal(value) 1710 except (decimal.InvalidOperation, TypeError, ValueError): # Catches everything except "nan" and "inf" 1711 raise exceptions.ValidationError( 1712 self.error_messages["invalid"], 1713 code="invalid", 1714 params={"value": value}, 1715 ) 1716 if not result.is_finite(): # Catches both "nan" and "inf" 1717 raise exceptions.ValidationError( 1718 self.error_messages["invalid"], 1719 code="invalid", 1720 params={"value": value}, 1721 ) 1722 return result
Thanks for your time!
Django offers both model et form validation. If you insert values without using either one, you are on your own and responsible for what you insert in the database. I dont' think Django should do more here. To be confirmed.