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

@@ -17,3 +17,10 @@ class Meta:
 
  def __str__(self):
  return str(self.num)
+
+
+class NullableJSONModel(models.Model):
+ value = models.JSONField(blank=True, null=True)
+
+ class Meta:
+ required_db_features = {"supports_json_field"}

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,72 @@ def test_eq_and_in(self):
  "lookup__book",
  [{"$match": {"$and": [{"isbn": {"$in": ("12345", "56789")}}, {"title": "Moby Dick"}]}}],
  )
+
+
+class NullValueLookupTests(MongoTestCaseMixin, TestCase):
+ _OPERATOR_PREDICATE_MAP = {
+ "exact": lambda field: {field: None},
+ "in": lambda field: {field: {"$in": [None]}},
+ }
+
+ @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.null_objs.append(NullableJSONModel.objects.create(value={"name": None}))
+ cls.unique_id = ObjectId()
+
+ def _test_none_filter_nullable_json(self, op, predicate, field):
+ with self.assertNumQueries(1) as ctx:
+ self.assertQuerySetEqual(
+ NullableJSONModel.objects.filter(
+ **{f"{field}__{op}": [None] if op == "in" else None}
+ ),
+ [],
+ )
+ self.assertAggregateQuery(
+ ctx.captured_queries[0]["sql"],
+ "lookup__nullablejsonmodel",
+ [{"$match": {"$and": [{"$exists": False}, predicate(field)]}}],
+ )
+
+ def _test_none_filter_binary_operator(self, op, predicate, field):
+ with self.assertNumQueries(1) as ctx:
+ self.assertQuerySetEqual(
+ Book.objects.filter(**{f"{field}__{op}": [None] if op == "in" else None}), []
+ )
+ self.assertAggregateQuery(
+ ctx.captured_queries[0]["sql"],
+ "lookup__book",
+ [
+ {
+ "$match": {
+ "$or": [
+ {"$and": [{field: {"$exists": True}}, predicate(field)]},
+ {"$expr": {"$eq": [{"$type": f"${field}"}, "missing"]}},
+ ]
+ }
+ }
+ ],
+ )
+
+ def _test_with_raw_data(self, model, test_function, field):
+ collection = connection.database.get_collection(model._meta.db_table)
+ try:
+ collection.insert_one({"_id": self.unique_id})
+
+ for op, predicate in self._OPERATOR_PREDICATE_MAP.items():
+ with self.subTest(op=op):
+ test_function(op, predicate, field)
+
+ finally:
+ collection.delete_one({"_id": self.unique_id})
+
+ def test_none_filter_nullable_json(self):
+ self._test_with_raw_data(NullableJSONModel, self._test_none_filter_nullable_json, "value")
+
+ def test_none_filter_binary_operator(self):
+ self._test_with_raw_data(Book, self._test_none_filter_binary_operator, "title")