Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#12239 closed (fixed)

Float is rounded to integer on queries on integer field

Reported by: Adrian Andreias Owned by: nobody
Component: Database layer (models, ORM) Version: 1.1
Severity: Keywords: __gte, __lte, query rounding
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

On queries (gte, lte) ran on an integer field with a float value to compare, the float value gets rounded to integer and this causes invalid results.

class Number(models.Model):
	val = models.IntegerField()

# Python 2.5.4 (r254:67916, Dec 23 2008, 15:10:54) [MSC v.1310 32 bit (Intel)] on win32
# Type "help", "copyright", "credits" or "license" for more information.
# (InteractiveConsole)

>>> n = Number(val=10)
>>> n.save()
>>> Number.objects.all()
[<Number: Number object>]
>>> Number.objects.filter(val__gte=10.1)
[<Number: Number object>]
>>> from django.db import connection
>>> connection.queries[-1]
{'time': '0.001', 'sql': 'SELECT "t1_number"."id", "t1_number"."val" FROM "t1_number" WHERE "t1_number"."val" >= 10  LIMIT 21'}
>>> import django
>>> django.VERSION
(1, 1, 1, 'final', 0)

Attachments (3)

bug12239.diff (1.8 KB) - added by aroy 7 years ago.
django-bug-12239.diff (2.3 KB) - added by Paul McLanahan 7 years ago.
Updated previous patch with suggestions and added tests.
django-bug-12239_fixed.diff (2.4 KB) - added by Paul McLanahan 7 years ago.
Update of the previous patch which fixes test failures.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 7 years ago by Russell Keith-Magee

milestone: 1.2
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 7 years ago by aroy

Owner: changed from nobody to aroy
Status: newassigned

Changed 7 years ago by aroy

Attachment: bug12239.diff added

comment:3 Changed 7 years ago by aroy

Has patch: set

The bug is only for gte queries; lte already worked as expected. I've attached a patch that fixes it.

comment:4 Changed 7 years ago by aroy

Owner: changed from aroy to nobody
Status: assignednew

comment:5 Changed 7 years ago by Alex Gaynor

Component: Core frameworkDatabase layer (models, ORM)

comment:6 in reply to:  4 Changed 7 years ago by anonymous

Replying to aroy:
How about "gt" lookup? That will also floored.

comment:7 Changed 7 years ago by Chris Beaven

Patch needs improvement: set

Flooring a 'gt' is fine, because field__gt=11.1 will floor to 11, which will still match field=12.

There's a problem with 'lt' too though, because field__lt=12.1 will floor to 12 which *should* still match field=12.

I also think that the patch should still run through get_prep_value. My suggestion is to just math.ceil float & Decimal values (if 'gte' or 'lt') and passing the result through super still.

comment:8 Changed 7 years ago by Paul McLanahan

Owner: changed from nobody to Paul McLanahan
Status: newassigned

Changed 7 years ago by Paul McLanahan

Attachment: django-bug-12239.diff added

Updated previous patch with suggestions and added tests.

comment:9 Changed 7 years ago by Paul McLanahan

Owner: changed from Paul McLanahan to nobody
Patch needs improvement: unset
Status: assignednew

Updated patch with changes suggested by SmileyChris, and added tests to cover the 'lt' case he mentioned. Should be ready for checkin.

comment:10 Changed 7 years ago by Karen Tracey

Owner: changed from nobody to Karen Tracey

comment:11 Changed 7 years ago by Karen Tracey

Resolution: fixed
Status: newclosed

(In [12819]) Fixed #12239: Fixed results of gte and lt queries when comparing floats to integer field values.
Thanks waverider, aroy, SmileyChris, and pmclanahan.

comment:12 Changed 7 years ago by Karen Tracey

(In [12820]) [1.1.X] Fixed #12239: Fixed results of gte and lt queries when comparing floats to integer field values.
Thanks waverider, aroy, SmileyChris, and pmclanahan.

comment:13 Changed 7 years ago by Karen Tracey

Resolution: fixed
Status: closedreopened

Fix here broke some other tests:

======================================================================
ERROR: test_limits (regressiontests.model_fields.tests.BigIntegerFieldTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/tests/regressiontests/model_fields/tests.py", line 250, in test_limits
    self.assertEqual(qs.count(), 1)
  File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/query.py", line 324, in count
    return self.query.get_count(using=self.db)
  File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/sql/query.py", line 366, in get_count
    number = obj.get_aggregation(using=using)[None]
  File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/sql/query.py", line 338, in get_aggregation
    result = query.get_compiler(using).execute_sql(SINGLE)
  File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/sql/compiler.py", line 729, in execute_sql
    cursor.execute(sql, params)
  File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/backends/sqlite3/base.py", line 197, in execute
    return Database.Cursor.execute(self, query, params)
OverflowError: long too big to convert

======================================================================
FAIL: Doctest: modeltests.order_with_respect_to.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/test/_doctest.py", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for modeltests.order_with_respect_to.models.__test__.API_TESTS
  File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/tests/modeltests/order_with_respect_to/models.py", line unknown line number, in API_TESTS

----------------------------------------------------------------------
File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/tests/modeltests/order_with_respect_to/models.py", line ?, in modeltests.order_with_respect_to.models.__test__.API_TESTS
Failed example:
    a4.get_previous_in_order()
Exception raised:
    Traceback (most recent call last):
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/test/_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest modeltests.order_with_respect_to.models.__test__.API_TESTS[17]>", line 1, in ?
        a4.get_previous_in_order()
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/utils/functional.py", line 55, in _curried
        return _curried_func(*(args+moreargs), **dict(kwargs, **morekwargs))
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/base.py", line 660, in _get_next_or_previous_in_order
        obj = self._default_manager.filter(**{
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/query.py", line 545, in filter
        return self._filter_or_exclude(False, *args, **kwargs)
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/query.py", line 563, in _filter_or_exclude
        clone.query.add_q(Q(*args, **kwargs))
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/sql/query.py", line 1103, in add_q
        can_reuse=used_aliases)
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/sql/query.py", line 1043, in add_filter
        connector)
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/sql/where.py", line 66, in add
        value = obj.prepare(lookup_type, value)
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/sql/where.py", line 275, in prepare
        return self.field.get_prep_lookup(lookup_type, value)
      File "/home/buildbot/slave-py2.4/parts/ubuntu9.10-py2.4-sqlite/django-trunk_ubuntu9.10-py2.4-sqlite/build/django/db/models/fields/__init__.py", line 888, in get_prep_lookup
        value = math.ceil(value)
    TypeError: a float is required


----------------------------------------------------------------------

Changed 7 years ago by Paul McLanahan

Attachment: django-bug-12239_fixed.diff added

Update of the previous patch which fixes test failures.

comment:14 Changed 7 years ago by Paul McLanahan

Owner: changed from Karen Tracey to nobody
Status: reopenednew

Updated the fix to only perform the math.ceil if value is a float.

comment:15 Changed 7 years ago by Karen Tracey

Resolution: fixed
Status: newclosed

(In [12821]) Fixed #12239, again: Refined the original fix to avoid the test errors introduced. Thanks pmclanahan.

comment:16 Changed 7 years ago by Karen Tracey

(In [12822]) [1.1.X] Fixed #12239, again: Refined the original fix to avoid the test errors introduced. Thanks pmclanahan.

r12821 from trunk.

comment:17 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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