#36065: order_by("pk") not equivalent to order_by(F("pk")) for
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
The changes made to `SQLCompiler.find_ordering_name`
[https://github.com/django/django/commit/978aae4334fa71ba78a3e94408f0f3aebde8d07c
#diff-
f58de2deaccecd2d53199c5ca29e3e1050ec2adb80fb057cdfc0b4e6accdf14fR1119 to
accommodate for] `order_by(*: str)` references to a composite primary key
failed to account for the fact that resolvable and direct `OrderBy`
instance can be passed to `QuerySet.order_by`.
The solution likely lies in adjusting `find_ordering_name` to create a
`ColPairs` when there are multiple targets (instead of calling
`composite.unnest`) and adjusting `OrderBy.as_sql` to properly deal with
`ColPairs`. I guess we should test for `F("pk").asc(nulls_first=True)` and
`F("pk").asc(nulls_last=True)` (even if primary keys cannot contain nulls)
while we're at it to ensure the proper `NULLS LAST` emulation is working
correction.
{{{#!diff
diff --git a/django/db/models/expressions.py
b/django/db/models/expressions.py
index 667e9f93c6..2e1828ff9b 100644
--- a/django/db/models/expressions.py
+++ b/django/db/models/expressions.py
@@ -1850,6 +1850,16 @@ def get_source_expressions(self):
return [self.expression]
def as_sql(self, compiler, connection, template=None,
**extra_context):
+ if isinstance(self.expression, ColPairs):
+ sql_parts = []
+ params = []
+ for col in self.expression.get_cols():
+ copy = self.copy()
+ copy.set_source_expressions([col])
+ sql, col_params = compiler.compile(copy)
+ sql_parts.append(sql)
+ params.extend(col_params)
+ return ", ".join(sql_parts), params
template = template or self.template
if connection.features.supports_order_by_nulls_modifier:
if self.nulls_last:
diff --git a/django/db/models/sql/compiler.py
b/django/db/models/sql/compiler.py
index 5bb491d823..251cc08e51 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -1117,10 +1117,9 @@ def find_ordering_name(
)
return results
targets, alias, _ = self.query.trim_joins(targets, joins, path)
- target_fields = composite.unnest(targets)
return [
(OrderBy(transform_function(t, alias),
descending=descending), False)
- for t in target_fields
+ for t in targets
]
def _setup_joins(self, pieces, opts, alias):
diff --git a/tests/composite_pk/test_filter.py
b/tests/composite_pk/test_filter.py
index 7e361c5925..be0e80a518 100644
--- a/tests/composite_pk/test_filter.py
+++ b/tests/composite_pk/test_filter.py
@@ -1,3 +1,4 @@
+from django.db.models import F
from django.test import TestCase
from .models import Comment, Tenant, User
@@ -78,6 +79,12 @@ def test_order_comments_by_pk_desc(self):
),
)
+ def test_order_comments_by_pk_asc_f(self):
+ self.assertQuerySetEqual(
+ Comment.objects.order_by("pk"),
+ Comment.objects.order_by(F("pk")),
+ )
+
def test_filter_comments_by_pk_gt(self):
c11, c12, c13, c24, c15 = (
self.comment_1
}}}
Note that we'll need to mark `OrderBy.allows_composite_expressions = True`
per the work on #36042 depending on the order in which the changes are
merged.
--
Ticket URL: <https://code.djangoproject.com/ticket/36065#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/010701943c5a1a5a-0172d42d-0acb-4247-9fdd-2d7fdd6b4f54-000000%40eu-central-1.amazonses.com.
