diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index abbb1e37cb..edc283d94c 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -727,7 +727,7 @@ class SQLCompiler: # If we get to this point and the field is a relation to another model, # append the default ordering for that model unless it is the pk # shortcut or the attribute name of the field that is specified. - if field.is_relation and opts.ordering and getattr(field, 'attname', None) != name and name != 'pk': + if field.is_relation and opts.ordering and getattr(field, 'attname', None) != pieces[-1] and pieces[-1] != 'pk': # Firstly, avoid infinite loops. already_seen = already_seen or set() join_tuple = tuple(getattr(self.query.alias_map[j], 'join_cols', None) for j in joins) diff --git a/tests/ordering/models.py b/tests/ordering/models.py index 2efb743e44..44c0f9d095 100644 --- a/tests/ordering/models.py +++ b/tests/ordering/models.py @@ -59,3 +59,17 @@ class Reference(models.Model): class Meta: ordering = ('article',) + + +# Models for testing self-referencing foreign key ordering +class OneModel(models.Model): + root = models.ForeignKey("self", on_delete=models.CASCADE, null=True) + oneval = models.BigIntegerField(null=True) + + class Meta: + ordering = ("-id",) + + +class TwoModel(models.Model): + record = models.ForeignKey(OneModel, on_delete=models.CASCADE) + twoval = models.BigIntegerField(null=True) diff --git a/tests/ordering/test_self_referencing_fk.py b/tests/ordering/test_self_referencing_fk.py new file mode 100644 index 0000000000..2f7b5b1670 --- /dev/null +++ b/tests/ordering/test_self_referencing_fk.py @@ -0,0 +1,124 @@ +""" +Test for self-referencing foreign key ordering bug. +""" +from django.test import TestCase + +from .models import OneModel, TwoModel + + +class SelfReferencingForeignKeyOrderingTests(TestCase): + """ + Tests for ordering by _id field on self-referencing foreign keys. + + Regression test for: Self referencing foreign key doesn't correctly + order by a relation "_id" field. + """ + + @classmethod + def setUpTestData(cls): + # Create test data + cls.one1 = OneModel.objects.create(oneval=1, root=None) + cls.one2 = OneModel.objects.create(oneval=2, root=cls.one1) + cls.one3 = OneModel.objects.create(oneval=3, root=cls.one2) + + cls.two1 = TwoModel.objects.create(record=cls.one1, twoval=10) + cls.two2 = TwoModel.objects.create(record=cls.one2, twoval=20) + cls.two3 = TwoModel.objects.create(record=cls.one3, twoval=30) + + def test_order_by_self_referencing_fk_id_field(self): + """ + Test ordering by _id field on a self-referencing foreign key. + + When ordering by record__root_id, it should: + 1. Use ASC order (not DESC from OneModel.Meta.ordering) + 2. Use a single INNER JOIN (not an extra LEFT OUTER JOIN) + 3. Order by the root_id column directly + """ + qs = TwoModel.objects.filter(record__oneval__in=[1, 2, 3]) + qs = qs.order_by("record__root_id") + + # Get the SQL query + sql = str(qs.query) + + # The query should have ascending order, not descending + # It should NOT have "ORDER BY T3.id DESC" or similar + # It should have "ORDER BY ... ASC" or just order by the root_id column + + # Check that there's only ONE join (INNER JOIN), not two + # Count the number of JOIN clauses + join_count = sql.upper().count(' JOIN ') + self.assertEqual( + join_count, 1, + f"Expected 1 JOIN but found {join_count}. SQL: {sql}" + ) + + # The query should not have a LEFT OUTER JOIN + self.assertNotIn( + 'LEFT OUTER JOIN', sql.upper(), + f"Unexpected LEFT OUTER JOIN in query. SQL: {sql}" + ) + + # The ORDER BY should be ascending (ASC or implicit) + # and should reference the root_id column directly + self.assertIn('ORDER BY', sql.upper()) + + # Split to get the ORDER BY clause + order_by_part = sql.upper().split('ORDER BY')[1].strip() + + # It should not end with DESC (from the model's default ordering) + # If the bug is present, it will have "T3"."id" DESC or similar + self.assertNotRegex( + order_by_part, + r'T\d+.*DESC', + f"ORDER BY clause has unexpected DESC with aliased table. SQL: {sql}" + ) + + def test_order_by_self_referencing_fk_id_field_desc(self): + """ + Test ordering by -record__root_id (descending). + + When explicitly ordering descending, it should respect that order, + not invert it based on the model's default ordering. + """ + qs = TwoModel.objects.filter(record__oneval__in=[1, 2, 3]) + qs = qs.order_by("-record__root_id") + + sql = str(qs.query) + + # Should still have only one JOIN + join_count = sql.upper().count(' JOIN ') + self.assertEqual( + join_count, 1, + f"Expected 1 JOIN but found {join_count}. SQL: {sql}" + ) + + # Should not have LEFT OUTER JOIN + self.assertNotIn( + 'LEFT OUTER JOIN', sql.upper(), + f"Unexpected LEFT OUTER JOIN in query. SQL: {sql}" + ) + + # The ORDER BY should be explicitly descending + order_by_part = sql.upper().split('ORDER BY')[1].strip() + self.assertIn('DESC', order_by_part) + + def test_order_by_self_referencing_fk_id_field_via_double_underscore_id(self): + """ + Test ordering by record__root__id (with __id at the end). + + This should work correctly and produce optimal SQL. + """ + qs = TwoModel.objects.filter(record__oneval__in=[1, 2, 3]) + qs = qs.order_by("record__root__id") + + sql = str(qs.query) + + # Should have only one JOIN + join_count = sql.upper().count(' JOIN ') + self.assertEqual( + join_count, 1, + f"Expected 1 JOIN but found {join_count}. SQL: {sql}" + ) + + # Should reference root_id column directly + self.assertIn('root_id', sql.lower())