Commit 5c481db2 authored by Anssi Kääriäinen's avatar Anssi Kääriäinen Committed by Tim Graham
Browse files

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.
parent 21e21c7b

django/db/models/sql/compiler.py

+1 −1
Original line number Diff line number Diff line
@@ -67,7 +67,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)

django/db/models/sql/query.py

+20 −1
Original line number Diff line number Diff line
@@ -106,6 +106,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
@@ -240,6 +244,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
@@ -303,6 +308,11 @@ class Query(object): 
            obj._setup_query()

        return obj



    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.
@@ -774,7 +784,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]
@@ -801,6 +813,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
@@ -999,6 +1014,9 @@ class Query(object): 
            value = value()

        elif hasattr(value, 'resolve_expression'):

            value = value.resolve_expression(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)
@@ -1562,6 +1580,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),

docs/releases/1.7.2.txt

+3 −0
Original line number Diff line number Diff line
@@ -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`).

tests/queries/models.py

+15 −0
Original line number Diff line number Diff line
@@ -698,3 +698,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()

tests/queries/tests.py

+38 −1
Original line number Diff line number Diff line
@@ -28,7 +28,8 @@ from .models import ( 
    JobResponsibilities, 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):
@@ -3615,3 +3616,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)