summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAnssi Kääriäinen <akaariai@gmail.com>2014-10-07 16:07:46 +0300
committerTim Graham <timograham@gmail.com>2014-11-20 13:53:28 -0500
commit01f2cf2aecc932d43b20b55fc19a8fa440457b5f (patch)
tree9bcbdba8451ca6bb5a1ffa8471aca328c43c510d
parent2e2607870d6201f305e56ab4117bbce2fef63f17 (diff)
[1.7.x] Fixed #23605 -- Fixed nested subquery regression
Added relabeled_clone() method to sql.Query to fix the problem. It manifested itself in rare cases where at least double nested subquery's filter condition might target non-existing alias. Thanks to Trac alias ris for reporting the problem. Backport of 5c481db29572a387651681b43d5d4523f96b3793 from master
-rw-r--r--django/db/models/sql/compiler.py2
-rw-r--r--django/db/models/sql/query.py21
-rw-r--r--docs/releases/1.7.2.txt3
-rw-r--r--tests/queries/models.py15
-rw-r--r--tests/queries/tests.py39
5 files changed, 77 insertions, 3 deletions
diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
index d8697fa06d..90ac90f9ae 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -56,7 +56,7 @@ class SQLCompiler(object):
if name in self.quote_cache:
return self.quote_cache[name]
if ((name in self.query.alias_map and name not in self.query.table_map) or
- name in self.query.extra_select):
+ name in self.query.extra_select or name in self.query.external_aliases):
self.quote_cache[name] = name
return name
r = self.connection.ops.quote_name(name)
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index c29b08556f..4c60a3b09e 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -114,6 +114,10 @@ class Query(object):
# type they are. The key is the alias of the joined table (possibly
# the table name) and the value is JoinInfo from constants.py.
self.alias_map = {}
+ # Sometimes the query contains references to aliases in outer queries (as
+ # a result of split_exclude). Correct alias quoting needs to know these
+ # aliases too.
+ self.external_aliases = set()
self.table_map = {} # Maps table names to list of aliases.
self.join_map = {}
self.default_cols = True
@@ -241,6 +245,7 @@ class Query(object):
obj.model = self.model
obj.alias_refcount = self.alias_refcount.copy()
obj.alias_map = self.alias_map.copy()
+ obj.external_aliases = self.external_aliases.copy()
obj.table_map = self.table_map.copy()
obj.join_map = self.join_map.copy()
obj.default_cols = self.default_cols
@@ -335,6 +340,11 @@ class Query(object):
# Return value depends on the type of the field being processed.
return self.convert_values(value, aggregate.field, connection)
+ def relabeled_clone(self, change_map):
+ clone = self.clone()
+ clone.change_aliases(change_map)
+ return clone
+
def get_aggregation(self, using, force_subq=False):
"""
Returns the dictionary with the values of the existing aggregations.
@@ -788,7 +798,9 @@ class Query(object):
ident = (change_map.get(ident[0], ident[0]),) + ident[1:]
self.join_map[ident] = aliases
for old_alias, new_alias in six.iteritems(change_map):
- alias_data = self.alias_map[old_alias]
+ alias_data = self.alias_map.get(old_alias)
+ if alias_data is None:
+ continue
alias_data = alias_data._replace(rhs_alias=new_alias)
self.alias_refcount[new_alias] = self.alias_refcount[old_alias]
del self.alias_refcount[old_alias]
@@ -815,6 +827,9 @@ class Query(object):
data = data._replace(lhs_alias=change_map[lhs])
self.alias_map[alias] = data
+ self.external_aliases = {change_map.get(alias, alias)
+ for alias in self.external_aliases}
+
def bump_prefix(self, outer_query):
"""
Changes the alias prefix to the next letter in the alphabet in a way
@@ -1048,6 +1063,9 @@ class Query(object):
elif isinstance(value, ExpressionNode):
# If value is a query expression, evaluate it
value = SQLEvaluator(value, self, reuse=can_reuse)
+ # Subqueries need to use a different set of aliases than the
+ # outer query. Call bump_prefix to change aliases of the inner
+ # query (the value).
if hasattr(value, 'query') and hasattr(value.query, 'bump_prefix'):
value = value._clone()
value.query.bump_prefix(self)
@@ -1509,6 +1527,7 @@ class Query(object):
lookup = lookup_class(Col(query.select[0].col[0], pk, pk),
Col(alias, pk, pk))
query.where.add(lookup, AND)
+ query.external_aliases.add(alias)
condition, needed_inner = self.build_filter(
('%s__in' % trimmed_prefix, query),
diff --git a/docs/releases/1.7.2.txt b/docs/releases/1.7.2.txt
index 8af832d1a5..cfa68b8b3a 100644
--- a/docs/releases/1.7.2.txt
+++ b/docs/releases/1.7.2.txt
@@ -71,3 +71,6 @@ Bugfixes
* Avoided unnecessary rollbacks of migrations from other apps when migrating
backwards (:ticket:`23410`).
+
+* Fixed a rare query error when using deeply nested subqueries
+ (:ticket:`23605`).
diff --git a/tests/queries/models.py b/tests/queries/models.py
index 604eed5f07..6e6a0ce5e1 100644
--- a/tests/queries/models.py
+++ b/tests/queries/models.py
@@ -675,3 +675,18 @@ class Student(models.Model):
class Classroom(models.Model):
school = models.ForeignKey(School)
students = models.ManyToManyField(Student, related_name='classroom')
+
+
+class Ticket23605A(models.Model):
+ pass
+
+
+class Ticket23605B(models.Model):
+ modela_fk = models.ForeignKey(Ticket23605A)
+ modelc_fk = models.ForeignKey("Ticket23605C")
+ field_b0 = models.IntegerField(null=True)
+ field_b1 = models.BooleanField(default=False)
+
+
+class Ticket23605C(models.Model):
+ field_c0 = models.FloatField()
diff --git a/tests/queries/tests.py b/tests/queries/tests.py
index 0fff68b21f..f6162508b9 100644
--- a/tests/queries/tests.py
+++ b/tests/queries/tests.py
@@ -27,7 +27,8 @@ from .models import (
BaseA, FK1, Identifier, Program, Channel, Page, Paragraph, Chapter, Book,
MyObject, Order, OrderItem, SharedConnection, Task, Staff, StaffUser,
CategoryRelationship, Ticket21203Parent, Ticket21203Child, Person,
- Company, Employment, CustomPk, CustomPkTag, Classroom, School, Student)
+ Company, Employment, CustomPk, CustomPkTag, Classroom, School, Student,
+ Ticket23605A, Ticket23605B, Ticket23605C)
class BaseQuerysetTest(TestCase):
@@ -3391,3 +3392,39 @@ class Ticket22429Tests(TestCase):
queryset = Student.objects.filter(~Q(classroom__school=F('school')))
self.assertQuerysetEqual(queryset, [st2], lambda x: x)
+
+
+class Ticket23605Tests(TestCase):
+ def test_ticket_23605(self):
+ # Test filtering on a complicated q-object from ticket's report.
+ # The query structure is such that we have multiple nested subqueries.
+ # The original problem was that the inner queries weren't relabeled
+ # correctly.
+ a1 = Ticket23605A.objects.create()
+ a2 = Ticket23605A.objects.create()
+ c1 = Ticket23605C.objects.create(field_c0=10000.0)
+ Ticket23605B.objects.create(
+ field_b0=10000.0, field_b1=True,
+ modelc_fk=c1, modela_fk=a1)
+ complex_q = Q(pk__in=Ticket23605A.objects.filter(
+ Q(
+ # True for a1 as field_b0 = 10000, field_c0=10000
+ # False for a2 as no ticket23605b found
+ ticket23605b__field_b0__gte=1000000 /
+ F("ticket23605b__modelc_fk__field_c0")
+ ) &
+ # True for a1 (field_b1=True)
+ Q(ticket23605b__field_b1=True) &
+ ~Q(ticket23605b__pk__in=Ticket23605B.objects.filter(
+ ~(
+ # Same filters as above commented filters, but
+ # double-negated (one for Q() above, one for
+ # parentheses). So, again a1 match, a2 not.
+ Q(field_b1=True) &
+ Q(field_b0__gte=1000000 / F("modelc_fk__field_c0"))
+ )
+ ))).filter(ticket23605b__field_b1=True))
+ qs1 = Ticket23605A.objects.filter(complex_q)
+ self.assertQuerysetEqual(qs1, [a1], lambda x: x)
+ qs2 = Ticket23605A.objects.exclude(complex_q)
+ self.assertQuerysetEqual(qs2, [a2], lambda x: x)