order_by("pk") not equivalent to order_by(F("pk")) for CompositePrimaryKey

[Jacob Walls]

For CompositePrimaryKey, order_by("pk") and order_by(F("pk")) produce different results.

For "pk", direction is distributed to all cols (col1 ASC, col2 ASC), but for F("pk"), it is not: (col1, col2 ASC).

Failing test:

tests/composite_pk/test_filter.py

@@ +1 @@
+from django.db.models import F
 from django.test import TestCase
 
 from .models import Comment, Tenant, User
@@ -78 +79 @@
             ),
         )
 
+    def test_order_by_comments_by_pk_asc_f(self):
+        self.assertSequenceEqual(
+            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,

======================================================================
FAIL: test_order_by_comments_by_pk_asc_f (composite_pk.test_filter.CompositePKFilterTests.test_order_by_comments_by_pk_asc_f)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/user/django/tests/composite_pk/test_filter.py", line 83, in test_order_by_comments_by_pk_asc_f
    self.assertSequenceEqual(
AssertionError: Sequences differ: <Quer[123 chars] Comment object ((1, 5))>, <Comment: Comment object ((2, 4))>]> != <Quer[123 chars] Comment object ((1, 5))>, <Comment: Comment object ((2, 4))>]>

  <QuerySet [<Comment: Comment object ((1, 1))>, <Comment: Comment object ((1, 2))>, <Comment: Comment object ((1, 3))>, <Comment: Comment object ((1, 5))>, <Comment: Comment object ((2, 4))>]>

----------------------------------------------------------------------
(0.000)
SELECT "composite_pk_comment"."tenant_id",
       "composite_pk_comment"."comment_id",
       "composite_pk_comment"."user_id",
       "composite_pk_comment"."text"
FROM "composite_pk_comment"
ORDER BY "composite_pk_comment"."tenant_id" ASC,
         "composite_pk_comment"."comment_id" ASC;

args=();

ALIAS=DEFAULT (0.000)
SELECT "composite_pk_comment"."tenant_id",
       "composite_pk_comment"."comment_id",
       "composite_pk_comment"."user_id",
       "composite_pk_comment"."text"
FROM "composite_pk_comment"
ORDER BY "composite_pk_comment"."tenant_id",
         "composite_pk_comment"."comment_id" ASC;

args=();

ALIAS=DEFAULT
----------------------------------------------------------------------
Ran 103 tests in 0.107s

FAILED (failures=1)

[Sarah Boyce]

Thank you!

[Simon Charette]

The changes made to SQLCompiler.find_ordering_name ​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.

django/db/models/expressions.py

@@ -1850 +1850 @@
         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:

django/db/models/sql/compiler.py

@@ -1117 +1117 @@
                 )
             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):

tests/composite_pk/test_filter.py

@@ +1 @@
+from django.db.models import F
 from django.test import TestCase
 
 from .models import Comment, Tenant, User
@@ -78 +79 @@
             ),
         )
 
+    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.