Code

Opened 7 years ago

Closed 6 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 bouldersprinters)

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

Attachments (0)

Change History (10)

comment:1 follow-up: Changed 7 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 7 years ago by anonymous

  • Cc mir@… added

comment:3 in reply to: ↑ 1 Changed 7 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 7 years ago by bouldersprinters

  • Component changed from Uncategorized to Database wrapper
  • Description modified (diff)
  • Keywords sqlite added
  • Owner changed from jacob to adrian
  • Version changed from other branch to SVN

comment:5 Changed 7 years ago by mboersma

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

comment:6 Changed 6 years ago by ikelly

  • Keywords qs-rf added

comment:7 Changed 6 years ago by mtredinnick

  • Summary changed from Misbehaving Q objects in boulder-oracle-sprint branch to Misbehaving Q objects
  • Triage Stage changed from Unreviewed to Accepted

comment:8 Changed 6 years ago by mtredinnick

(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 6 years ago by mtredinnick

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

comment:10 Changed 6 years ago by mtredinnick

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

(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

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.