Fix incorrect ordering with self-referencing foreign keys

Ensure order_by on related "_id" fields takes precedence over
model Meta.ordering, avoiding extra joins and unexpected sort
order. Resolves issues with self-referential relations.
This commit is contained in:
utkarsh.arya@zomato.com 2025-11-15 23:01:02 +00:00
parent a59de6e89e
commit 745277205f
3 changed files with 139 additions and 1 deletions

View file

@ -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)

View file

@ -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)

View file

@ -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())