Database identifiers are not properly escaped in some queries

[Nikolay Kurevin]

Happens on 1.7.2-1.7.7, does not happens on 1.7.1

Rough setup:

class Bmodel(models.Model):
    is_active = models.BooleanField()

class Amodel(models.Model):
    active = models.BooleanField()
    bmodel = models.ForeignKey(Bmodel, related_name='Amodel_bmodel')

class Cmodel(models.Model):
    amodel = models.ForeignKey(Amodel)

Depending on order of Q filters there are 2 outcomes:

>>> models.Amodel.objects.exclude(Q(active=False, bmodel__is_active=False) & Q(cmodel__isnull=True)).query.__str__()

u'SELECT (...cut...) FROM "coreApi_amodel" INNER JOIN "coreApi_bmodel" ON ( "coreApi_amodel"."bmodel_id" = "coreApi_bmodel"."id" ) WHERE NOT ("coreApi_amodel"."active" = False AND "coreApi_bmodel"."is_active" = False AND "coreApi_amodel"."id" IN (SELECT U0."id" AS "id" FROM coreApi_amodel U0 LEFT OUTER JOIN "coreApi_cmodel" U1 ON ( U0."id" = U1."amodel_id" ) WHERE (U1."id" IS NULL AND U0."id" = (coreApi_amodel."id"))))'

>>> models.Amodel.objects.exclude(Q(active=False, bmodel__is_active=False) & Q(cmodel__isnull=True))
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File ".../.venv/src/django/django/db/models/query.py", line 116, in __repr__
    data = list(self[:REPR_OUTPUT_SIZE + 1])
  File ".../.venv/src/django/django/db/models/query.py", line 141, in __iter__
    self._fetch_all()
  File ".../.venv/src/django/django/db/models/query.py", line 966, in _fetch_all
    self._result_cache = list(self.iterator())
  File ".../.venv/src/django/django/db/models/query.py", line 265, in iterator
    for row in compiler.results_iter():
  File ".../.venv/src/django/django/db/models/sql/compiler.py", line 700, in results_iter
    for rows in self.execute_sql(MULTI):
  File ".../.venv/src/django/django/db/models/sql/compiler.py", line 786, in execute_sql
    cursor.execute(sql, params)
  File ".../.venv/src/django/django/db/backends/utils.py", line 81, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File ".../.venv/src/django/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File ".../.venv/src/django/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File ".../.venv/src/django/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
ProgrammingError: relation "coreapi_amodel" does not exist
LINE 1: ...pi_amodel"."id" IN (SELECT U0."id" AS "id" FROM coreApi_am...
                                                             ^

Change Q order:

>>> models.Amodels.objects.exclude(Q(cmodel__isnull=True) & Q(active=False, bmodel__is_active=False)).query.__str__()

u'SELECT (...cut...) FROM "coreApi_amodel" INNER JOIN "coreApi_bmodel" ON ( "coreApi_amodel"."bmodel_id" = "coreApi_bmodel"."id" ) WHERE NOT ("coreApi_amodel"."id" IN (SELECT U0."id" AS "id" FROM "coreApi_amodel" U0 LEFT OUTER JOIN "coreApi_cmodel" U1 ON ( U0."id" = U1."amodel_id" ) WHERE U1."id" IS NULL) AND "coreApi_amodel"."active" = False AND "coreApi_bmodel"."is_active" = False)'

>>> models.Amodels.objects.exclude(Q(cmodel__isnull=True) & Q(active=False, bmodel__is_active=False))
[...results...]

[Tim Graham]

Could you please edit the ticket to include all the models and fields that your query references?

[Nikolay Kurevin]

Done.
Basic query to reproduce:

>>> models.Amodel.objects.exclude(Q(bmodel__id=False) & Q(cmodel__isnull=True)).query.__str__()

u'SELECT "TestTest_amodel"."id", "TestTest_amodel"."bmodel_id" FROM "TestTest_amodel" WHERE NOT ("TestTest_amodel"."bmodel_id" = 0 AND "TestTest_amodel"."id" IN (SELECT U0."id" AS "id" FROM TestTest_amodel U0 LEFT OUTER JOIN "TestTest_cmodel" U1 ON ( U0."id" = U1."amodel_id" ) WHERE (U1."id" IS NULL AND U0."id" = (TestTest_amodel."id"))))'

Bug is DB backend agnostic.

[Tim Graham]

Bisected to 01f2cf2aecc932d43b20b55fc19a8fa440457b5f. 1.7.x is now in security-fix only mode, but we can fix in 1.8.x.

[priidukull]

I have reproduced the issue by having added the following lines into the tests.queries module:

tests.py:

class TestTicket24605(TestCase):
    def test_ticket_24605(self):
        results = Amodel.objects.exclude(bmodel__id=False, cmodel__isnull=True)
        self.assertEqual(0, len(results))

models.py:

class Bmodel(models.Model):
    id = models.AutoField(primary_key=True)
    is_active = models.BooleanField()

    class Meta:
        db_table = 'Bmodel'


class Amodel(models.Model):
    active = models.BooleanField()
    bmodel = models.ForeignKey(Bmodel, related_name='Amodel_bmodel')

    class Meta:
        db_table = 'Amodel'


class Cmodel(models.Model):
    amodel = models.ForeignKey(Amodel)

    class Meta:
        db_table = 'Cmodel'

However, the test does not fail every time it runs. I have run the test for 100 consecutive times twice. Passed 42 times the first time I ran it and 45 times the second time I ran it. Whether the test fails or passes depends on in which order the children of the Q-object are iterated over. The test fails if the order of q_object.children is:

('bmodel__id', False), ('cmodel__isnull', True)

and passes when the order is inverse.

The order of q_object children is determined in the method Q.__init__()

super(Q, self).__init__(children=list(args) + list(six.iteritems(kwargs)))  

To make the test fail every time, change that line to

super(Q, self).__init__(children=list(args) + sorted(list(six.iteritems(kwargs))))

The line of code that caused the regression is ​https://github.com/django/django/commit/01f2cf2aecc932d43b20b55fc19a8fa440457b5f#diff-0edd853580d56db07e4020728d59e193R1530

However, removing that line from the code, would cause another test to fail.

[Simon Charette]

Your testcase probably sporadically fails and pass because of dict ordering.

I guess the following would always trigger a failure:

Amodel.objects.exclude(Q(bmodel__id=False), Q(cmodel__isnull=True))

[Simon Charette]

From further investigation with Priidu it looks like Query.split_exclude doesn't generate the correct query when specific aliases can be reused.

We reduced the test case to the following models:

from django.db import models


class Individual(models.Model):
    alive = models.BooleanField()

    class Meta:
        db_table = 'Individual'


class Child(models.Model):
    parent = models.ForeignKey(Individual, related_name='children')

    class Meta:
        db_table = 'Children'

where differently ordered queryset filters generate the following SQL.

# Individual.objects.exclude(Q(children__isnull=True), Q(alive=False))

SELECT "Individual"."id",
       "Individual"."alive"
FROM "Individual"
WHERE NOT ("Individual"."id" IN
             (SELECT U0."id" AS Col1
              FROM "Individual" U0
              LEFT OUTER JOIN "Children" U1 ON (U0."id" = U1."parent_id")
              WHERE U1."id" IS NULL)
           AND "Individual"."alive" = FALSE)

# Individual.objects.exclude(Q(alive=False), Q(children__isnull=True))

SELECT "Individual"."id",
       "Individual"."alive"
FROM "Individual"
WHERE NOT ("Individual"."alive" = FALSE
           AND "Individual"."id" IN
             (SELECT U0."id" AS Col1
              FROM Individual U0
              LEFT OUTER JOIN "Children" U1 ON (U0."id" = U1."parent_id")
              WHERE (U1."id" IS NULL
                     AND U0."id" = (Individual."id"))))

Notice how the second query's NOT IN subquery checks against U0."id" = (Individual."id") when it doesn't have too.

[Simon Charette]

Patch submitted by Anssi is looking good.

[Tim Graham <timograham@…>]

In 355c5ed:

Fixed #24605 -- Fixed incorrect reference to alias in subquery.

Thanks to charettes and priidukull for investigating the issue, and to
kurevin for the report.

[Tim Graham <timograham@…>]

In 581afddc:

[1.8.x] Fixed #24605 -- Fixed incorrect reference to alias in subquery.

Thanks to charettes and priidukull for investigating the issue, and to
kurevin for the report.

Backport of 355c5edd9390caad5725375abca03460805f663b from master

[Tim Graham <timograham@…>]

In c3a9820:

[1.7.x] Fixed #24605 -- Fixed incorrect reference to alias in subquery.

Thanks to charettes and priidukull for investigating the issue, and to
kurevin for the report.

Backport of 355c5edd9390caad5725375abca03460805f663b from master