[Django] #34533: OuterRef not resolved as part oh ORDER BY clause
Django
-------------------------------------+-------------------------------------
Reporter: Alombaros | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 3.2
layer (models, ORM) |
Severity: Normal | Keywords: OuterRef, OrderBy
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I feel like OuterRef are not resolved properly where used for ordering.
Here is a simple example showcasing the issue:
{{{
MyModel.objects.annotate(foo=Subquery(MyModel.objects.order_by("pk" -
OuterRef("pk")).values("pk")[:1]))
}}}
The above fails with :
{{{ ValueError: This queryset contains a reference to an outer query and
may only be used in a subquery. }}}
Because the as_sql method of ResolvedOuterRef is called.
I think this a bug because it did not raise any notice regarding OuterRef
and order_by not being compatible.
--
Ticket URL: <https://code.djangoproject.com/ticket/34533>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Thanks 👍
Just FYI this will work (note you must wrap 'pk' with an F):
{{{
MyModel.objects.annotate(foo=Subquery(MyModel.objects.annotate(order=F("pk")
- OuterRef("pk")).order_by("order").values("pk")[:1]))
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:1>
Django
-------------------------------------+-------------------------------------
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Jordan Bae
* status: new => assigned
Comment:
I will try to fix this!
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by REGNIER Guillaume):
I've been working in and out on this for the last three weeks. I'm sorry I
didn't claim the ticket...
Here is a branch where the issue is solved:
https://github.com/Alombaros/django/tree/ticket_34533_3_2
I'm unsure on how to proceed.
Previously, in the following query :
`MyModel.objects.annotate(foo=Subquery(MyOtherModel.objects.order_by(F("pk")).values("pk")[:1]))`
The `F("pk")` was resolved as the `pk` of the outer query (`MyModel` in
this case) which does not seems right to me.
This was due to the order by expressions not being resolved until the last
minute.
I force the resolution when annotating a SubQuery so that the OuterRef can
do its job but this made so `F` objects are resolved as field of the inner
query.
To me, this seems to be closer to the intended behavior because its what
append when calling `.filter` or `.annotate` but if anyone used the prior
behavior, thing will break
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Jordan Bae => (none)
* status: assigned => new
Comment:
no worry! i think it's better tyou bring this ticket. i deassign this
ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:4>
Django
-------------------------------------+-------------------------------------
| Guillaume
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jordan Bae):
* owner: (none) => REGNIER Guillaume
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:5>
Django
-------------------------------------+-------------------------------------
| Patel
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: REGNIER Guillaume => Umang Patel
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: Umang
| Patel
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Umang Patel):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/17326
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:7>
Django
-------------------------------------+-------------------------------------
| Guillaume
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Umang Patel => REGNIER Guillaume
* needs_better_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/16994 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:8>
Django
-------------------------------------+-------------------------------------
| Adzhani
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: REGNIER Guillaume => Shafiya Adzhani
Comment:
I will try to improve what can be done from previous PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: Shafiya
| Adzhani
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
The hard part is here is likely due to two factor
1. `OuterRef` resolving is quite finicky and depends on a specific chain
of resolving during query composition (subquery is resolved and then the
outer query is)
2. `order_by` members are unfortunately not resolved at query composition
time (when `QuerySet.order_by` is called) but
[https://github.com/django/django/blob/12ffcfc350a19bbfbc203126a9b6c84b5e0d0ba2/django/db/models/sql/compiler.py#L466
at query compilation time].
The reason for 2. is that `QuerySet.order_by` calls are not additive but
destructive and that the query composition logic doesn't support a generic
way of eliding joins that we previously created for some operations.
For example, say that you have a query that does `qs =
Book.objects.order_by("author__name")`. If the order by clause is
immediately resolved then a `JOIN` to `author` must be created. If the
same query has then its ordering changed to `qs.order_by("title")` then
we'd want to clear the join to `author` as it's no longer necessary.
In order to avoid implementing the logic to ''unreference'' relations the
`order_by` method deferred the resolving to the very end of query
compilation which is a different path than the ''normal'' resolving taking
place in additive methods such as `annotate` and `filter`.
I suspect this ticket will be hard to solve without tackling the large
problem of compile time resolving of `order_by`.
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: Shafiya
| Adzhani
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Replying to [comment:10 Simon Charette]:
> The hard part is here is likely due to two factor
>
> 1. `OuterRef` resolving is quite finicky and depends on a specific chain
of resolving during query composition (subquery is resolved and then the
outer query is)
> 2. `order_by` members are unfortunately not resolved at query
composition time (when `QuerySet.order_by` is called) but
[https://github.com/django/django/blob/12ffcfc350a19bbfbc203126a9b6c84b5e0d0ba2/django/db/models/sql/compiler.py#L466
at query compilation time].
>
> The reason for 2. is that `QuerySet.order_by` calls are not additive but
destructive and that the query composition logic doesn't support a generic
way of eliding joins that we previously created for some operations.
>
> For example, say that you have a query that does `qs =
Book.objects.order_by("author__name")`. If the order by clause is
immediately resolved then a `JOIN` to `author` must be created. If the
same query has then its ordering changed to `qs.order_by("title")` then
we'd want to clear the join to `author` as it's no longer necessary.
>
> In order to avoid implementing the logic to ''unreference'' relations
the `order_by` method deferred the resolving to the very end of query
compilation which is a different path than the ''normal'' resolving taking
place in additive methods such as `annotate` and `filter`.
>
> I suspect this ticket will be hard to solve without tackling the large
problem of compile time resolving of `order_by`.
Thank you for the pointer! Since this ticket is complicated, I'll leave it
to someone who is interested in solving this problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:11>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Shafiya Adzhani => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
could i try fix this ticket?
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
You're welcome to try with the disclaimer that you understand what Simon
has said above about it being quite hard and it will take an in-depth
understanding of how expressions are resolved & compiled :)
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Thank you for answer.
I'll consider it further.
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
@HeejunShin, you can try running the example from the ticket Description -
see if that works with the main branch. I don't think this issue has been
fixed, though.
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
I am able to reproduce this issue with a simple unit test still against
the main branch.
{{{#!diff
diff --git a/tests/queries/tests.py b/tests/queries/tests.py
index ec88fa558d..a9dd943ee8 100644
--- a/tests/queries/tests.py
+++ b/tests/queries/tests.py
@@ -2377,6 +2377,13 @@ class SubqueryTests(TestCase):
NamedCategory.objects.create(id=3, name="third")
NamedCategory.objects.create(id=4, name="fourth")
+ def test_outer_ref_order_by(self):
+ values = NamedCategory.objects.annotate(
+
foo=NamedCategory.objects.order_by(OuterRef("pk").desc()).values("name")
+ ).first()
+
+ self.assertEqual(["fourth", "third", "second", "first"],
values.foo)
+
def test_ordered_subselect(self):
"Subselects honor any manual ordering"
query = DumbCategory.objects.filter(
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:17>
Django
-------------------------------------+-------------------------------------
| Khatri
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Ayush Khatri
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: REGNIER Guillaume | Owner: Ayush
| Khatri
Type: Bug | Status: assigned
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: OuterRef, OrderBy | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
https://github.com/django/django/pull/18878
--
Ticket URL: <https://code.djangoproject.com/ticket/34533#comment:19>