From 512da9d585513bcf4aaf977e4ad82c208897a7b1 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sat, 27 Jun 2020 17:41:32 -0400 Subject: [PATCH] Fixed #23797 -- Fixed QuerySet.exclude() when rhs is a nullable column. --- AUTHORS | 1 + django/db/models/sql/query.py | 19 +++++++++++++------ tests/queries/models.py | 1 + tests/queries/tests.py | 17 ++++++++++++++++- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/AUTHORS b/AUTHORS index 7ae9bb185a..d7f47d63d7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -392,6 +392,7 @@ answer newbie questions, and generally made Django that much better: Jacob Burch Jacob Green Jacob Kaplan-Moss + Jacob Walls Jakub Paczkowski Jakub Wilk Jakub Wiśniowski diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index d65141b834..c913267476 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1324,9 +1324,7 @@ class Query(BaseExpression): require_outer = lookup_type == 'isnull' and condition.rhs is True and not current_negated if current_negated and (lookup_type != 'isnull' or condition.rhs is False) and condition.rhs is not None: require_outer = True - if (lookup_type != 'isnull' and ( - self.is_nullable(targets[0]) or - self.alias_map[join_list[-1]].join_type == LOUTER)): + if lookup_type != 'isnull': # The condition added here will be SQL like this: # NOT (col IS NOT NULL), where the first NOT is added in # upper layers of code. The reason for addition is that if col @@ -1336,9 +1334,18 @@ class Query(BaseExpression): # (col IS NULL OR col != someval) # <=> # NOT (col IS NOT NULL AND col = someval). - lookup_class = targets[0].get_lookup('isnull') - col = self._get_col(targets[0], join_info.targets[0], alias) - clause.add(lookup_class(col, False), AND) + if ( + self.is_nullable(targets[0]) or + self.alias_map[join_list[-1]].join_type == LOUTER + ): + lookup_class = targets[0].get_lookup('isnull') + col = self._get_col(targets[0], join_info.targets[0], alias) + clause.add(lookup_class(col, False), AND) + # If someval is a nullable column, someval IS NOT NULL is + # added. + if isinstance(value, Col) and self.is_nullable(value.target): + lookup_class = value.target.get_lookup('isnull') + clause.add(lookup_class(value, False), AND) return clause, used_joins if not require_outer else () def add_filter(self, filter_clause): diff --git a/tests/queries/models.py b/tests/queries/models.py index ecf0b29130..a4f6245aa8 100644 --- a/tests/queries/models.py +++ b/tests/queries/models.py @@ -142,6 +142,7 @@ class Cover(models.Model): class Number(models.Model): num = models.IntegerField() other_num = models.IntegerField(null=True) + another_num = models.IntegerField(null=True) def __str__(self): return str(self.num) diff --git a/tests/queries/tests.py b/tests/queries/tests.py index eeb8a5e591..2863cfb568 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -2372,7 +2372,10 @@ 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', 'other_num', 'id__count')) + self.assertEqual( + values._fields, + ('num2', 'id', 'num', 'other_num', 'another_num', 'id__count'), + ) self.assertEqual(values.num, 72) self.assertEqual(values.num2, 73) self.assertEqual(values.id__count, 1) @@ -2855,6 +2858,18 @@ class ExcludeTests(TestCase): self.r1.delete() self.assertFalse(qs.exists()) + def test_exclude_nullable_fields(self): + number = Number.objects.create(num=1, other_num=1) + Number.objects.create(num=2, other_num=2, another_num=2) + self.assertSequenceEqual( + Number.objects.exclude(other_num=F('another_num')), + [number], + ) + self.assertSequenceEqual( + Number.objects.exclude(num=F('another_num')), + [number], + ) + class ExcludeTest17600(TestCase): """