Opened 10 years ago

Closed 9 years ago

#4289 closed (fixed)

Misbehaving Q objects

Reported by: Ben Khoo Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: Q Oracle sqlite OR qs-rf-fixed
Cc: mir@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by Matt Boersma)

Hi,

The following ticket will try to highlight a problem that I have found when using the 'OR' operator on Q objects in the boulder-oracle-sprint branch.

My model looks like this.

class Test(models.Model):
    num = models.IntegerField()

    def __str__(self):
        return 'num=%d'%self.num

The following code will highlight the bug:

In [1]: from testapp.models import *

In [2]: from django.db.models import Q

In [3]: for i in [4,8,12]:
   ...:         Test(num=i).save()
   ...:

In [4]: Test.objects.filter(num__lt=4)
Out[4]: []

In [5]: Test.objects.filter(num__gt=8, num__lt=12)
Out[5]: []

In [6]: Test.objects.filter(Q(num__lt = 4) | Q(num__gt=8, num__lt=12))
Out[6]: [<Test: num=12>, <Test: num=4>, <Test: num=8>]

In [7]: Test.objects.filter(Q(num__gt=8, num__lt=12) | Q(num__lt = 4))
Out[7]: [<Test: num=12>, <Test: num=4>, <Test: num=8>]

In [8]: Test.objects.filter(Q(num__gt=8) & Q(num__lt=12) | Q(num__lt = 4))
Out[8]: []

Lines 6 and 7 illustrate the bug.
The query appears to indicate that there are three Test objects where 'num' is less than 4 or, greater than 8 and less than 12.
Lines 4 and 5 show that no such object should exist.
Line 8 shows how I would expect the query to run.

The following is the formated (but otherwise unmodified) SQL query produced by django.

SELECT * 
FROM "TESTAPP_TEST"
WHERE (("TESTAPP_TEST"."NUM" < 4 OR 
        "TESTAPP_TEST"."NUM" > 8 OR 
        "TESTAPP_TEST"."NUM" < 12))

The issue is that the second 'OR' operator should be an AND operator. Also for the sake of safety/sanity I feel that queries specified within the Q objects should also be surrounded by a bracket.

The corrected query should read

SELECT * 
FROM "TESTAPP_TEST"
WHERE ((("TESTAPP_TEST"."NUM" < 4) OR 
        ("TESTAPP_TEST"."NUM" > 8 AND
         "TESTAPP_TEST"."NUM" < 12)))

Regards
Ben

Change History (10)

comment:1 Changed 10 years ago by Malcolm Tredinnick

This is almost certainly the same problem we have with some Q() constructions on trunk as well and is one of the motivations behind refactoring the QuerySet class.

comment:2 Changed 10 years ago by anonymous

Cc: mir@… added

comment:3 in reply to:  1 Changed 10 years ago by anonymous

Replying to mtredinnick:

This is almost certainly the same problem we have with some Q() constructions on trunk as well and is one of the motivations behind refactoring the QuerySet class.

Confirmed. I tried this against the sqlite3 backend in the trunk and got the same results.

comment:4 Changed 10 years ago by Matt Boersma

Component: UncategorizedDatabase wrapper
Description: modified (diff)
Keywords: sqlite added
Owner: changed from Jacob to Adrian Holovaty
Version: other branchSVN

comment:5 Changed 10 years ago by Matt Boersma

I retested trunk during the "worldwide sprint," and the same behavior persists against Oracle and sqlite3 backends, just FYI.

comment:6 Changed 9 years ago by Ian Kelly

Keywords: qs-rf added

comment:7 Changed 9 years ago by Malcolm Tredinnick

Summary: Misbehaving Q objects in boulder-oracle-sprint branchMisbehaving Q objects
Triage Stage: UnreviewedAccepted

comment:8 Changed 9 years ago by Malcolm Tredinnick

(In [6901]) queryset-refactor: Added a test to show that various Q() combinations work when the same field with different lookup types are combined. Refs #4289.

comment:9 Changed 9 years ago by Malcolm Tredinnick

Keywords: qs-rf-fixed added; qs-rf removed

comment:10 Changed 9 years ago by Malcolm Tredinnick

Resolution: fixed
Status: newclosed

(In [7477]) Merged the queryset-refactor branch into trunk.

This is a big internal change, but mostly backwards compatible with existing
code. Also adds a couple of new features.

Fixed #245, #1050, #1656, #1801, #2076, #2091, #2150, #2253, #2306, #2400, #2430, #2482, #2496, #2676, #2737, #2874, #2902, #2939, #3037, #3141, #3288, #3440, #3592, #3739, #4088, #4260, #4289, #4306, #4358, #4464, #4510, #4858, #5012, #5020, #5261, #5295, #5321, #5324, #5325, #5555, #5707, #5796, #5817, #5987, #6018, #6074, #6088, #6154, #6177, #6180, #6203, #6658

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