Skip to content

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

Closed
blink1073 wants to merge 5 commits into
mongodb:6.0.xfrom
blink1073:6.0-INTPYTHON-1082
Closed

blink1073 wants to merge 5 commits into
mongodb:6.0.xfrom
blink1073:6.0-INTPYTHON-1082

Conversation

@blink1073

@blink1073 blink1073 commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

INTPYTHON-1082 (https://jira.mongodb.org/browse/INTPYTHON-1082)

Backport of #640.

Changes in this PR

On 6.0.x, an index declared with an ordering expression such as Index(F("priority").desc(), name="...") was silently skipped, and migrate crashed on Index subclasses that report contains_expressions=False (django-tasks-db's ConditionalPartialIndex).

  • F() and OrderBy() orderings now translate into MongoDB index keys; anything else, including NULLS FIRST/LAST, is skipped without raising.
  • The models.W043 warning now matches whether the index is actually created.
  • Dropping a never-created expression index no longer raises IndexNotFound; field-only removals still surface it.

The only delta from #640 is keeping the column_prefix parameter that 6.0.x still needs (main dropped it in the Django 6.1 update), on the rewritten get_pymongo_index_model and on SearchIndex.get_pymongo_index_model.

Test Plan

  • New tests cover creation, skipping, warnings, and removal.
  • New test exercising the column_prefix parameter (in the second commit).
  • On the Django 6.0 fork with a local mongod: the tests fail on unpatched 6.0.x (10 failures, 3 errors), then pass 89 (9 pre-existing skips).
  • Django's schema and constraints apps pass; pre-commit run --all-files is clean.

Checklist

Checklist for Author

  • Did you update the changelog (if necessary)?
  • Is the intention of the code captured in relevant tests?
  • If there are new TODOs, has a related JIRA ticket been created?

Checklist for Reviewer

  • Does the title of the PR reference a JIRA Ticket?
  • Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?)
  • Have you checked for spelling & grammar errors?
  • Is all relevant documentation (README or docstring) updated?

…e dropped or crash migrate (mongodb#640)

* INTPYTHON-1082: Create indexes declared with ordering expressions

* INTPYTHON-1082: Address review feedback on expression index handling

* trim prose

* trim prose

* INTPYTHON-1082: Address second review round on expression index handling

- remove_index() records the drop directly when collecting migration SQL instead of making a live listIndexes call.
- get_pymongo_index_model() skips an index with untranslatable expressions before translating its condition, so an unsupported condition lookup no longer raises.

* INTPYTHON-1082: Fix index removal with unsupported partial-filter conditions

* INTPYTHON-1082: Warn for untranslatable expressions when Index.check omits W043

* trim text

* Warn when an index is skipped for untranslatable expressions

* Simplify index removal and deduplicate W043 tests

* Consolidate index-existence checks and drop dead mocks

* Only tolerate IndexNotFound when removing expression indexes

(cherry picked from commit 4bf0186)
@blink1073
blink1073 changed the base branch from main to 6.0.x October 26, 2026 22:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved correctness issues affect cross-database checks, expression translation, and embedded-index cleanup.

3 open issues
What changed in this PR

Backports #640 to the 6.0.x backend, adding MongoDB index support for field-ordering expressions while retaining embedded-field column prefixes.

Changes:

  • Translates supported F() and OrderBy() expressions into index keys and aligns warnings with translation support.
  • Handles removal of expression indexes skipped by earlier versions.
  • Adds regression tests and release notes.
File Description
tests/​schema_/​test_embedded_model.py Tests embedded expression indexes with column prefixes.
tests/​indexes_/​test_expressions.py Tests expression translation, skipping, and removal.
tests/​indexes_/​test_checks.py Tests expression-index warnings.
tests/​indexes_/​test_base.py Adds a custom index subclass test fixture.
docs/​releases/​6.0.x.rst Documents fixes and upgrade guidance.
django_mongodb_backend/​schema.py Updates expression-index removal handling.
django_mongodb_backend/​indexes.py Implements expression translation and warning checks.
django_mongodb_backend/​features.py Clarifies support and adjusts expected test failures.

Review effort: Balanced

Comment thread django_mongodb_backend/indexes.py Resolved
Comment thread django_mongodb_backend/indexes.py Resolved
Comment thread django_mongodb_backend/schema.py Resolved
@blink1073

Copy link
Copy Markdown
Member Author

Blocked by #646

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

An active moderate issue causes embedded-model expression indexes with conditions to generate partial filters at the wrong document path, so the index can match the wrong documents and requires a code fix plus regression coverage.

1 open issue
3 resolved since last review

Review effort: Lite

Comment thread django_mongodb_backend/indexes.py Resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved critical and moderate defects can make embedded nullable unique constraints ineffective and cause embedded index creation to fail for list-valued conditions.

2 open issues
1 resolved since last review

Review effort: Lite

Comment thread django_mongodb_backend/constraints.py Resolved
Comment thread django_mongodb_backend/indexes.py Resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain, and the change is supported by regression tests and documented handling for expression indexes, warnings, embedded paths, and index removal.

No open issues 🎉

2 resolved since last review

Review effort: Lite

@blink1073
blink1073 marked this pull request as ready for review October 27, 2026 20:40
@blink1073
blink1073 requested a review from a team as a code owner October 27, 2026 20:40
@blink1073
blink1073 requested review from NoahStapp and removed request for a team October 27, 2026 20:40
@blink1073

Copy link
Copy Markdown
Member Author

Closing this as not worth the effort to backport due to the need to handle column_prefix.

@blink1073 blink1073 closed this Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants