Opened 4 years ago

Closed 4 years ago

#17877 closed Bug (fixed)

query.extra(where=...) lack parenthesis

Reported by: Marek Brzóska <brzoskamarek@…> Owned by: Fandekasp
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords:
Cc: brzoskamarek@…, eleather, lemaire.adrien@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no


When using some_query.extra(where="some_code OR more_code") documentation says the where-clause will be ANDed with other conditions.

I believe the ANDing part does not surround the user specified where-clause with parenthesis which in this case leads to erroneous code because AND has higher priority than OR.

Attachments (1)

t17877.diff (3.0 KB) - added by Fandekasp 4 years ago.
patch + doc + tests

Download all attachments as: .zip

Change History (8)

comment:1 Changed 4 years ago by eleather

  • Cc eleather added
  • Easy pickings set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I was able to replicate what I believe is the reported issue. Marek, if you were trying to report a broader issue than interaction between where strings in a call to extra, please comment with an extended explanation.

My steps to replicate:

  1. Setup a new Django project and app.
  2. Create a model called Thing with three CharFields, foo, bar, and baz:
class Thing(models.Model):
    foo = models.CharField(max_length=200)
    bar = models.CharField(max_length=200)
    baz = models.CharField(max_length=200)

    def __unicode__(self):
        return "%s,%s,%s" % (,, self.baz)
  1. Run ./ shell.
  2. Create some Things:
from t17877 import Thing
t = Thing(foo='a', bar='a', baz='a') # Test Case 1: should appear in queryset.
t = Thing(foo='b', bar='a', baz='a') # Test Case 2: should appear in queryset.
t = Thing(foo='a', bar='a', baz='b') # Test Case 3: should not appear in queryset, bug case.
t = Thing(foo='b', bar='a', baz='b') # Test Case 4: should not appear in queryset.
t = Thing(foo='b', bar='b', baz='a') # Test Case 5: should not appear in queryset.
t = Thing(foo='a', bar='b', baz='b') # Test Case 6: should not appear in queryset, bug case.
  1. Run this query
Thing.objects.extra(where=["foo='a' OR bar = 'a'", "baz = 'a'"])
  1. The correct result would be to see Things 1 and 2. This bug is manifested because Things 3 and 6 are also in the result.

This confirms that that query is retrieving 'foo OR (bar AND baz)' instead of '(foo OR bar) AND baz'. This would be fixable by the reporter's suggestion that each 'extra, where' string be surrounded by parentheses when being added to the sql query.

comment:2 Changed 4 years ago by Fandekasp

  • Owner changed from nobody to Fandekasp

I'm looking at it. We should look at a better example for this where case, because it might incite people who read the doc fast to start using Entry.objects.extra(where=['id IN (3, 4, 5, 20)'])
instead of Entry.objects.filter(id__in=[3, 4, 5, 20])

comment:3 Changed 4 years ago by Fandekasp

hmmm I can see 2 options:

  1. where becomes a string, and we force the user to write :
    Thing.objects.extra(where="(foo='a' OR bar = 'a') AND baz = 'a'")
  1. Add a bit of code that look for the string ' OR ' in each element of the where list, and wrap the element if the string is present:
    Thing.objects.extra(where=["foo='a' OR bar = 'a'", "baz = 'a'")

will become:

Thing.objects.extra(where=["(foo='a' OR bar = 'a')", "baz = 'a'")

in ExtraWhere.as_sql() (sql/

The first solution would be simplest, but backward-incompatible. I'll start working on the 2nd solution

comment:4 Changed 4 years ago by Fandekasp

  • Cc lemaire.adrien@… added
  • Has patch set
  • Needs tests set

Easy fix, I just wrapped each element with parenthesis.
Doc updated with eleather example.
Just realize that it needs tests, will work on it now

Changed 4 years ago by Fandekasp

patch + doc + tests

comment:5 Changed 4 years ago by Fandekasp

  • Needs tests unset

comment:6 Changed 4 years ago by julien

  • Triage Stage changed from Accepted to Ready for checkin

LGTM, thanks!

comment:7 Changed 4 years ago by julien

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

In [17880]:

Fixed #17877 -- Ensured that extra WHERE clauses get correctly ANDed when they contain OR operations. Thanks to Marek Brzóska for the report, to eleather for the test case and to Adrien Lemaire for the patch.

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