comment edits

Author: timgraham

README.md

@@ -138,7 +138,16 @@ Migrations for 'admin':
 - The `tzinfo` parameter of the `Trunc` database functions doesn't work
   properly because MongoDB converts the result back to UTC.
 
-- In `jsonfield`, when a binary operator (like `exact`) is used, the behavior may differ from other backends if either operand (left-hand or right-hand side) is `null`.
+- When querying `JSONField`:
+  - There is no way to distinguish between a JSON "null" (represented by
+    `Value(None, JSONField())`) and a SQL null (queried using the `isnull`
+    lookup). Both of these queries return both of these nulls.
+  - Some queries with `Q` objects, e.g. `Q(value__foo="bar")`, don't work
+    properly, particularly with `QuerySet.exclude()`.
+  - Filtering for a `None` key, e.g. `QuerySet.filter(value__j=None)`
+    incorrectly returns objects where the key doesn't exist.
+  - You can study the skipped tests in `DatabaseFeatures.django_test_skips` for
+    more details on known issues.
 
 ## Troubleshooting
 

django_mongodb/features.py

@@ -417,11 +417,21 @@ def django_test_expected_failures(self):
             "db_functions.comparison.test_cast.CastTests.test_cast_from_python_to_datetime",
             "db_functions.comparison.test_cast.CastTests.test_cast_to_duration",
         },
-        "MongoDB's null behavior is different from SQL's.": {
+        "Known issue querying JSONField.": {
+            # An ExpressionWrapper annotation with KeyTransform followed by
+            # .filter(expr__isnull=False) doesn't use KeyTransformIsNull as it
+            # needs to.
             "model_fields.test_jsonfield.TestQuerying.test_expression_wrapper_key_transform",
+            # There is no way to distinguish between a JSON "null" (represented
+            # by Value(None, JSONField())) and a SQL null (queried using the
+            # isnull lookup). Both of these queries return both nulls.
             "model_fields.test_jsonfield.TestSaveLoad.test_json_null_different_from_sql_null",
+            # Some queries with Q objects, e.g. Q(value__foo="bar"), don't work
+            # properly, particularly with QuerySet.exclude().
             "model_fields.test_jsonfield.TestQuerying.test_lookup_exclude",
             "model_fields.test_jsonfield.TestQuerying.test_lookup_exclude_nonexistent_key",
+            # Queries like like QuerySet.filter(value__j=None) incorrectly
+            # returns objects where the key doesn't exist.
             "model_fields.test_jsonfield.TestQuerying.test_none_key",
             "model_fields.test_jsonfield.TestQuerying.test_none_key_exclude",
         },

django_mongodb/fields/json.py

@@ -26,40 +26,30 @@ def data_contains(self, compiler, connection):  # noqa: ARG001
 
 
 def _has_key_predicate(path, root_column, negated=False):
-    result = {"$and": [{"$ne": [{"$type": path}, "missing"]}, {"$ne": [root_column, None]}]}
+    """Return MQL to check for the existence of `path`."""
+    result = {
+        "$and": [
+            # The path must exist (i.e. not be "missing").
+            {"$ne": [{"$type": path}, "missing"]},
+            # If the JSONField value is None, an additional check for not null
+            # is needed since $type returns null instead of "missing".
+            {"$ne": [root_column, None]},
+        ]
+    }
     if negated:
         result = {"$not": result}
     return result
 
 
 def has_key_lookup(self, compiler, connection):
-    """
-    Performs a key lookup in a JSONField to check for the existence of a key.
-
-    The key lookup in a JSONField is defined by the following conditions:
-    1. The path type must be different from "missing".
-    2. In cases where the JSONField is None (as the $type returns Null instead of "missing"),
-       an additional check for not null is added.
-
-    Returns:
-        A dictionary representing the MongoDB query for key lookup.
-
-    Raises:
-        AssertionError: If `self.mongo_operator` is None and there are multiple keys.
-
-    Note:
-        The function handles key lookup in two ways:
-        - Via `KeyTransform`
-        - Directly via key
-
-        Both are transformed into a `KeyTransform` for consistent handling.
-    """
+    """Return MQL to check for the existence of a key."""
     rhs = self.rhs
     lhs = process_lhs(self, compiler, connection)
     if not isinstance(rhs, list | tuple):
         rhs = [rhs]
     paths = []
-    # Transform all keys into KeyTransform instances for consistent handling
+    # Transform any "raw" keys into KeyTransforms to allow consistent handling
+    # in the code that follows.
     for key in rhs:
         rhs_json_path = key if isinstance(key, KeyTransform) else KeyTransform(key, self.lhs)
         paths.append(rhs_json_path.as_mql(compiler, connection))
@@ -85,19 +75,12 @@ def json_exact_process_rhs(self, compiler, connection):
 
 def key_transform(self, compiler, connection):
     """
-    Transforms a Django KeyTransform (JSON path) into a MongoDB query path.
-
-    In MongoDB, JSON paths cannot always be represented simply as $var.key1.key2.key3
-    due to potential array types. Therefore, indexing arrays requires the use of
-    `arrayElemAt`. Additionally, a conditional check (if statement) is necessary
-    to verify the type before performing the operation.
-
-    Returns:
-        A dictionary representing the MongoDB query for the JSON path.
+    Return MQL for this KeyTransform (JSON path).
 
-    Note:
-        The function constructs the MongoDB path by iterating through the key transforms
-        and handling both field access and array indexing appropriately.
+    JSON paths cannot always be represented simply as $var.key1.key2.key3 due
+    to possible array types. Therefore, indexing arrays requires the use of
+    `arrayElemAt`. Additionally, $cond is necessary to verify the type before
+    performing the operation.
     """
     key_transforms = [self.key_name]
     previous = self.lhs
@@ -107,11 +90,11 @@ def key_transform(self, compiler, connection):
         previous = previous.lhs
     lhs_mql = previous.as_mql(compiler, connection)
     result = lhs_mql
-    # Build the MongoDB path using the collected key transforms
+    # Build the MQL path using the collected key transforms.
     for key in key_transforms:
         get_field = {"$getField": {"input": result, "field": key}}
-        # Handle array indexing if the key is a digit
-        # if we have an index like '001' is not an array index.
+        # Handle array indexing if the key is a digit. If key is something
+        # like '001', it's not an array index despite isdigit() returning True.
         if key.isdigit() and str(int(key)) == key:
             result = {
                 "$cond": {
@@ -127,54 +110,33 @@ def key_transform(self, compiler, connection):
 
 def key_transform_in(self, compiler, connection):
     """
-    Checks if a JSON path's values are within a set of specified values and ensures the key exists.
-
-    This function performs two main checks:
-    1. Verifies that the resulting path values are within the right-hand side (rhs) values.
-    2. Ensures that the key exists.
-
-    Returns:
-        A dictionary representing the MongoDB query that checks both conditions.
-
-    Note:
-        The function processes the left-hand side (lhs) and right-hand side (rhs)
-        of the query, constructs the MongoDB expression, and checks both the value
-        existence in rhs and the key existence in the JSON document.
+    Return MQL to check if a JSON path exists and that its values are in the
+    set of specified values (rhs).
     """
     lhs_mql = process_lhs(self, compiler, connection)
-    # Traverse to the root column
+    # Traverse to the root column.
     previous = self.lhs
     while isinstance(previous, KeyTransform):
         previous = previous.lhs
     root_column = previous.as_mql(compiler, connection)
     value = process_rhs(self, compiler, connection)
-    # Construct the expression to check if lhs_mql values are in rhs values
+    # Construct the expression to check if lhs_mql values are in rhs values.
     expr = connection.mongo_operators[self.lookup_name](lhs_mql, value)
     return {"$and": [_has_key_predicate(lhs_mql, root_column), expr]}
 
 
 def key_transform_is_null(self, compiler, connection):
     """
-    Handles the KeyTransformIsNull lookup.
-
-    This function borrows logic from HasKey for `isnull=False`. If `isnull=True`,
-    the query should match only objects that do not have the key.
-
-    Returns:
-        A dictionary representing the MongoDB query for checking the nullability of the key.
+    Return MQL to check the nullability of a key.
 
-    Note:
-        If `isnull=True`, the query matches objects where the key is missing
-                          or the root column is null.
-        If `isnull=False`, the query negates the result to match objects where the key exists
-                           and the root column isn't null.
+    If `isnull=True`, the query matches objects where the key is missing or the
+    root column is null. If `isnull=False`, the query negates the result to
+    match objects where the key exists.
 
-    Reference:
-        https://code.djangoproject.com/ticket/32252
+    Reference: https://code.djangoproject.com/ticket/32252
     """
     lhs_mql = process_lhs(self, compiler, connection)
     rhs_mql = process_rhs(self, compiler, connection)
-
     # Get the root column.
     previous = self.lhs
     while isinstance(previous, KeyTransform):
@@ -185,23 +147,12 @@ def key_transform_is_null(self, compiler, connection):
 
 def key_transform_numeric_lookup_mixin(self, compiler, connection):
     """
-    Checks if the field exists and fulfills the given numeric lookup expression.
-
-    This function ensures that the field in the JSON document:
-    1. Exists (i.e., is not "missing" or "null").
-    2. Satisfies the specified numeric lookup expression.
-
-    Returns:
-        A dictionary representing the MongoDB query that checks both conditions.
-
-    Note:
-        The function constructs the MongoDB expression to check if the field exists
-        and fulfills the numeric lookup expression, and combines these conditions
-        using $and.
+    Return MQL to check if the field exists (i.e., is not "missing" or "null")
+    and that the field matches the given numeric lookup expression.
     """
     expr = builtin_lookup(self, compiler, connection)
     lhs = process_lhs(self, compiler, connection)
-    # Check if the type of lhs is not "missing" or "null"
+    # Check if the type of lhs is not "missing" or "null".
     not_missing_or_null = {"$not": {"$in": [{"$type": lhs}, ["missing", "null"]]}}
     return {"$and": [expr, not_missing_or_null]}