#7777: DecimalField validation ignores infinity and nan
---------------------------------------------+------------------------------
Reporter: thebitguru | Owner: thebitguru
Status: assigned | Milestone:
Component: Core framework | Version: SVN
Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Comment (by thebitguru):
Adding some useful discussion from the [http://groups.google.com/group
/django-developers/browse_thread/thread/deb3047be9de21f?hl=en mailing
list].
----
That said: the patch itself looks like a fine implementation of a given
behaviour.
What isn't clear is that the behavior implemented is correct. Malcolm
flagged this in his original comments - NaN and Inf are CPU and database-
dependent values. Postgres can't support Inf, but can support NaN; I don't
know what the situation is with other databases. Inf and NaN are both
Decimal values - unconditionally raising a validation error doesn't seem
like the right approach to me.
So - the patch is fine, as long as we accept the design it implements.
Now you need to convince us that the design is correct.
Yours,
Russ Magee %-)
----
I need to revise the patch, but I need some confirmation. I am not sure
if it is worth determining what all the different databases support. I
think optional parameters (allow_{inf,nan}=False) that let the user
specify that django should allow these two cases would be sufficient.
Note that I am suggesting setting these to False, which will break
compatibility, but I believe that this is really what the average user is
expecting anyways.
If I can get a confirmation that this behavior will be acceptable then I
will go ahead and submit an updated patch that reflects this change. This
time I will ping again in one week :)
- Farhan
----
How could it break compatibility? From my testing (i.e., running your test
cases), passing in NaN or Inf raises exceptions at the moment. At this
point, it's impossible for any patch dealing with Inf and NaN to break
backwards compatibility, because there is no working baseline behavior.
Adding a configuration doesn't always solve a problem. More often than
not, it adds a new problem. This is one of those cases.
Putting a choice in the hands of a user which can come back and bite them
is not in the style of Django. Defining a DecimalField with allow_inf=True
will cause errors if you use a Postgres database. I'm not about to commit
code that will allow you to have a valid Django configuration that throws
errors in production.
This is why a survey of the current level of support in databases is
important. If, for example, no database supports Inf, then there is no
reason to even have an allow_inf setting. If support is varied, then we
need to have a discussion about the right way to handle the variation.
There is also the issue of good API design to consider. From a practical
standpoint, I'm having difficulty thinking of a reason why allow_nan needs
to exist - why should an end user be allowed to submit as a form value
that is, a priori, known to be invalid? Essentially, you're allowing the
user to submit "divide by zero" as a valid form value - what is the use
case for this?
In summary, the process goes like this:
1. Get the API right
2. Implement it.
We're still on step 1. :-)
Yours,
Russ Magee %-)
----
OK, here is what I have gathered about the databases listed under
DATABASE_ENGINE [4].
{{{
Postgresql 8: Supports all three, +/-Inf and NaN [0]
MySQL: No support for either NaN or Inf [1]
sqlite3: No support for either NaN or Inf [2]
Oracle: Supports all three [3]
}}}
So, we have a 50/50 split. What do you say?
- Farhan
{{{
[0] http://www.postgresql.org/docs/8.2/static/datatype-numeric.html
[1] http://dev.mysql.com/doc/refman/5.4/en/numeric-types.html
[2] http://www.sqlite.org/datatype3.html
[3] http://dev.mysql.com/doc/refman/5.4/en/numeric-types.html
[4] http://docs.djangoproject.com/en/dev/ref/settings/#database-engine
}}}
----
Combine the complete absence of support in SQLite and MySQL with my
previous comment about the nonsensical nature of NaN and Inf as user
inputs, and my gut reaction is that NaN and Inf should be rejected as
invalid inputs. I'm willing to be convinced otherwise, though.
Yours,
Russ Magee %-)
--
Ticket URL: <http://code.djangoproject.com/ticket/7777#comment:10>
Django <http://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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
