Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#7818 closed (fixed)

Merging querysets can result in wrong operator grouping

Reported by: andrewbadr Owned by: mtredinnick
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If you do (Foo.objects.filter(bar=x) | Foo.objects.filter(bar=y)) & Foo.objects.filter(baz=x) the query's WHERE clause will look like "bar=x OR bar=y AND baz=z". This is wrong because the ORs should be parenthesized. Using the |, & operators on Q objects results in the correct behavior.

Attachments (0)

Change History (8)

comment:1 Changed 6 years ago by mir

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to worksforme
  • Status changed from new to closed

I tested it like this:

 qs = (models.Foo.objects.filter(a1=1)|models.Foo.objects.filter(a2=2)) & models.Foo.objects.filter(a3=3)

Here's the resulting query which I got by putting a breakpoint into MySQLdb:

SELECT `testapp_foo`.`id`, `testapp_foo`.`a1`, `testapp_foo`.`a2`, `testapp_foo`.`a3` FROM `testapp_foo` WHERE ((`testapp_foo`.`a1` = BINARY 1  OR `testapp_foo`.`a2` = BINARY 2 ) AND `testapp_foo`.`a3` = BINARY 3 )

So the query goes out fine. If you're results are different they are repeatable, please tell us more about your setup.

comment:2 Changed 6 years ago by andrewbadr

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I remembered what this bug was about. The title is still accurate but the description is wrong.

In [12]: Post.objects.filter(Q(id=6) | Q(idlte=6)).extra(where=id > 6?)
Out[12]: []

In [13]: (Post.objects.filter(id=6) | Post.objects.filter(idlte=6)).extra(where=id > 6?)
Out[13]: [<Post: Post object>]

If you look at the query, the WHERE clauses on the first one are parenthesized correctly, but there are no parens in the second one.

comment:3 Changed 6 years ago by andrewbadr

The above was formatted incorrectly.

In [12]: Post.objects.filter(Q(id=6) | Q(id__lte=6)).extra(where=['id > 6'])
Out[12]: []

In [13]: (Post.objects.filter(id=6) | Post.objects.filter(id__lte=6)).extra(where=['id > 6'])
Out[13]: [<Post: Post object>]

comment:4 Changed 6 years ago by mtredinnick

  • milestone set to 1.0
  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 6 years ago by mtredinnick

@andrewbadr: Can you check that you see this against a recent version of Django (preferably the HEAD of trunk)? I cannot repeat it, using your example or any variations on the theme. The sort of SQL query I'm seeing for your second case is exactly as in the first case:

>>> (Number.objects.filter(Q(id=8)) | Number.objects.filter(Q(id__lte=8))).extra(where=['id > 8']).query.as_sql()
('SELECT "queries_number"."id", "queries_number"."num" FROM "queries_number" WHERE ("queries_number"."id" = %s  OR "queries_number"."id" <= %s ) AND id > 8', (8, 8))

The grouping is correct there and it returns an empty result set (that example is using the Number class from tests/regressiontests/queries/models.py. I'll leave this open for a bit to let you check out your end. Maybe a patch against that test file to show the problem might help in case I'm doing something really stupid, but I can't see what it is.

If no action in a week, I'll close it as "fixed by accident in the interim".

comment:6 Changed 6 years ago by andrewbadr

Looks fixed to me too. Sorry to waste your time.

comment:7 Changed 6 years ago by mtredinnick

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

comment:8 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 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.