[Django] #31922: Changing ordering of Q statements in filter produces different SQL and results.
Django
results.
-------------------------------------+-------------------------------------
Reporter: cbellinz | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 2.2
layer (models, ORM) | Keywords: orm, subquery, q,
Severity: Normal | order
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
This issue has been found when combining two Q statements that filter on
the same models in subqueries, but with slightly different where clauses.
By combining the two Q in different orders the SQL resulting from the
queries is different, one producing the correct result and one not. The
SQL that is not correct has had an additional where clause added to the
subquery.
Here is an example:
Two simple models (in module = utils)
{{{
class Book(BaseModel):
name = models.CharField(max_length=100, null=True, blank=True)
class BookStatus(BaseModel):
book = models.ForeignKey(Book, null=False, blank=False,
on_delete=models.CASCADE, related_name='statuses')
status = models.CharField(max_length=100, null=True, blank=True)
valid_from = models.DateTimeField(db_index=True)
valid_to = models.DateTimeField(db_index=True, blank=True, null=True,
default=None)
}}}
We will add some data:
from django.utils import timezone
from utils.models import *
from django.db.models import Q, Subquery
b1 = Book.objects.create(name='Green is the new Blue')
b2 = Book.objects.create(name='Orange is the new Red')
BookStatus.objects.create(book=b1, status='DRAFT',
valid_from=timezone.now())
BookStatus.objects.create(book=b1, status='PUBLISHED',
valid_from=timezone.now())
BookStatus.objects.create(book=b2, status='DRAFT',
valid_from=timezone.now())
Now we want to find all books that are in draft but not published.
{{{
q1 =
Q(statuses__id__in=Subquery(BookStatus.objects.only('id').filter(status='DRAFT')))
q2 =
~Q(statuses__id__in=Subquery(BookStatus.objects.only('id').filter(status='PUBLISHED')))
qs = Book.objects.only('name').filter(q1 & q2) # NOTICE Q1 BEFORE Q2
str(qs.query)
}}}
The resulting query is:
{{{
SELECT "utils_book"."id",
"utils_book"."name"
FROM "utils_book"
INNER JOIN "utils_bookstatus" ON ("utils_book"."id" =
"utils_bookstatus"."book_id")
WHERE (
"utils_bookstatus"."id" IN (
SELECT U0."id"
FROM "utils_bookstatus" U0
WHERE U0."status" = 'DRAFT'
)
AND NOT (
"utils_book"."id" IN (
SELECT V1."book_id"
FROM "utils_bookstatus" V1
WHERE (
V1."id" IN (
SELECT U0."id"
FROM "utils_bookstatus" U0
WHERE U0."status" = 'PUBLISHED'
)
AND V1."id" = ("utils_bookstatus"."id")
)
)
)
)
}}}
You can see the last where clause which joins the inner book status to the
outer one.
The result being that both books are returned:
{{{
qs
<QuerySet [<Book: Book object (1)>, <Book: Book object (2)>]>
}}}
Now, if we run the same query again, but reverse the order of the Q I get
a different result:
{{{
qs = Book.objects.only('name').filter(q2 & q1) # NOTICE Q2 BEFORE Q1
str(qs.query)
SELECT "utils_book"."id",
"utils_book"."name"
FROM "utils_book"
INNER JOIN "utils_bookstatus" ON ("utils_book"."id" =
"utils_bookstatus"."book_id")
WHERE (
NOT (
"utils_book"."id" IN (
SELECT V1."book_id"
FROM "utils_bookstatus" V1
WHERE V1."id" IN (
SELECT U0."id"
FROM "utils_bookstatus" U0
WHERE U0."status" = 'PUBLISHED'
)
)
)
AND "utils_bookstatus"."id" IN (
SELECT U0."id"
FROM "utils_bookstatus" U0
WHERE U0."status" = 'DRAFT'
)
)
}}}
The result of this query is correct:
{{{
qs
<QuerySet [<Book: Book object (2)>]>
}}}
There are better ways of trying to get the answer this query is looking
for, but this is a simplification of a more complex scenario and is for
demoing the issue.
Unfortunately, I have not been able to work out why this is happening or
where in the code the subquery is being amended.
Regards
Chris
--
Ticket URL: <https://code.djangoproject.com/ticket/31922>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
results.
-------------------------------------+-------------------------------------
Reporter: cbellinz | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: orm, subquery, q, | Triage Stage:
order | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
Here is an example:
The resulting query is:
{{{
qs
str(qs.query)
{{{
qs
Regards
Chris
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31922#comment:1>
Django
results.
-------------------------------------+-------------------------------------
Reporter: Chris Bell | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
order |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 2.2 => master
* stage: Unreviewed => Accepted
Comment:
Thanks for the detailed report, I was able to reproduce this issue.
Everything works properly with `Exists()`:
{{{
>>> q1_exists = Exists(BookStatus.objects.filter(status='DRAFT',
book__id=OuterRef('pk')))
>>> q2_exists = ~Exists(BookStatus.objects.filter(status='PUBLISHED',
book__id=OuterRef('pk')))
>>> Book.objects.filter(q2_exists & q1_exists)
<QuerySet [<Book: Book object (2)>]>
>>> Book.objects.filter(q1_exists & q2_exists)
<QuerySet [<Book: Book object (2)>]>
}}}
Reproduced at 41725602afebe2ddb018b99afe134384cc3bf69e.
--
Ticket URL: <https://code.djangoproject.com/ticket/31922#comment:2>
Django
results.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: orm, subquery, q, | Triage Stage: Accepted
order |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Chris Bell):
Hi @felixmm et al.
Thanks for confirming the issue and suggestion for using Exists.
The concern for us is how wide reaching this problem is. Ideally our tests
would catch erroneous SQL but that is not guaranteed and we are concerned
as to the possibility of security related issues arising from a similar
scenario.
We are looking into ways of trying to identify examples of our Q usages
that may be susceptible to this problem. In the meantime, if anyone in the
community has any tips/thoughts on where this issue may originate, we
would love to hear from you. We may be able to help investigate it with a
bit of assistance, specifically from those with experience of the inner
workings of the ORM.
Many thanks in advance
Chris
--
Ticket URL: <https://code.djangoproject.com/ticket/31922#comment:3>
Django
results.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: orm, subquery, q, | Triage Stage: Accepted
order |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Christian Klus):
I'm experiencing this on a few very complicated querysets - reverting back
to 3.0.6 fixes it, I'm thinking some of the queryset changes introduced in
3.0.7 are the cause of this.
--
Ticket URL: <https://code.djangoproject.com/ticket/31922#comment:4>
Django
results.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: orm, subquery, q, | Triage Stage: Accepted
order |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
Replying to [comment:4 Christian Klus]:
> I'm experiencing this on a few very complicated querysets - reverting
back to 3.0.6 fixes it, I'm thinking some of the queryset changes
introduced in 3.0.7 are the cause of this.
I'm able to reproduce the issue in Django < 3.0.6, so it seems that you've
encountered a different problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/31922#comment:5>
Django
results.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: orm, subquery, q, | Triage Stage: Accepted
order |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jacob Walls):
Dropping a failing test for tests/queries/tests.py:
{{{
def test_ticket_31922(self):
from django.db.models import Subquery
a1 = Order.objects.create(id=1, name='a1')
a2 = Order.objects.create(id=2, name='a2')
OrderItem.objects.create(order=a1, status=1)
OrderItem.objects.create(order=a1, status=2)
OrderItem.objects.create(order=a2, status=1)
q1 =
Q(items__id__in=Subquery(OrderItem.objects.only('id').filter(status=1)))
q2 =
~Q(items__id__in=Subquery(OrderItem.objects.only('id').filter(status=2)))
qs = Order.objects.filter(q1 & q2)
self.assertQuerysetEqual(qs, [repr(a2)])
}}}
This patch skips putting the join on the subquery, but it breaks the test
for #14511, where the join is desired on `exclude()`, so although it
doesn't seem like the right patch it at least shows Chris where the
changes are happening. If we need this join for `exclude()` but not for
`filter()` I'm all ears for how we're going to tell that apart upstream.
{{{
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 4648daf395..fa707d6b04 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -1792,10 +1792,12 @@ class Query(BaseExpression):
lookup_class = select_field.get_lookup('exact')
# Note that the query.select[0].alias is different from alias
# due to bump_prefix above.
- lookup = lookup_class(pk.get_col(query.select[0].alias),
- pk.get_col(alias))
- query.where.add(lookup, AND)
- query.external_aliases[alias] = True
+ col1 = pk.get_col(query.select[0].alias)
+ col2 = pk.get_col(alias)
+ if col1.target != col2.target:
+ lookup = lookup_class(col1, col2)
+ query.where.add(lookup, AND)
+ query.external_aliases[alias] = True
condition, needed_inner = self.build_filter(
('%s__in' % trimmed_prefix, query),
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31922#comment:6>