Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#12239 closed (fixed)

Float is rounded to integer on queries on integer field

Reported by: waverider 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 4 years ago.
django-bug-12239.diff (2.3 KB) - added by pmclanahan 4 years ago.
Updated previous patch with suggestions and added tests.
django-bug-12239_fixed.diff (2.4 KB) - added by pmclanahan 4 years ago.
Update of the previous patch which fixes test failures.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 4 years ago by russellm

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

comment:2 Changed 4 years ago by aroy

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

Changed 4 years ago by aroy

comment:3 Changed 4 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 follow-up: Changed 4 years ago by aroy

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

comment:5 Changed 4 years ago by Alex

  • Component changed from Core framework to Database layer (models, ORM)

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

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

comment:7 Changed 4 years ago by SmileyChris

  • 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 4 years ago by pmclanahan

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

Changed 4 years ago by pmclanahan

Updated previous patch with suggestions and added tests.

comment:9 Changed 4 years ago by pmclanahan

  • Owner changed from pmclanahan to nobody
  • Patch needs improvement unset
  • Status changed from assigned to new

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 4 years ago by kmtracey

  • Owner changed from nobody to kmtracey

comment:11 Changed 4 years ago by kmtracey

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

(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 4 years ago by kmtracey

(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 4 years ago by kmtracey

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 4 years ago by pmclanahan

Update of the previous patch which fixes test failures.

comment:14 Changed 4 years ago by pmclanahan

  • Owner changed from kmtracey to nobody
  • Status changed from reopened to new

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

comment:15 Changed 4 years ago by kmtracey

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

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

comment:16 Changed 4 years ago by kmtracey

(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 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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.