From 9dee8515d6f2876fa039aaebdfe8e2bc9de63085 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Tue, 9 Jul 2019 17:26:37 -0400 Subject: [PATCH] [2.2.x] 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. Backport of ee6e93ec8727d0f5ed33190a3c354867669ed72f from master --- 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 ccb9636503..a67de51cdc 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -372,7 +372,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 e66dcd6297..f0819992d2 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, @@ -839,6 +839,7 @@ class ExpressionsTests(TestCase): ) +@isolate_apps('expressions') class SimpleExpressionTests(SimpleTestCase): def test_equal(self): @@ -852,6 +853,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( @@ -863,6 +873,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 af0af1d10c..5751738c95 100644 --- a/tests/queries/models.py +++ b/tests/queries/models.py @@ -143,6 +143,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 3902db59e2..0c1c614999 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 @@ -225,3 +225,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 c655fe52cb..e72ecaa654 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -2345,7 +2345,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)