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

[Marek Brzóska <brzoskamarek@…>]

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.

[eleather]

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.foo, self.bar, self.baz)

3. Run ./manage.py shell.
4. Create some Things:

from t17877 import Thing
t = Thing(foo='a', bar='a', baz='a') # Test Case 1: should appear in queryset.
t.save()
t = Thing(foo='b', bar='a', baz='a') # Test Case 2: should appear in queryset.
t.save()
t = Thing(foo='a', bar='a', baz='b') # Test Case 3: should not appear in queryset, bug case.
t.save()
t = Thing(foo='b', bar='a', baz='b') # Test Case 4: should not appear in queryset.
t.save()
t = Thing(foo='b', bar='b', baz='a') # Test Case 5: should not appear in queryset.
t.save()
t = Thing(foo='a', bar='b', baz='b') # Test Case 6: should not appear in queryset, bug case.
t.save()

5. Run this query

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

6. 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.

[Adrien Lemaire]

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])

[Adrien Lemaire]

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'")
   

2. 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/where.py)

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

[Adrien Lemaire]

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

[Adrien Lemaire]

patch + doc + tests

[Julien Phalip]

LGTM, thanks!

[Julien Phalip]

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.