Skip to content

add support for creating and deleting indexes #125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 3 additions & 34 deletions django_mongodb/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,39 +70,16 @@ class DatabaseFeatures(BaseDatabaseFeatures):
"backends.tests.ThreadTests.test_closing_non_shared_connections",
"backends.tests.ThreadTests.test_default_connection_thread_local",
# AddField
"schema.tests.SchemaTests.test_add_indexed_charfield",
"schema.tests.SchemaTests.test_add_unique_charfield",
# Add/RemoveIndex
"migrations.test_operations.OperationTests.test_add_index",
"migrations.test_operations.OperationTests.test_alter_field_with_index",
"migrations.test_operations.OperationTests.test_remove_index",
"migrations.test_operations.OperationTests.test_rename_index",
"migrations.test_operations.OperationTests.test_rename_index_unknown_unnamed_index",
"migrations.test_operations.OperationTests.test_rename_index_unnamed_index",
"schema.tests.SchemaTests.test_add_remove_index",
"schema.tests.SchemaTests.test_composed_desc_index_with_fk",
"schema.tests.SchemaTests.test_composed_index_with_fk",
"schema.tests.SchemaTests.test_create_index_together",
"schema.tests.SchemaTests.test_order_index",
"schema.tests.SchemaTests.test_text_field_with_db_index",
# AlterField
"schema.tests.SchemaTests.test_alter_field_add_index_to_integerfield",
"schema.tests.SchemaTests.test_alter_field_fk_keeps_index",
"schema.tests.SchemaTests.test_alter_field_fk_to_o2o",
"schema.tests.SchemaTests.test_alter_field_o2o_keeps_unique",
"schema.tests.SchemaTests.test_alter_field_o2o_to_fk",
"schema.tests.SchemaTests.test_alter_int_pk_to_int_unique",
"schema.tests.SchemaTests.test_alter_not_unique_field_to_primary_key",
# AlterField (db_index)
"schema.tests.SchemaTests.test_alter_renames_index",
"schema.tests.SchemaTests.test_indexes",
"schema.tests.SchemaTests.test_remove_db_index_doesnt_remove_custom_indexes",
# AlterField (unique)
"schema.tests.SchemaTests.test_indexes",
"schema.tests.SchemaTests.test_unique",
"schema.tests.SchemaTests.test_unique_and_reverse_m2m",
# alter_index_together
"migrations.test_operations.OperationTests.test_alter_index_together",
"schema.tests.SchemaTests.test_index_together",
# alter_unique_together
"migrations.test_operations.OperationTests.test_alter_unique_together",
"schema.tests.SchemaTests.test_unique_together",
Expand All @@ -114,9 +91,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
"schema.tests.SchemaTests.test_composed_constraint_with_fk",
"schema.tests.SchemaTests.test_remove_ignored_unique_constraint_not_create_fk_index",
"schema.tests.SchemaTests.test_unique_constraint",
# subclasses of BaseDatabaseIntrospection may require a get_constraints() method
"migrations.test_operations.OperationTests.test_add_func_unique_constraint",
"migrations.test_operations.OperationTests.test_remove_func_unique_constraint",
}
# $bitAnd, #bitOr, and $bitXor are new in MongoDB 6.3.
_django_test_expected_failures_bitwise = {
Expand Down Expand Up @@ -220,6 +194,7 @@ def django_test_expected_failures(self):
"get_or_create.tests.GetOrCreateThroughManyToMany.test_something",
"get_or_create.tests.UpdateOrCreateTests.test_manual_primary_key_test",
"get_or_create.tests.UpdateOrCreateTestsWithManualPKs.test_create_with_duplicate_primary_key",
"introspection.tests.IntrospectionTests.test_get_constraints_unique_indexes_orders",
"model_fields.test_filefield.FileFieldTests.test_unique_when_same_filename",
"one_to_one.tests.OneToOneTests.test_multiple_o2o",
"queries.test_bulk_update.BulkUpdateTests.test_database_routing_batch_atomicity",
Expand Down Expand Up @@ -613,14 +588,8 @@ def django_test_expected_failures(self):
"introspection.tests.IntrospectionTests.test_get_table_description_types",
"introspection.tests.IntrospectionTests.test_smallautofield",
},
"DatabaseIntrospection.get_constraints() not implemented.": {
"introspection.tests.IntrospectionTests.test_get_constraints",
"introspection.tests.IntrospectionTests.test_get_constraints_index_types",
"introspection.tests.IntrospectionTests.test_get_constraints_indexes_orders",
"introspection.tests.IntrospectionTests.test_get_constraints_unique_indexes_orders",
"introspection.tests.IntrospectionTests.test_get_primary_key_column",
},
"MongoDB can't introspect primary key.": {
"introspection.tests.IntrospectionTests.test_get_primary_key_column",
"schema.tests.SchemaTests.test_alter_primary_key_the_same_name",
"schema.tests.SchemaTests.test_primary_key",
},
Expand Down
26 changes: 26 additions & 0 deletions django_mongodb/introspection.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,32 @@
from django.db.backends.base.introspection import BaseDatabaseIntrospection
from django.db.models import Index
from pymongo import ASCENDING, DESCENDING


class DatabaseIntrospection(BaseDatabaseIntrospection):
ORDER_DIR = {ASCENDING: "ASC", DESCENDING: "DESC"}

def table_names(self, cursor=None, include_views=False):
return sorted([x["name"] for x in self.connection.database.list_collections()])

def get_constraints(self, cursor, table_name):
indexes = self.connection.get_collection(table_name).index_information()
constraints = {}
for name, details in indexes.items():
# Remove underscore prefix from "_id" columns in primary key index.
if is_primary_key := name == "_id_":
name = "id"
details["key"] = [("id", 1)]
constraints[name] = {
"check": False,
"columns": [field for field, order in details["key"]],
"definition": None,
"foreign_key": None,
"index": True,
"orders": [self.ORDER_DIR[order] for field, order in details["key"]],
"primary_key": is_primary_key,
"type": Index.suffix,
"unique": details.get("unique", False),
"options": {},
}
return constraints
129 changes: 122 additions & 7 deletions django_mongodb/schema.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
from django.db.backends.base.schema import BaseDatabaseSchemaEditor
from django.db.models import Index
from pymongo import ASCENDING, DESCENDING
from pymongo.operations import IndexModel

from .query import wrap_database_errors
from .utils import OperationCollector
Expand All @@ -18,11 +21,30 @@ def get_database(self):
@wrap_database_errors
def create_model(self, model):
self.get_database().create_collection(model._meta.db_table)
self._create_model_indexes(model)
# Make implicit M2M tables.
for field in model._meta.local_many_to_many:
if field.remote_field.through._meta.auto_created:
self.create_model(field.remote_field.through)

def _create_model_indexes(self, model):
"""
Create all indexes (field indexes, index_together, Meta.indexes) for
the specified model.
"""
if not model._meta.managed or model._meta.proxy or model._meta.swapped:
return
# Field indexes
for field in model._meta.local_fields:
if self._field_should_be_indexed(model, field):
self._add_field_index(model, field)
# Meta.index_together (RemovedInDjango51Warning)
for field_names in model._meta.index_together:
self._add_composed_index(model, field_names)
Comment on lines +41 to +43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For users of Django 5.1, the below for loop should include the index_together elements, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support for Meta.index_together is removed in Django 5.1, so these lines will be removed.

Copy link
Contributor

@Jibola Jibola Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, looked over the documentation and see that this was deprecated since 4.2

# Meta.indexes
for index in model._meta.indexes:
self.add_index(model, index)

def delete_model(self, model):
# Delete implicit M2m tables.
for field in model._meta.local_many_to_many:
Expand All @@ -40,6 +62,9 @@ def add_field(self, model, field):
self.get_collection(model._meta.db_table).update_many(
{}, [{"$set": {column: self.effective_default(field)}}]
)
# Add an index, if required.
if self._field_should_be_indexed(model, field):
self._add_field_index(model, field)

def _alter_field(
self,
Expand All @@ -53,15 +78,27 @@ def _alter_field(
strict=False,
):
collection = self.get_collection(model._meta.db_table)
# Removed an index?
old_field_indexed = self._field_should_be_indexed(model, old_field)
new_field_indexed = self._field_should_be_indexed(model, new_field)
if old_field_indexed and not new_field_indexed:
self._remove_field_index(model, old_field)
# Have they renamed the column?
if old_field.column != new_field.column:
collection.update_many({}, {"$rename": {old_field.column: new_field.column}})
# Move index to the new field, if needed.
if old_field_indexed and new_field_indexed:
self._remove_field_index(model, old_field)
self._add_field_index(model, new_field)
Comment on lines +91 to +92
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Add then remove. A dropped index has no further use, but adding one may need to be used quickly.

Copy link
Collaborator Author

@timgraham timgraham Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the scenario you imagine zero-downtime migrations? In that case, I think the developer would be advised to roll out schema changes in advance of the application logic that uses those fields, and thus we shouldn't worry about this sort of optimization.

Otherwise, a site should be in maintenance mode while deploying a new version that requires schema changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, okay. That was the exact case I had been thinking of.

Thought of it as a small optimization but if that's not standard practice I'm fine leaving it as is.

# Replace NULL with the field default if the field and was changed from
# NULL to NOT NULL.
if new_field.has_default() and old_field.null and not new_field.null:
column = new_field.column
default = self.effective_default(new_field)
collection.update_many({column: {"$eq": None}}, [{"$set": {column: default}}])
# Added an index?
if not old_field_indexed and new_field_indexed:
self._add_field_index(model, new_field)

def remove_field(self, model, field):
# Remove implicit M2M tables.
Expand All @@ -71,21 +108,99 @@ def remove_field(self, model, field):
# Unset field on existing documents.
if column := field.column:
self.get_collection(model._meta.db_table).update_many({}, {"$unset": {column: ""}})
if self._field_should_be_indexed(model, field):
self._remove_field_index(model, field)

def alter_index_together(self, model, old_index_together, new_index_together):
pass
olds = {tuple(fields) for fields in old_index_together}
news = {tuple(fields) for fields in new_index_together}
# Deleted indexes
for field_names in olds.difference(news):
self._remove_composed_index(model, field_names, {"index": True, "unique": False})
# Created indexes
for field_names in news.difference(olds):
self._add_composed_index(model, field_names)

def alter_unique_together(self, model, old_unique_together, new_unique_together):
pass

def add_index(self, model, index):
pass

def rename_index(self, model, old_index, new_index):
pass
def add_index(self, model, index, field=None):
if index.contains_expressions:
return
Comment on lines +128 to +129
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not raise a NotSupportedError or what's the tactic for handling indexes with expressions. UUIC these are complex types that exist beyond the standard ASC/DESC indexing capabilities of MongoDB.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are ignored (in case the index is useful on another database backend) as per https://docs.djangoproject.com/en/dev/ref/models/indexes/#django.db.models.Index.expressions.

index_orders = (
[(field.column, ASCENDING)]
if field
else [
# order is "" if ASCENDING or "DESC" if DESCENDING (see
# django.db.models.indexes.Index.fields_orders).
(model._meta.get_field(field_name).column, ASCENDING if order == "" else DESCENDING)
for field_name, order in index.fields_orders
]
)
idx = IndexModel(index_orders, name=index.name)
self.get_collection(model._meta.db_table).create_indexes([idx])

def _add_composed_index(self, model, field_names):
"""Add an index on the given list of field_names."""
idx = Index(fields=field_names)
idx.set_name_with_model(model)
self.add_index(model, idx)

def _add_field_index(self, model, field):
"""Add an index on a field with db_index=True."""
index = Index(fields=[field.name])
index.name = self._create_index_name(model._meta.db_table, [field.column])
self.add_index(model, index, field=field)

def remove_index(self, model, index):
pass
if index.contains_expressions:
return
self.get_collection(model._meta.db_table).drop_index(index.name)

def _remove_composed_index(self, model, field_names, constraint_kwargs):
"""
Remove the index on the given list of field_names created by
index/unique_together, depending on constraint_kwargs.
"""
meta_constraint_names = {constraint.name for constraint in model._meta.constraints}
meta_index_names = {constraint.name for constraint in model._meta.indexes}
columns = [model._meta.get_field(field).column for field in field_names]
constraint_names = self._constraint_names(
model,
columns,
exclude=meta_constraint_names | meta_index_names,
**constraint_kwargs,
)
if len(constraint_names) != 1:
num_found = len(constraint_names)
columns_str = ", ".join(columns)
raise ValueError(
f"Found wrong number ({num_found}) of constraints for "
f"{model._meta.db_table}({columns_str})."
)
collection = self.get_collection(model._meta.db_table)
collection.drop_index(constraint_names[0])

def _remove_field_index(self, model, field):
"""Remove a field's db_index=True index."""
collection = self.get_collection(model._meta.db_table)
meta_index_names = {index.name for index in model._meta.indexes}
index_names = self._constraint_names(
model,
[field.column],
index=True,
# Retrieve only BTREE indexes since this is what's created with
# db_index=True.
type_=Index.suffix,
exclude=meta_index_names,
)
if len(index_names) != 1:
num_found = len(index_names)
raise ValueError(
f"Found wrong number ({num_found}) of constraints for "
f"{model._meta.db_table}.{field.column}."
)
collection.drop_index(index_names[0])

def add_constraint(self, model, constraint):
pass
Expand Down
3 changes: 3 additions & 0 deletions django_mongodb/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,12 @@ class OperationDebugWrapper:
wrapped_methods = {
"aggregate",
"create_collection",
"create_indexes",
"drop",
"index_information",
"insert_many",
"delete_many",
"drop_index",
"rename",
"update_many",
}
Expand Down