#33954: NaN can be stored in DecimalField but cannot be retrieved
-------------------------------------+-------------------------------------
Reporter: Xabier Bello | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> 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.
New description:
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.
--
Comment (by Xabier Bello):
Replying to [comment:2 Mariusz Felisiak]:
> Replying to [comment:1 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!
--
Ticket URL: <https://code.djangoproject.com/ticket/33954#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/01070182d41b4bfe-31030308-c15f-479f-8ce2-423364ebe27f-000000%40eu-central-1.amazonses.com.
