From ee6e93ec8727d0f5ed33190a3c354867669ed72f Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Tue, 9 Jul 2019 17:26:37 -0400 Subject: [PATCH] Fixed #30628 -- Adjusted expression identity to differentiate bound fields. Expressions referring to different bound fields should not be considered equal. Thanks Julien Enselme for the detailed report. Regression in bc7e288ca9554ac1a0a19941302dea19df1acd21. --- django/db/models/expressions.py | 5 ++++- docs/releases/2.2.4.txt | 4 +++- tests/expressions/tests.py | 21 ++++++++++++++++++++- tests/queries/models.py | 1 + tests/queries/test_qs_combinators.py | 9 ++++++++- tests/queries/tests.py | 2 +- 6 files changed, 37 insertions(+), 5 deletions(-) diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index 36f88f99ec..b757426440 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -376,7 +376,10 @@ class BaseExpression: identity = [self.__class__] for arg, value in arguments: if isinstance(value, fields.Field): - value = type(value) + if value.name and value.model: + value = (value.model._meta.label, value.name) + else: + value = type(value) else: value = make_hashable(value) identity.append((arg, value)) diff --git a/docs/releases/2.2.4.txt b/docs/releases/2.2.4.txt index 0bc4dce295..a1a849680d 100644 --- a/docs/releases/2.2.4.txt +++ b/docs/releases/2.2.4.txt @@ -9,4 +9,6 @@ Django 2.2.4 fixes several bugs in 2.2.3. Bugfixes ======== -* ... +* Fixed a regression in Django 2.2 when ordering a ``QuerySet.union()``, + ``intersection()``, or ``difference()`` by a field type present more than + once results in the wrong ordering being used (:ticket:`30628`). diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index fb9fd3d1e2..f5d1553dfb 100644 --- a/tests/expressions/tests.py +++ b/tests/expressions/tests.py @@ -21,7 +21,7 @@ from django.db.models.functions import ( from django.db.models.sql import constants from django.db.models.sql.datastructures import Join from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature -from django.test.utils import Approximate +from django.test.utils import Approximate, isolate_apps from .models import ( UUID, UUIDPK, Company, Employee, Experiment, Number, RemoteEmployee, @@ -898,6 +898,7 @@ class ExpressionsTests(TestCase): ) +@isolate_apps('expressions') class SimpleExpressionTests(SimpleTestCase): def test_equal(self): @@ -911,6 +912,15 @@ class SimpleExpressionTests(SimpleTestCase): Expression(models.CharField()) ) + class TestModel(models.Model): + field = models.IntegerField() + other_field = models.IntegerField() + + self.assertNotEqual( + Expression(TestModel._meta.get_field('field')), + Expression(TestModel._meta.get_field('other_field')), + ) + def test_hash(self): self.assertEqual(hash(Expression()), hash(Expression())) self.assertEqual( @@ -922,6 +932,15 @@ class SimpleExpressionTests(SimpleTestCase): hash(Expression(models.CharField())), ) + class TestModel(models.Model): + field = models.IntegerField() + other_field = models.IntegerField() + + self.assertNotEqual( + hash(Expression(TestModel._meta.get_field('field'))), + hash(Expression(TestModel._meta.get_field('other_field'))), + ) + class ExpressionsNumericTests(TestCase): diff --git a/tests/queries/models.py b/tests/queries/models.py index eaddba1c8d..f228a61a70 100644 --- a/tests/queries/models.py +++ b/tests/queries/models.py @@ -148,6 +148,7 @@ class Cover(models.Model): class Number(models.Model): num = models.IntegerField() + other_num = models.IntegerField(null=True) def __str__(self): return str(self.num) diff --git a/tests/queries/test_qs_combinators.py b/tests/queries/test_qs_combinators.py index 6d1ac9c2bf..e7cfbfb0d1 100644 --- a/tests/queries/test_qs_combinators.py +++ b/tests/queries/test_qs_combinators.py @@ -9,7 +9,7 @@ from .models import Number, ReservedName class QuerySetSetOperationTests(TestCase): @classmethod def setUpTestData(cls): - Number.objects.bulk_create(Number(num=i) for i in range(10)) + Number.objects.bulk_create(Number(num=i, other_num=10 - i) for i in range(10)) def number_transform(self, value): return value.num @@ -251,3 +251,10 @@ class QuerySetSetOperationTests(TestCase): qs1 = Number.objects.all() qs2 = Number.objects.intersection(Number.objects.filter(num__gt=1)) self.assertEqual(qs1.difference(qs2).count(), 2) + + def test_order_by_same_type(self): + qs = Number.objects.all() + union = qs.union(qs) + numbers = list(range(10)) + self.assertNumbersEqual(union.order_by('num'), numbers) + self.assertNumbersEqual(union.order_by('other_num'), reversed(numbers)) diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 7e2284e751..c49a0231ac 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -2335,7 +2335,7 @@ class ValuesQuerysetTests(TestCase): qs = Number.objects.extra(select={'num2': 'num+1'}).annotate(Count('id')) values = qs.values_list(named=True).first() self.assertEqual(type(values).__name__, 'Row') - self.assertEqual(values._fields, ('num2', 'id', 'num', 'id__count')) + self.assertEqual(values._fields, ('num2', 'id', 'num', 'other_num', 'id__count')) self.assertEqual(values.num, 72) self.assertEqual(values.num2, 73) self.assertEqual(values.id__count, 1)