[Django] #29545: Nested OuterRef not looking on the right model for the field.
Django
-------------------------------------+-------------------------------------
Reporter: pwfff | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 2.0
layer (models, ORM) |
Severity: Normal | Keywords: outerref, subquery
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I ran into this and made a test case for it. It seems similar to the test
case above it. I'm not sure what's different about it and causing it to
fail.
Let me know if the query is just build wrong.
{{{
diff --git a/tests/expressions/models.py b/tests/expressions/models.py
index 42e4a37bb0..164bda3b3d 100644
--- a/tests/expressions/models.py
+++ b/tests/expressions/models.py
@@ -83,6 +83,28 @@ class SimulationRun(models.Model):
return "%s (%s to %s)" % (self.midpoint, self.start, self.end)
+class Customer(models.Model):
+ name = models.TextField()
+
+
+class Item(models.Model):
+ pass
+
+
+class Invoice(models.Model):
+ INVOICE = 'invoice'
+ EXPENSE = 'expense'
+
+ KIND_CHOICES = (
+ (INVOICE, 'Invoice'),
+ (EXPENSE, 'Expense'),
+ )
+
+ kind = models.CharField(choices=KIND_CHOICES, max_length=255,
default=None)
+ owner = models.ForeignKey(Customer, models.CASCADE)
+ items = models.ManyToManyField(Item, related_name='invoices')
+
+
class UUIDPK(models.Model):
id = models.UUIDField(primary_key=True, default=uuid.uuid4)
diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py
index d1e622a2d4..2c50744fc8 100644
--- a/tests/expressions/tests.py
+++ b/tests/expressions/tests.py
@@ -24,7 +24,7 @@ from django.test.utils import Approximate
from .models import (
UUID, UUIDPK, Company, Employee, Experiment, Number, Result,
SimulationRun,
- Time,
+ Time, Customer, Item, Invoice
)
@@ -536,6 +536,55 @@ class BasicExpressionsTests(TestCase):
# This is a contrived example. It exercises the double OuterRef
form.
self.assertCountEqual(outer, [first, second, third])
+ def test_nested_subquery_outer_ref_3(self):
+ customer = Customer.objects.create(name='Test Customer')
+ other_customer = Customer.objects.create(name='Ohter Customer')
+
+ unexpensed_invoice = Invoice.objects.create(kind=Invoice.INVOICE,
owner=customer)
+ unexpensed_item_1 = Item.objects.create()
+ unexpensed_invoice.items.add(unexpensed_item_1)
+
+ partially_expensed_invoice =
Invoice.objects.create(kind=Invoice.INVOICE, owner=customer)
+ expense_1 = Invoice.objects.create(kind=Invoice.EXPENSE,
owner=customer)
+ expensed_item_1 = Item.objects.create()
+ unexpensed_item_2 = Item.objects.create()
+ partially_expensed_invoice.items.add(expensed_item_1,
unexpensed_item_2)
+ expense_1.items.add(expensed_item_1)
+
+ expensed_invoice = Invoice.objects.create(kind=Invoice.INVOICE,
owner=customer)
+ Invoice.objects.create(kind=Invoice.EXPENSE, owner=customer)
+ expensed_item_2 = Item.objects.create()
+ expensed_invoice.items.add(expensed_item_2)
+ expense_1.items.add(expensed_item_2)
+
+ other_invoice = Invoice.objects.create(kind=Invoice.INVOICE,
owner=other_customer)
+ other_invoice.items.add(unexpensed_item_1)
+ other_expense = Invoice.objects.create(kind=Invoice.EXPENSE,
owner=other_customer)
+ other_expense.items.add(unexpensed_item_1)
+
+ inner = Invoice.objects.filter(
+ kind=Invoice.EXPENSE,
+ owner=OuterRef(OuterRef('owner')),
+ items=OuterRef('id'),
+ )
+ middle = Item.objects.filter(
+ invoices=OuterRef('id'),
+ ).annotate(
+ expensed=Exists(inner),
+ ).filter(
+ expensed=False,
+ )
+ outer = Invoice.objects.filter(
+ kind=Invoice.INVOICE,
+ owner=customer,
+ ).annotate(
+ unexpensed=Exists(middle),
+ )
+
+ self.assertTrue(outer.get(pk=unexpensed_invoice.pk).unexpensed)
+
self.assertTrue(outer.get(pk=partially_expensed_invoice.pk).unexpensed)
+ self.assertFalse(outer.get(pk=expensed_invoice.pk).unexpensed)
+
def test_nested_subquery_outer_ref_with_autofield(self):
first = Time.objects.create(time='09:00')
second = Time.objects.create(time='17:00')
}}}
{{{
django.core.exceptions.FieldError: Cannot resolve keyword 'owner' into
field. Choices are: expensed, id, invoices
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29545>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: pwfff | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: outerref, subquery | Triage Stage:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
I ran into this and made a test case for it. It seems similar to the test
case above it. I'm not sure what's different about it and causing it to
fail.
Let me know if the query is just built wrong.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
I can't definitely say that it's a bug. Accepting for investigation.
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Can Sarıgöl (added)
Comment:
IMO this query join order should be like this:
{{{
inner = Item.objects.filter(
middle = Invoice.objects.filter(
outer = Item.objects.filter(
}}}
If you want to use nested outerref, middle query has to contain foreign
keys. Otherwise, this usage needs to include deep related field.
{{{
middle = Item.objects.filter(invoices=OuterRef('id'),).annotate(
expensed=Exists(inner),
owner=F('invoices__owner')
).filter(expensed=False,)
}}}
or maybe not necessary nested outerref
{{{
inner = Invoice.objects.filter(
kind=Invoice.EXPENSE,
owner=OuterRef('invoices__owner'),
items=OuterRef('id'),
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Oskar Persson):
I also ran into this just now and can confirm that the bug exists in both
2.2.4 and master, tested with the models in tests/queries/models.py:
{{{
def test_nested_outerref(self):
from .models import Item, Node, Number
Number.objects.create(num=5)
Node.objects.create(num=5)
qs = Number.objects.annotate(
has_item=Exists(
Item.objects.annotate(
has_tag=Exists(
Node.objects.filter(num=OuterRef(OuterRef('num')))
)
).filter(has_tag=True)
)
)
print(qs)
}}}
This results in the following exception
{{{
django.core.exceptions.FieldError: Cannot resolve keyword 'num' into
field. Choices are: cover, created,
creator, creator_id, has_tag, id, modified, name, note, note_id, tags
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:4>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Oskar Persson (added)
* version: 2.0 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:5>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Thanks for simplified test case Oskar, that's really valuable.
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:6>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => Simon Charette
Comment:
This is more of a brain dump than a final resolution but I think I might
have nailed it down. I thought I'd share how I came with what I believe is
the solution to share a bit of knowledge about how the ORM resolves
expressions.
By breaking the query construction in multiple statements and adding
breakpoints in `OuterRef.resolve_expression` and
`ResolvedOuterRef.resolve_expression` I noticed that the latter was being
called '''after''' the complete query construction when it was being
executed which should not happen. In short all expressions should be
resolved to their '''final''' expression at this point and in the case of
`OuterRef` and `ResolvedOuterRef` they should be `Col` instances.
I then assumed that something must not be have been esolved properly so I
added a breakpoint before the `print(qs)` and dug into `qs.query`
internals. By walking the chain of
`qs.query.annotations['has_item'].query` I noticed that
`.where.children[0].lhs` was and `Exist` instance and that its
`.query.where.children[0].rhs` happened to be a `ResolvedOuterRef`
instance. I then knew that this was the culprit as it should have been a
`Col` by this point. I also noticed that the copy of the `Exists` present
as the `.has_tag` expression had its `.query.where.children[0].rhs`
correctly resolved to a `Col` so I adjusted the test to replace the
unresolved reference to the `Col` and got the test passing.
{{{#!python
def test_nested_outerref(self):
number = Number.objects.create(num=5)
Node.objects.create(num=5)
nested_query = Node.objects.filter(num=OuterRef(OuterRef('num')))
nested_exist = Exists(nested_query)
query = Item.objects.annotate(
has_tag=nested_exist,
).filter(has_tag=True)
exists = Exists(query)
qs = Number.objects.annotate(
has_item=exists
)
col =
qs.query.annotations['has_item'].query.annotations['has_tag'].query.where.children[0].rhs
qs.query.annotations['has_item'].query.where.children[0].lhs.query.where.children[0].rhs
= col
print(str(qs.query))
self.assertEqual(qs.get(), number)
}}}
From that point I knew that there was an issue resolving
`qs.query.annotations['has_item'].query.where.children[0].lhs` and then I
remembered that `sql.Where.resolve_expression` was performing resolving
for `.rhs` of lookups but not for `.lhs` which brought me to
[https://github.com/django/django/compare/master...charettes:ticket-29545
this still incomplete patch] that happens to solve this issue and pass the
test suite on SQLite so far.
I've assigned the ticket to myself as I plan to submit the above branch
for review in the next days. I wouldn't be surprised if it fixed a other
issues related to subqueries and `OuterRef`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:7>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/11677
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:8>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Can Sarıgöl):
Sorry I misinterpreted. Simon is right. Thanks for the explanation.
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:9>
Django
-------------------------------------+-------------------------------------
| Charette
Component: Database layer | Version: master
(models, ORM) |
Keywords: outerref, subquery | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"8b4a43dda702fe72c254388f1be2c0c75b7a3efc" 8b4a43d]:
{{{
#!CommitTicketReference repository=""
revision="8b4a43dda702fe72c254388f1be2c0c75b7a3efc"
Fixed #29545 -- Fixed using filter lookups againts nested subquery
expressions.
Made sql.Where resolve lhs of its child nodes. This is necessary to
allow filter lookups against nested subquery expressions to properly
resolve their OuterRefs to Cols.
Thanks Oskar Persson for the simplified test case.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29545#comment:10>