Code

Opened 7 years ago

Closed 8 months ago

#4287 closed Bug (fixed)

FloatField will not handle infinity values

Reported by: oBeattie (oliver@… Owned by: dlanger
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: nan, infinity, mysql, float
Cc: oliver@…, calmez, maxime.bargiel@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When saving an object with an infinity float in a FloatField, the database throws an error (in MySQL at least):

Traceback (most recent call last):
  File "<console>", line 2, in ?
  File "/Django/obeattie/../obeattie/generic/managers.py", line 104, in sync_for_location
    condition.save()
  File "/opt/local/lib/python2.4/site-packages/django/db/models/base.py", line 243, in save
    ','.join(placeholders)), db_values)
  File "/opt/local/lib/python2.4/site-packages/django/db/backends/util.py", line 12, in execute
    return self.cursor.execute(sql, params)
  File "/opt/local/lib/python2.4/site-packages/MySQLdb/cursors.py", line 163, in execute
    self.errorhandler(self, exc, value)
  File "/opt/local/lib/python2.4/site-packages/MySQLdb/connections.py", line 35, in defaulterrorhandler
    raise errorclass, errorvalue
OperationalError: (1054, "Unknown column 'inf' in 'field list'")

While I realize that while this may not be a problem with Django itself, it certainly is a flaw, as Python does support infinite floats? Would it be possible to work around this at all (without resorting to 999… values?)

Attachments (3)

4287_tests.diff (1.4 KB) - added by ikelly 7 years ago.
Test cases for storing inf, -inf, nan in a FloatField
inf-bug.patch (1.2 KB) - added by calmez 3 years ago.
inf-bug.2.patch (3.3 KB) - added by calmez 3 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 7 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Since FloatField isn't a true float field at all, rather a fixed-precision field, this is all a bit moot. We will shortly be renaming FloatField to DecimalField and probably introducing a real FloatField.

Leaving this open until FloatField is implemented (or rejected) so that we can remember to test that infinity is supported for those database backends that can do it.

Changed 7 years ago by ikelly

Test cases for storing inf, -inf, nan in a FloatField

comment:2 Changed 7 years ago by ikelly

Running the test cases against each of the backends produces these results:

  • postgresql: passes
  • postgresql_psycopg2: fails -- the insert sql appears to be incorrect
  • sqlite3: passes
  • mysql: fails -- the insert sql appears to be incorrect
  • oracle: fails

Oracle 10g includes new BINARY_FLOAT and BINARY_DOUBLE types, and the tests pass if I modify the Oracle type mapping to use BINARY_DOUBLE rather than the current DOUBLE PRECISION. However, we're trying to also support Oracle 9i, which lacks those types, so I'm a bit hesitant to go ahead and do that.

comment:3 Changed 3 years ago by lrekucki

  • Severity set to Normal
  • Type set to Bug

comment:4 Changed 3 years ago by calmez

I looked at this with django from trunk and could reproduce the bug (working in sqlite3, not working in mysql).
I had a look at mysql an found out that it is not possible to store infinity values in DOUBLE columns because "Standard SQL's FLOAT (not just MySQL's) is *not* an IEEE754 float" (1).
I patched it by using the smallest (-2147483648) and the biggest (2147483647) float value as -inf and inf.

Would it be okay to fix that bug that way?


(1) http://forums.mysql.com/read.php?39,220571,220573#msg-220573

Changed 3 years ago by calmez

comment:5 Changed 3 years ago by calmez

  • Has patch set

comment:6 Changed 3 years ago by lukeplant

  • Easy pickings unset
  • Needs tests set

As discussed on django-devs, the approach in this patch is certainly not one we would consider. We will not work around lack of support for this feature in the database backend, and certainly not with magic values.

So, given that the original bug is basically fixed for DBs that support it, the only thing left to do for this ticket, as Malcolm said, is to add tests for the DBs that do support it.

So I'm marking as 'needs tests' for that reason, and so that this ticket does not appear on the 'patches needing review' report - but not to say that the current patch would be appropriate and just needs tests.

comment:7 Changed 3 years ago by calmez

  • Cc calmez added
  • UI/UX unset

I did another patch that adds the feature to enable saving infinity values on FloatFields. After enabling this a check happens if the used database is supporting this. By default this support is disabled. I enabled it for the sqlite backend since it can store infinity values in float columns.
Now the user can decide to be able to store infinity. If the user's setup does not allow storing infinity the reaction to that will be more appropriate.

Changed 3 years ago by calmez

comment:8 Changed 2 years ago by adamnelson

As of !MySQL 5.1.24, infinite values produces a NULL result instead of +/-inf. I'm not sure if this changes the test at all and I'm presuming !MySQL < 5.1.24 isn't much supported anyway - which would be consistent with !Postgres 8.1 no longer being supported.

comment:9 Changed 14 months ago by maxime.bargiel@…

  • Cc maxime.bargiel@… added
  • Keywords nan, added

There was a similar issue with DecimalField validation (see https://code.djangoproject.com/ticket/7777) that has been fixed by simply rejecting float('inf') and float('nan') values during validation.

In Django 1.4.2, custom validation needs to be implemented by the developers to exclude NaN and Inf, otherwise the error described in this ticket will occur. Nothing leads me to believe it's different in Django 1.5. Is there a reason why FloatFields should behave differently than DecimalFields with regards to NaN/Inf? While I understand that it might be interesting to have support for NaN/Inf saved in the DB, I think it's more important to have a robust system until this feature is supported.

If necessary, I'll open a new ticket.

comment:10 Changed 8 months ago by dlanger

  • Owner changed from nobody to dlanger
  • Status changed from new to assigned

comment:11 Changed 8 months ago by dlanger

https://github.com/django/django/pull/1559 passes on 2.7 with sqlite and MySQL.

comment:12 Changed 8 months ago by dlanger

  • Needs tests unset

comment:13 Changed 8 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In cc957cb16cfdad7e6c9e97dc885fc415abbf5eaa:

Fixed #4287 -- Fixed NaN and +/- Infinity handling in FloatField

NaN, +Inf, and -Inf are no longer valid values for FloatFields.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.