INTPYTHON-749: Fix bug in null matching on queryoptimizer

django_mongodb_backend/query_conversion/expression_converters.py

@@ -44,8 +44,12 @@ def convert(cls, args):
             ):
                 field_name = field_expr[1:]  # Remove the $ prefix.
                 if cls.operator == "$eq":
-                    return {field_name: value}
-                return {field_name: {cls.operator: value}}
+                    query = {field_name: value}
+                else:
+                    query = {field_name: {cls.operator: value}}
+                if value is None:
+                    query = {"$and": [{field_name: {"$exists": True}}, query]}
+                return query
         return None
 
 
@@ -102,7 +106,12 @@ def convert(cls, in_args):
                 if isinstance(values, (list, tuple, set)) and all(
                     cls.is_simple_value(v) for v in values
                 ):
-                    return {field_name: {"$in": values}}
+                    core_check = {field_name: {"$in": values}}
+                    return (
+                        {"$and": [{field_name: {"$exists": True}}, core_check]}
+                        if None in values
+                        else core_check
+                    )
         return None
 
 

tests/expression_converter_/test_op_expressions.py

@@ -7,6 +7,12 @@
 from django_mongodb_backend.query_conversion.expression_converters import convert_expression
 
 
+def _wrap_condition_if_null(_type, condition, path):
+    if _type is None:
+        return {"$and": [{path: {"$exists": True}}, condition]}
+    return condition
+
+
 class ConversionTestCase(SimpleTestCase):
     CONVERTIBLE_TYPES = {
         "int": 42,
@@ -53,10 +59,14 @@ def test_no_conversion_dict_value(self):
         self.assertNotOptimizable({"$eq": ["$status", {"$gt": 5}]})
 
     def _test_conversion_valid_type(self, _type):
-        self.assertConversionEqual({"$eq": ["$age", _type]}, {"age": _type})
+        self.assertConversionEqual(
+            {"$eq": ["$age", _type]}, _wrap_condition_if_null(_type, {"age": _type}, "age")
+        )
 
     def _test_conversion_valid_array_type(self, _type):
-        self.assertConversionEqual({"$eq": ["$age", _type]}, {"age": _type})
+        self.assertConversionEqual(
+            {"$eq": ["$age", _type]}, _wrap_condition_if_null(_type, {"age": _type}, "age")
+        )
 
     def test_conversion_various_types(self):
         self._test_conversion_various_types(self._test_conversion_valid_type)
@@ -78,7 +88,10 @@ def test_no_conversion_dict_value(self):
         self.assertNotOptimizable({"$in": ["$status", [{"bad": "val"}]]})
 
     def _test_conversion_valid_type(self, _type):
-        self.assertConversionEqual({"$in": ["$age", [_type]]}, {"age": {"$in": [_type]}})
+        self.assertConversionEqual(
+            {"$in": ["$age", [_type]]},
+            _wrap_condition_if_null(_type, {"age": {"$in": [_type]}}, "age"),
+        )
 
     def test_conversion_various_types(self):
         for _type, val in self.CONVERTIBLE_TYPES.items():
@@ -170,7 +183,10 @@ def test_no_conversion_dict_value(self):
         self.assertNotOptimizable({"$gt": ["$price", {}]})
 
     def _test_conversion_valid_type(self, _type):
-        self.assertConversionEqual({"$gt": ["$price", _type]}, {"price": {"$gt": _type}})
+        self.assertConversionEqual(
+            {"$gt": ["$price", _type]},
+            _wrap_condition_if_null(_type, {"price": {"$gt": _type}}, "price"),
+        )
 
     def test_conversion_various_types(self):
         self._test_conversion_various_types(self._test_conversion_valid_type)
@@ -193,7 +209,7 @@ def test_no_conversion_dict_value(self):
     def _test_conversion_valid_type(self, _type):
         expr = {"$gte": ["$price", _type]}
         expected = {"price": {"$gte": _type}}
-        self.assertConversionEqual(expr, expected)
+        self.assertConversionEqual(expr, _wrap_condition_if_null(_type, expected, "price"))
 
     def test_conversion_various_types(self):
         self._test_conversion_various_types(self._test_conversion_valid_type)
@@ -210,7 +226,10 @@ def test_no_conversion_dict_value(self):
         self.assertNotOptimizable({"$lt": ["$price", {}]})
 
     def _test_conversion_valid_type(self, _type):
-        self.assertConversionEqual({"$lt": ["$price", _type]}, {"price": {"$lt": _type}})
+        self.assertConversionEqual(
+            {"$lt": ["$price", _type]},
+            _wrap_condition_if_null(_type, {"price": {"$lt": _type}}, "price"),
+        )
 
     def test_conversion_various_types(self):
         self._test_conversion_various_types(self._test_conversion_valid_type)
@@ -227,7 +246,10 @@ def test_no_conversion_dict_value(self):
         self.assertNotOptimizable({"$lte": ["$price", {}]})
 
     def _test_conversion_valid_type(self, _type):
-        self.assertConversionEqual({"$lte": ["$price", _type]}, {"price": {"$lte": _type}})
+        self.assertConversionEqual(
+            {"$lte": ["$price", _type]},
+            _wrap_condition_if_null(_type, {"price": {"$lte": _type}}, "price"),
+        )
 
     def test_conversion_various_types(self):
         self._test_conversion_various_types(self._test_conversion_valid_type)

tests/lookup_/models.py

@@ -6,7 +6,7 @@ class Book(models.Model):
     isbn = models.CharField(max_length=13)
 
     def __str__(self):
-        return self.title
+        return self.title or "Title Not Found"
 
 
 class Number(models.Model):
@@ -17,3 +17,7 @@ class Meta:
 
     def __str__(self):
         return str(self.num)
+
+
+class NullableJSONModel(models.Model):
+    value = models.JSONField(blank=True, null=True)

tests/lookup_/tests.py

@@ -1,8 +1,10 @@
+from bson import ObjectId
+from django.db import connection
 from django.test import TestCase
 
 from django_mongodb_backend.test import MongoTestCaseMixin
 
-from .models import Book, Number
+from .models import Book, NullableJSONModel, Number
 
 
 class NumericLookupTests(TestCase):
@@ -66,3 +68,77 @@ def test_eq_and_in(self):
             "lookup__book",
             [{"$match": {"$and": [{"isbn": {"$in": ("12345", "56789")}}, {"title": "Moby Dick"}]}}],
         )
+
+
+class NullValueLookupTests(MongoTestCaseMixin, TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.book_objs = Book.objects.bulk_create(
+            Book(title=f"Book {i}", isbn=str(i)) for i in range(5)
+        )
+
+        cls.null_objs = NullableJSONModel.objects.bulk_create(NullableJSONModel() for _ in range(5))
+        cls.unique_id = ObjectId()
+        for model in (Book, NullableJSONModel):
+            collection = connection.database.get_collection(model._meta.db_table)
+            collection.insert_one({"_id": cls.unique_id})
+
+    def test_none_filter_nullable_json_exact(self):
+        with self.assertNumQueries(1) as ctx:
+            list(NullableJSONModel.objects.filter(value=None))
+        self.assertAggregateQuery(
+            ctx.captured_queries[0]["sql"],
+            "lookup__nullablejsonmodel",
+            [{"$match": {"$and": [{"value": {"$exists": True}}, {"value": None}]}}],
+        )
+        self.assertQuerySetEqual(
+            NullableJSONModel.objects.filter(value=None), self.null_objs[:-1], ordered=False
+        )
+
+    def test_none_filter_nullable_json_in(self):
+        with self.assertNumQueries(1) as ctx:
+            objs = list(NullableJSONModel.objects.filter(value__in=[None]))
+        self.assertAggregateQuery(
+            ctx.captured_queries[0]["sql"],
+            "lookup__nullablejsonmodel",
+            [{"$match": {"$and": [{"value": {"$exists": True}}, {"value": {"$in": [None]}}]}}],
+        )
+        self.assertQuerySetEqual(objs, self.null_objs[:-1], ordered=False)
+
+    def test_none_filter_binary_operator_exact(self):
+        with self.assertNumQueries(1) as ctx:
+            list(Book.objects.filter(title=None))
+        self.assertAggregateQuery(
+            ctx.captured_queries[0]["sql"],
+            "lookup__book",
+            [
+                {
+                    "$match": {
+                        "$or": [
+                            {"$and": [{"title": {"$exists": True}}, {"title": None}]},
+                            {"$expr": {"$eq": [{"$type": "$title"}, "missing"]}},
+                        ]
+                    }
+                }
+            ],
+        )
+        self.assertQuerySetEqual(Book.objects.filter(title=None), [], ordered=False)
+
+    def test_none_filter_binary_operator_in(self):
+        with self.assertNumQueries(1) as ctx:
+            list(Book.objects.filter(title__in=[None]))
+        self.assertAggregateQuery(
+            ctx.captured_queries[0]["sql"],
+            "lookup__book",
+            [
+                {
+                    "$match": {
+                        "$or": [
+                            {"$and": [{"title": {"$exists": True}}, {"title": None}]},
+                            {"$expr": {"$eq": [{"$type": "$title"}, "missing"]}},
+                        ]
+                    }
+                }
+            ],
+        )
+        self.assertQuerySetEqual(Book.objects.filter(title__in=[None]), [], ordered=False)