INTPYTHON-1082 [Django] Indexes declared with ordering expressions are dropped or crash migrate

django_mongodb_backend/constraints.py

@@ -68,7 +68,9 @@ def get_pymongo_index_model(self, model, schema_editor, field=None, column_prefi
     kwargs = {}
     filter_expression = defaultdict(dict)
     if self.condition:
-        filter_expression.update(self._get_condition_mql(model, schema_editor))
+        filter_expression.update(
+            self._get_condition_mql(model, schema_editor, column_prefix=column_prefix)
+        )
     if self.nulls_distinct is None or self.nulls_distinct:
         # Indexing on $type matches the value of most SQL databases by allowing
         # multiple null values for the unique constraint. nulls_distinct will
@@ -82,7 +84,7 @@ def get_pymongo_index_model(self, model, schema_editor, field=None, column_prefi
         else:
             for field_name in self.fields:
                 field_ = get_field(model, field_name)
-                filter_expression[field_.column].update(
+                filter_expression[column_prefix + field_.column].update(
                     _get_partial_unique_filter(field_.field, schema_editor.connection)
                 )
     if filter_expression:

django_mongodb_backend/features.py

@@ -32,6 +32,7 @@ class DatabaseFeatures(GISFeatures, BaseDatabaseFeatures):
     supports_deferrable_unique_constraints = False
     supports_explaining_query_execution = True
     supports_expression_defaults = False
+    # Only ordering expressions (F, OrderBy) translate into MongoDB index keys.
     supports_expression_indexes = False
     supports_foreign_keys = False
     supports_frame_range_fixed_distance = True
@@ -150,6 +151,9 @@ def django_test_expected_failures(self):
             "migrations.test_operations.OperationTests.test_rename_field_index_together",
             "migrations.test_operations.OperationTests.test_rename_field_unique_together",
         },
+        "MongoDB creates F()/OrderBy() expression indexes.": {
+            "schema.tests.SchemaTests.test_func_index_unsupported",
+        },
         "AutoField not supported.": {
             "bulk_create.tests.BulkCreateTests.test_bulk_insert_nullable_fields",
             "introspection.tests.IntrospectionTests.test_sequence_list",

django_mongodb_backend/indexes.py

@@ -6,7 +6,8 @@
 from django.core.exceptions import FieldDoesNotExist
 from django.db import NotSupportedError
 from django.db.backends.utils import names_digest, split_identifier
-from django.db.models import FloatField, Index, IntegerField
+from django.db.models import F, FloatField, Index, IntegerField
+from django.db.models.expressions import OrderBy
 from django.db.models.lookups import BuiltinLookup
 from django.db.models.sql.query import Query
 from django.db.models.sql.where import AND, XOR, WhereNode
@@ -29,12 +30,34 @@
 FieldColumn = namedtuple("FieldColumn", ["field", "column"])
 
 
-def _get_condition_mql(self, model, schema_editor):
+def _prefix_mql_field_paths(mql, column_prefix):
+    """
+    Prepend column_prefix to each field path in the MQL dict. Only logical
+    operator values ($and/$or) nest field paths; operator dicts ($gte, $in,
+    ...) map operators to values that never contain field paths.
+    """
+    prefixed = {}
+    for key, value in mql.items():
+        if key.startswith("$"):
+            if isinstance(value, list):
+                value = [_prefix_mql_field_paths(item, column_prefix) for item in value]
+        else:
+            key = column_prefix + key
+        prefixed[key] = value
+    return prefixed
+
+
+def _get_condition_mql(self, model, schema_editor, column_prefix=""):
     """Analogous to Index._get_condition_sql()."""
     query = Query(model=model, alias_cols=False)
     where = query.build_where(self.condition)
     compiler = query.get_compiler(connection=schema_editor.connection)
-    return where.as_mql_idx(compiler, schema_editor.connection)
+    mql = where.as_mql_idx(compiler, schema_editor.connection)
+    if column_prefix:
+        # The condition's field paths are relative to `model`; an embedded
+        # model's index stores its data under the prefixed paths.
+        return _prefix_mql_field_paths(mql, column_prefix)
+    return mql
 
 
 def builtin_lookup_idx(self, compiler, connection):
@@ -83,20 +106,87 @@ def get_field(model, field_name):
     return FieldColumn(field, ".".join(path))
 
 
+def expression_index_orders(expressions, model):
+    """
+    Return the MongoDB index keys for expressions as (column, direction)
+    tuples, or None if they can't be translated.
+    """
+    orders = []
+    for expression in expressions:
+        direction = ASCENDING
+        if isinstance(expression, OrderBy):
+            # MongoDB index keys can't express NULLS FIRST/LAST.
+            if expression.nulls_first or expression.nulls_last:
+                return None
+            direction = DESCENDING if expression.descending else ASCENDING
+            expression = expression.expression
+        # An F subclass (e.g. Sliced) would silently index the entire field.
+        if type(expression) is not F:
+            return None
+        try:
+            column = get_field(model, expression.name).column
+        except FieldDoesNotExist:
+            # e.g. F() with a transform ("name__lower") or nonexistent field.
+            return None
+        orders.append((column, direction))
+    return orders
+
+
+def index_may_exist(index, model):
+    """Return whether this index could exist on the server."""
+    return expression_index_orders(index.expressions, model) is not None
+
+
+_django_index_check = Index.check
+
+
+def check(self, model, connection):
+    """
+    Wrap Index.check() to keep the models.W043 warning in sync with
+    whether this index is actually created.
+    """
+    if connection.vendor != "mongodb":
+        # register_indexes() patches Index.check globally, so this wrapper
+        # also runs for other backends in multi-database projects. Leave
+        # their results alone.
+        return _django_index_check(self, model, connection)
+    errors = _django_index_check(self, model, connection)
+    if index_may_exist(self, model):
+        # Created: W043 is a false positive from Django's model-level emission.
+        return [e for e in errors if e.id != "models.W043"]
+    # Skipped: Django never emits W043 for subclasses that report
+    # contains_expressions=False, so add it.
+    if "supports_expression_indexes" not in model._meta.required_db_features and not any(
+        e.id == "models.W043" for e in errors
+    ):
+        errors.append(
+            Warning(
+                f"{connection.display_name} does not support indexes on expressions.",
+                hint=(
+                    "An index won't be created. Silence this warning if you don't care about it."
+                ),
+                obj=model,
+                id="models.W043",
+            )
+        )
+    return errors
+
+
 def get_pymongo_index_model(self, model, schema_editor, field=None, column_prefix=""):
     """Return a pymongo IndexModel for this Django Index."""
-    if self.contains_expressions:
-        return None
-    kwargs = {}
-    filter_expression = defaultdict(dict)
-    if self.condition:
-        filter_expression.update(self._get_condition_mql(model, schema_editor))
-    if filter_expression:
-        kwargs["partialFilterExpression"] = filter_expression
-    index_orders = (
-        [(column_prefix + field.column, ASCENDING)]
-        if field
-        else [
+    if field:
+        index_orders = [(column_prefix + field.column, ASCENDING)]
+    elif self.expressions:
+        # Check self.expressions rather than self.contains_expressions: some
+        # Index subclasses report contains_expressions=False.
+        index_orders = expression_index_orders(self.expressions, model)
+        if index_orders is None:
+            # Skip untranslatable expressions before translating the
+            # condition, which may raise for its own unsupported lookups.
+            return None
+        index_orders = [(column_prefix + column, direction) for column, direction in index_orders]
+    else:
+        index_orders = [
             # order is "" if ASCENDING or "DESC" if DESCENDING (see
             # django.db.models.indexes.Index.fields_orders).
             (
@@ -105,7 +195,14 @@ def get_pymongo_index_model(self, model, schema_editor, field=None, column_prefi
             )
             for field_name, order in self.fields_orders
         ]
-    )
+    kwargs = {}
+    filter_expression = defaultdict(dict)
+    if self.condition:
+        filter_expression.update(
+            self._get_condition_mql(model, schema_editor, column_prefix=column_prefix)
+        )
+    if filter_expression:
+        kwargs["partialFilterExpression"] = filter_expression
     return IndexModel(index_orders, name=self.name, **kwargs)
 
 
@@ -461,5 +558,6 @@ def get_pymongo_index_model(self, model, schema_editor, field=None, column_prefi
 def register_indexes():
     BuiltinLookup.as_mql_idx = builtin_lookup_idx
     Index._get_condition_mql = _get_condition_mql
+    Index.check = check
     Index.get_pymongo_index_model = get_pymongo_index_model
     WhereNode.as_mql_idx = where_node_idx

django_mongodb_backend/schema.py

@@ -3,9 +3,10 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.db.backends.base.schema import BaseDatabaseSchemaEditor
 from django.db.models import Index, UniqueConstraint
+from pymongo.errors import OperationFailure
 from pymongo.operations import SearchIndexModel
 
-from django_mongodb_backend.indexes import SearchIndex
+from django_mongodb_backend.indexes import SearchIndex, index_may_exist
 
 from .fields import EmbeddedModelField
 from .gis.schema import GISSchemaEditor
@@ -220,7 +221,7 @@ def _remove_model_indexes(self, model, column_prefix="", parent_model=None):
             self.remove_constraint(parent_model or model, constraint)
         # Meta.indexes
         for index in model._meta.indexes:
-            self.remove_index(parent_model or model, index)
+            self.remove_index(parent_model or model, index, resolve_model=model)
 
     @ignore_embedded_models
     def alter_index_together(self, model, old_index_together, new_index_together, column_prefix=""):
@@ -290,16 +291,25 @@ def _add_field_index(self, model, field, *, column_prefix=""):
         self.add_index(model, index, field=field, column_prefix=column_prefix)
 
     @ignore_embedded_models
-    def remove_index(self, model, index):
-        if index.contains_expressions:
-            return
+    def remove_index(self, model, index, resolve_model=None):
         collection = self.get_collection(model._meta.db_table)
         if isinstance(index, SearchIndex):
             # Drop the index if it's supported.
             if self.connection.features.supports_search:
                 collection.drop_search_index(index.name)
                 self.wait_until_index_dropped(collection, index.name)
+        elif index.expressions:
+            # Expression indexes may not exist; their expressions resolve
+            # against the embedded model that declared the index.
+            if index_may_exist(index, resolve_model or model):
+                try:
+                    collection.drop_index(index.name)
+                except OperationFailure as e:
+                    if e.code != 27:  # IndexNotFound
+                        raise
         else:
+            # Field-only indexes are always created by add_index(), so
+            # IndexNotFound is an error worth surfacing.
             collection.drop_index(index.name)
 
     def _remove_composed_index(

docs/releases/6.0.x.rst

@@ -2,6 +2,28 @@
 Django MongoDB Backend 6.0.x
 ============================
 
+6.0.5
+=====
+
+*Unreleased*
+
+Bug fixes
+---------
+
+- Index definitions using ordering expressions (e.g.
+  ``Index(F("priority").desc(), name="...")``) are now created instead of
+  being silently skipped. Expressions that can't be translated into MongoDB
+  index keys (anything other than :class:`~django.db.models.F` and
+  :class:`~django.db.models.OrderBy` field orderings) are skipped without
+  raising an error.
+
+- Fixed a ``migrate`` crash when an ``Index`` subclass reports
+  ``contains_expressions=False`` for an index with expressions.
+
+- Indexes that earlier versions skipped aren't retroactively created by
+  already-applied ``AddIndex`` migrations: create a new migration or add the
+  index manually on upgraded deployments.
+
 6.0.4
 =====
 

tests/indexes_/test_base.py

@@ -1,4 +1,16 @@
 from django.db import connection
+from django.db.models import Index
+
+
+class ConditionalPartialIndex(Index):
+    """Mimics django-tasks-db's ConditionalPartialIndex.
+
+    Reports contains_expressions=False so backends still create it.
+    """
+
+    @property
+    def contains_expressions(self):
+        return False
 
 
 class SchemaAssertionMixin: