From 0e3704963693ddfaaa525054c42333f6ffdb4b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Mon, 28 Apr 2014 15:27:36 +0300 Subject: [PATCH] [1.6.x] Fixed #22429 -- Incorrect SQL when using ~Q and F Backpatch of 5e1f4656b98816c96a1cc051224c1b699db480e0 from master. Conflicts: django/db/models/sql/query.py tests/queries/models.py tests/queries/tests.py --- django/db/models/sql/query.py | 28 +++++++++++++++++++--------- docs/releases/1.6.5.txt | 3 +++ tests/queries/models.py | 15 +++++++++++++++ tests/queries/tests.py | 18 +++++++++++++++++- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 7868c19ab8e..cbdf1bc207e 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -780,6 +780,9 @@ class Query(object): if lhs in change_map: data = data._replace(lhs_alias=change_map[lhs]) self.alias_map[alias] = data + # 4. Update the temporary _lookup_joins list + if hasattr(self, '_lookup_joins'): + self._lookup_joins = [change_map.get(lj, lj) for lj in self._lookup_joins] def bump_prefix(self, exceptions=()): """ @@ -1100,6 +1103,9 @@ class Query(object): allow_explicit_fk=True) if can_reuse is not None: can_reuse.update(join_list) + # split_exclude() needs to know which joins were generated for the + # lookup parts + self._lookup_joins = join_list except MultiJoin as e: return self.split_exclude(filter_expr, LOOKUP_SEP.join(parts[:e.level]), can_reuse, e.names_with_path) @@ -1822,17 +1828,21 @@ class Query(object): for _, paths in names_with_path: all_paths.extend(paths) contains_louter = False - for pos, path in enumerate(all_paths): + # Trim and operate only on tables that were generated for + # the lookup part of the query. That is, avoid trimming + # joins generated for F() expressions. + lookup_tables = [t for t in self.tables if t in self._lookup_joins or t == self.tables[0]] + for trimmed_paths, path in enumerate(all_paths): if path.m2m: break - if self.alias_map[self.tables[pos + 1]].join_type == self.LOUTER: + if self.alias_map[lookup_tables[trimmed_paths + 1]].join_type == self.LOUTER: contains_louter = True - self.unref_alias(self.tables[pos]) + self.unref_alias(lookup_tables[trimmed_paths]) # The path.join_field is a Rel, lets get the other side's field join_field = path.join_field.field # Build the filter prefix. + paths_in_prefix = trimmed_paths trimmed_prefix = [] - paths_in_prefix = pos for name, path in names_with_path: if paths_in_prefix - len(path) < 0: break @@ -1844,12 +1854,12 @@ class Query(object): # Lets still see if we can trim the first join from the inner query # (that is, self). We can't do this for LEFT JOINs because we would # miss those rows that have nothing on the outer side. - if self.alias_map[self.tables[pos + 1]].join_type != self.LOUTER: + if self.alias_map[lookup_tables[trimmed_paths + 1]].join_type != self.LOUTER: select_fields = [r[0] for r in join_field.related_fields] - select_alias = self.tables[pos + 1] - self.unref_alias(self.tables[pos]) + select_alias = lookup_tables[trimmed_paths + 1] + self.unref_alias(lookup_tables[trimmed_paths]) extra_restriction = join_field.get_extra_restriction( - self.where_class, None, self.tables[pos + 1]) + self.where_class, None, lookup_tables[trimmed_paths + 1]) if extra_restriction: self.where.add(extra_restriction, AND) else: @@ -1857,7 +1867,7 @@ class Query(object): # inner query if it happens to have a longer join chain containing the # values in select_fields. Lets punt this one for now. select_fields = [r[1] for r in join_field.related_fields] - select_alias = self.tables[pos] + select_alias = lookup_tables[trimmed_paths] self.select = [SelectInfo((select_alias, f.column), f) for f in select_fields] return trimmed_prefix, contains_louter diff --git a/docs/releases/1.6.5.txt b/docs/releases/1.6.5.txt index 3c46b953b73..891a6880307 100644 --- a/docs/releases/1.6.5.txt +++ b/docs/releases/1.6.5.txt @@ -14,3 +14,6 @@ Bugfixes * Fixed ``pgettext_lazy`` crash when receiving bytestring content on Python 2 (`#22565 `_). + +* Fixed the SQL generated when filtering by a negated ``Q`` object that contains + a ``F`` object. (`#22429 `_). diff --git a/tests/queries/models.py b/tests/queries/models.py index 6d9e35d0bfe..431d3c05018 100644 --- a/tests/queries/models.py +++ b/tests/queries/models.py @@ -536,3 +536,18 @@ class Ticket21203Parent(models.Model): class Ticket21203Child(models.Model): childid = models.AutoField(primary_key=True) parent = models.ForeignKey(Ticket21203Parent) + + +# Bug #22429 + +class School(models.Model): + pass + + +class Student(models.Model): + school = models.ForeignKey(School) + + +class Classroom(models.Model): + school = models.ForeignKey(School) + students = models.ManyToManyField(Student, related_name='classroom') diff --git a/tests/queries/tests.py b/tests/queries/tests.py index e38d573e804..833f65f7386 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -27,7 +27,8 @@ from .models import ( ModelA, ModelB, ModelC, ModelD, Responsibility, Job, JobResponsibilities, BaseA, FK1, Identifier, Program, Channel, Page, Paragraph, Chapter, Book, MyObject, Order, OrderItem, Task, Staff, StaffUser, Ticket21203Parent, - Ticket21203Child) + Ticket21203Child, Classroom, School, Student) + class BaseQuerysetTest(TestCase): def assertValueQuerysetEqual(self, qs, values): @@ -3105,3 +3106,18 @@ class ForeignKeyToBaseExcludeTests(TestCase): SpecialCategory.objects.filter(categoryitem__id=c1.pk), [sc1], lambda x: x ) + + +class Ticket22429Tests(TestCase): + def test_ticket_22429(self): + sc1 = School.objects.create() + st1 = Student.objects.create(school=sc1) + + sc2 = School.objects.create() + st2 = Student.objects.create(school=sc2) + + cr = Classroom.objects.create(school=sc1) + cr.students.add(st1) + + queryset = Student.objects.filter(~Q(classroom__school=F('school'))) + self.assertQuerysetEqual(queryset, [st2], lambda x: x)