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

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
@@ -83,20 +84,91 @@ 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. An expression is
+    translatable only if it's a plain F() that resolves to an existing
+    model field, optionally wrapped in OrderBy() without NULLS FIRST/LAST.
+    """
+    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
+        # Subclasses such as Sliced (F("headline")[1:5]) or OuterRef carry
+        # semantics that MongoDB index keys can't express; translating
+        # them 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 +177,12 @@ 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))
+    if filter_expression:
+        kwargs["partialFilterExpression"] = filter_expression
     return IndexModel(index_orders, name=self.name, **kwargs)
 
 
@@ -461,5 +538,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:

tests/indexes_/test_checks.py

@@ -2,9 +2,12 @@
 
 from django.core import checks
 from django.db import connection, models
+from django.db.models import F, Index
+from django.db.models.functions import Lower
 from django.test import TestCase
 from django.test.utils import isolate_apps
 
+import django_mongodb_backend.indexes
 from django_mongodb_backend.fields import ArrayField, ObjectIdField
 from django_mongodb_backend.indexes import (
     EmbeddedFieldIndex,
@@ -14,6 +17,17 @@
 from django_mongodb_backend.models import EmbeddedModel
 
 from .models import DataHolder, Movie, Owner, Person, Store
+from .test_base import ConditionalPartialIndex
+
+
+def w043_warning(model):
+    """Return the expected models.W043 warning for model."""
+    return checks.Warning(
+        "MongoDB 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",
+    )
 
 
 @isolate_apps("indexes_")
@@ -335,3 +349,94 @@ def test_nonexistent_nested_polymorphic_subfield(self):
             errors[0].msg,
             "'indexes' refers to the nonexistent field 'thing.tags.xxx'.",
         )
+
+
+@isolate_apps("indexes_")
+class ExpressionIndexCheckTests(TestCase):
+    def test_no_warning_for_translatable_expressions(self):
+        class Article(models.Model):
+            number = models.IntegerField()
+
+            class Meta:
+                indexes = [Index(F("number").desc(), name="ordering_idx")]
+
+        self.assertEqual(Article.check(databases={"default"}), [])
+
+    def test_warning_for_untranslatable_expressions(self):
+        class Article(models.Model):
+            headline = models.CharField(max_length=100)
+
+            class Meta:
+                indexes = [Index(Lower("headline"), name="lower_idx")]
+
+        self.assertEqual(
+            Article.check(databases={"default"}),
+            [w043_warning(Article)],
+        )
+
+    def test_warning_for_sliced_expression(self):
+        # Sliced subclasses F, but the slice has no MongoDB index-key
+        # equivalent: the index is skipped and W043 fires rather than
+        # silently creating an index on the entire field.
+        class Article(models.Model):
+            headline = models.CharField(max_length=100)
+
+            class Meta:
+                indexes = [Index(F("headline")[1:5], name="slice_idx")]
+
+        self.assertEqual(
+            Article.check(databases={"default"}),
+            [w043_warning(Article)],
+        )
+
+    def test_warning_for_untranslatable_expressions_in_subclass(self):
+        # contains_expressions=False doesn't stop the skip, so W043 fires.
+        class Article(models.Model):
+            headline = models.CharField(max_length=100)
+
+            class Meta:
+                indexes = [ConditionalPartialIndex(Lower("headline"), name="lower_idx")]
+
+        self.assertEqual(
+            Article.check(databases={"default"}),
+            [w043_warning(Article)],
+        )
+
+    def test_mixed_translatable_and_untranslatable_indexes(self):
+        class Article(models.Model):
+            number = models.IntegerField()
+            headline = models.CharField(max_length=100)
+
+            class Meta:
+                indexes = [
+                    Index(F("number").desc(), name="ordering_idx"),
+                    Index(Lower("headline"), name="lower_idx"),
+                ]
+
+        self.assertEqual(
+            Article.check(databases={"default"}),
+            [
+                # Only the untranslatable index warns.
+                w043_warning(Article)
+            ],
+        )
+
+    def test_check_passes_through_on_non_mongodb_connections(self):
+        # The global Index.check patch must leave other backends' results
+        # untouched in multi-database projects (e.g. a PostgreSQL database
+        # supports expression indexes and shouldn't see models.W043).
+        class Article(models.Model):
+            number = models.IntegerField()
+
+            class Meta:
+                indexes = [Index(F("number").desc(), name="ordering_idx")]
+
+        index = Article._meta.indexes[0]
+        other = mock.Mock()
+        other.vendor = "postgresql"
+        with mock.patch.object(
+            django_mongodb_backend.indexes, "_django_index_check", return_value=["sentinel"]
+        ) as django_check:
+            errors = index.check(Article, other)
+        self.assertEqual(errors, ["sentinel"])
+        django_check.assert_called_once_with(index, Article, other)