Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fixed #14511 -- bug in .exclude() query
  • Loading branch information
akaariai committed Nov 2, 2013
1 parent 7548aa8 commit bec0b2a
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 4 deletions.
15 changes: 15 additions & 0 deletions django/db/models/sql/datastructures.py
Expand Up @@ -4,6 +4,21 @@
""" """




class Col(object):
def __init__(self, alias, col):
self.alias = alias
self.col = col

def as_sql(self, qn, connection):
return '%s.%s' % (qn(self.alias), self.col), []

def prepare(self):
return self

def relabeled_clone(self, relabels):
return self.__class__(relabels.get(self.alias, self.alias), self.col)


class EmptyResultSet(Exception): class EmptyResultSet(Exception):
pass pass


Expand Down
17 changes: 15 additions & 2 deletions django/db/models/sql/query.py
Expand Up @@ -22,7 +22,7 @@
from django.db.models.sql import aggregates as base_aggregates_module from django.db.models.sql import aggregates as base_aggregates_module
from django.db.models.sql.constants import (QUERY_TERMS, ORDER_DIR, SINGLE, from django.db.models.sql.constants import (QUERY_TERMS, ORDER_DIR, SINGLE,
ORDER_PATTERN, JoinInfo, SelectInfo) ORDER_PATTERN, JoinInfo, SelectInfo)
from django.db.models.sql.datastructures import EmptyResultSet, Empty, MultiJoin from django.db.models.sql.datastructures import EmptyResultSet, Empty, MultiJoin, Col
from django.db.models.sql.expressions import SQLEvaluator from django.db.models.sql.expressions import SQLEvaluator
from django.db.models.sql.where import (WhereNode, Constraint, EverythingNode, from django.db.models.sql.where import (WhereNode, Constraint, EverythingNode,
ExtraWhere, AND, OR, EmptyWhere) ExtraWhere, AND, OR, EmptyWhere)
Expand Down Expand Up @@ -820,6 +820,9 @@ def bump_prefix(self, outer_query):
conflict. Even tables that previously had no alias will get an alias conflict. Even tables that previously had no alias will get an alias
after this call. after this call.
""" """
if self.alias_prefix != outer_query.alias_prefix:
# No clashes between self and outer query should be possible.
return
self.alias_prefix = chr(ord(self.alias_prefix) + 1) self.alias_prefix = chr(ord(self.alias_prefix) + 1)
while self.alias_prefix in self.subq_aliases: while self.alias_prefix in self.subq_aliases:
self.alias_prefix = chr(ord(self.alias_prefix) + 1) self.alias_prefix = chr(ord(self.alias_prefix) + 1)
Expand Down Expand Up @@ -1514,9 +1517,19 @@ def split_exclude(self, filter_expr, prefix, can_reuse, names_with_path):
# since we are adding a IN <subquery> clause. This prevents the # since we are adding a IN <subquery> clause. This prevents the
# database from tripping over IN (...,NULL,...) selects and returning # database from tripping over IN (...,NULL,...) selects and returning
# nothing # nothing
alias, col = query.select[0].col
if self.is_nullable(query.select[0].field): if self.is_nullable(query.select[0].field):
alias, col = query.select[0].col
query.where.add((Constraint(alias, col, query.select[0].field), 'isnull', False), AND) query.where.add((Constraint(alias, col, query.select[0].field), 'isnull', False), AND)
if alias in can_reuse:
pk = query.select[0].field.model._meta.pk
# Need to add a restriction so that outer query's filters are in effect for
# the subquery, too.
query.bump_prefix(self)
query.where.add(
(Constraint(query.select[0].col[0], pk.column, pk),
'exact', Col(alias, pk.column)),
AND
)


condition = self.build_filter( condition = self.build_filter(
('%s__in' % trimmed_prefix, query), ('%s__in' % trimmed_prefix, query),
Expand Down
19 changes: 19 additions & 0 deletions tests/queries/models.py
Expand Up @@ -544,3 +544,22 @@ class Ticket21203Parent(models.Model):
class Ticket21203Child(models.Model): class Ticket21203Child(models.Model):
childid = models.AutoField(primary_key=True) childid = models.AutoField(primary_key=True)
parent = models.ForeignKey(Ticket21203Parent) parent = models.ForeignKey(Ticket21203Parent)


class Person(models.Model):
name = models.CharField(max_length=128)


@python_2_unicode_compatible
class Company(models.Model):
name = models.CharField(max_length=128)
employees = models.ManyToManyField(Person, related_name='employers', through='Employment')

def __str__(self):
return self.name


class Employment(models.Model):
employer = models.ForeignKey(Company)
employee = models.ForeignKey(Person)
title = models.CharField(max_length=128)
39 changes: 37 additions & 2 deletions tests/queries/tests.py
Expand Up @@ -25,7 +25,8 @@
ModelA, ModelB, ModelC, ModelD, Responsibility, Job, JobResponsibilities, ModelA, ModelB, ModelC, ModelD, Responsibility, Job, JobResponsibilities,
BaseA, FK1, Identifier, Program, Channel, Page, Paragraph, Chapter, Book, BaseA, FK1, Identifier, Program, Channel, Page, Paragraph, Chapter, Book,
MyObject, Order, OrderItem, SharedConnection, Task, Staff, StaffUser, MyObject, Order, OrderItem, SharedConnection, Task, Staff, StaffUser,
CategoryRelationship, Ticket21203Parent, Ticket21203Child) CategoryRelationship, Ticket21203Parent, Ticket21203Child, Person,
Company, Employment)


class BaseQuerysetTest(TestCase): class BaseQuerysetTest(TestCase):
def assertValueQuerysetEqual(self, qs, values): def assertValueQuerysetEqual(self, qs, values):
Expand Down Expand Up @@ -2352,7 +2353,7 @@ def test_no_extra_params(self):
except TypeError: except TypeError:
self.fail("Creation of an instance of a model with only the PK field shouldn't error out after bulk insert refactoring (#17056)") self.fail("Creation of an instance of a model with only the PK field shouldn't error out after bulk insert refactoring (#17056)")


class ExcludeTest(TestCase): class ExcludeTests(TestCase):
def setUp(self): def setUp(self):
f1 = Food.objects.create(name='apples') f1 = Food.objects.create(name='apples')
Food.objects.create(name='oranges') Food.objects.create(name='oranges')
Expand All @@ -2375,6 +2376,37 @@ def test_to_field(self):
Responsibility.objects.exclude(jobs__name='Manager'), Responsibility.objects.exclude(jobs__name='Manager'),
['<Responsibility: Programming>']) ['<Responsibility: Programming>'])


def test_ticket14511(self):
alex = Person.objects.get_or_create(name='Alex')[0]
jane = Person.objects.get_or_create(name='Jane')[0]

oracle = Company.objects.get_or_create(name='Oracle')[0]
google = Company.objects.get_or_create(name='Google')[0]
microsoft = Company.objects.get_or_create(name='Microsoft')[0]
intel = Company.objects.get_or_create(name='Intel')[0]

def employ(employer, employee, title):
Employment.objects.get_or_create(employee=employee, employer=employer, title=title)

employ(oracle, alex, 'Engineer')
employ(oracle, alex, 'Developer')
employ(google, alex, 'Engineer')
employ(google, alex, 'Manager')
employ(microsoft, alex, 'Manager')
employ(intel, alex, 'Manager')

employ(microsoft, jane, 'Developer')
employ(intel, jane, 'Manager')

alex_tech_employers = alex.employers.filter(
employment__title__in=('Engineer', 'Developer')).distinct().order_by('name')
self.assertQuerysetEqual(alex_tech_employers, [google, oracle], lambda x: x)

alex_nontech_employers = alex.employers.exclude(
employment__title__in=('Engineer', 'Developer')).distinct().order_by('name')
self.assertQuerysetEqual(alex_nontech_employers, [google, intel, microsoft], lambda x: x)


class ExcludeTest17600(TestCase): class ExcludeTest17600(TestCase):
""" """
Some regressiontests for ticket #17600. Some of these likely duplicate Some regressiontests for ticket #17600. Some of these likely duplicate
Expand Down Expand Up @@ -3135,3 +3167,6 @@ def test_values_no_promotion_for_existing(self):
def test_non_nullable_fk_not_promoted(self): def test_non_nullable_fk_not_promoted(self):
qs = ObjectB.objects.values('objecta__name') qs = ObjectB.objects.values('objecta__name')
self.assertTrue(' INNER JOIN ' in str(qs.query)) self.assertTrue(' INNER JOIN ' in str(qs.query))


class ExcludeJoinTest(TestCase):

0 comments on commit bec0b2a

Please sign in to comment.