Fixed #26808 -- Introduce Meta.indexes #6857

akki · 2016-06-30T23:45:26Z

Other PR that needs to be merged before this - ~~!6871~~.

timgraham · 2016-07-01T15:15:52Z

docs/ref/schema-editor.txt

+---------------
+
+.. method:: BaseDatabaseSchemaEditor.add_index(model, index)
+


.. versionadded:: 1.11

timgraham · 2016-07-07T15:01:27Z

django/db/backends/base/schema.py

@@ -890,6 +890,9 @@ def _model_indexes_sql(self, model):
        for field_names in model._meta.index_together:
            fields = [model._meta.get_field(field) for field in field_names]
            output.append(self._create_index_sql(model, fields, suffix="_idx"))
+
+        for index in model._meta.indexes:
+            output.append(index.create_sql(self))


Rebase and add model argument.

akki · 2016-07-09T08:05:47Z

@timgraham @MarkusH Please let me know if anything else is required.

timgraham · 2016-07-13T19:13:14Z

django/db/migrations/state.py

+        # Sanity-check that indexes have their names set.
+        for index in self.options['indexes']:
+            if not index._name:
+                raise ValueError("Indexes passed to ModelState require a name attribute, %r doesn't have one." % index)


Use:

raise ValueError( "Indexes passed to ModelState require a name attribute, " "%r doesn't have one." % index )

While we allow longer lines, we typically break up long strings like this.

timgraham · 2016-07-13T19:36:45Z

Is adding an index using AddIndex if it's not in Model.indexes (as suggested in migration-operations.txt) valid? Is there a risk Django might drop that index in some future alter operation since it doesn't know about it in models?

akki · 2016-07-14T12:42:50Z

Ahh, right! I think we can remove that from the docs, none of the other operations does that either. I have made this change in the last commit, if it seems right.

I also wanted to know if (and how) I should mention this in the release docs.

Thanks

timgraham · 2016-07-14T14:18:20Z

docs/ref/migration-operations.txt

-
-If you're writing your own migration to add an index, you must assign a
-``name`` to the ``index`` as done above.
+It only accepts an `Index` instance for which the `name` attribute is defined.


Not sure this is important to mention since most users shouldn't encounter it and a helpful message is raised in that case. Otherwise, you need to use double backticks around "Index" and "name" to have an effect on the rendered docss.

Better to remove it then.

timgraham · 2016-07-14T14:19:46Z

I think we can wait until the end of this project before writing release notes as things may evolve.

timgraham · 2016-07-14T20:02:03Z

django/db/migrations/autodetector.py

@@ -176,6 +176,9 @@ def _detect_changes(self, convert_apps=None, graph=None):
        self.generate_altered_options()
        self.generate_altered_managers()

+        # Create the altered indexes and store them in self.altered_indexes


timgraham · 2016-07-15T17:32:09Z

django/db/migrations/state.py

@@ -353,6 +353,13 @@ def __init__(self, app_label, name, fields, options=None, bases=None, managers=N
                    'ModelState.fields cannot refer to a model class - "%s.through" does. '
                    'Use a string reference instead.' % name
                )
+        # Sanity-check that indexes have their names set.


names -> name

MarkusH · 2016-07-20T20:26:07Z

django/db/migrations/autodetector.py

@@ -521,6 +529,9 @@ def generate_created_models(self):
                    related_fields[field.name] = field
                if getattr(field.remote_field, "through", None) and not field.remote_field.through._meta.auto_created:
                    related_fields[field.name] = field
+            # Are there any indexes to defer?


This comment doesn't really make sense. Could you explain why you're doing what you're doing?

If we have indexes defined in Meta.indexes while creating a new model we also create AddIndex operations in the migration. Similar to what has been done for AlterUniqu/IndexTogether and order_with_respect_to here.

timgraham · 2016-08-04T23:27:24Z

django/db/migrations/autodetector.py

@@ -175,13 +176,20 @@ def _detect_changes(self, convert_apps=None, graph=None):
        self.generate_altered_options()
        self.generate_altered_managers()

+        # Create the altered indexes and store them in self.altered_indexes.
+        # This avoids the same computation in generate_removed_indexes


generate_removed_indexes() and generate_added_indexes()
(add parentheses)

timgraham · 2016-08-04T23:37:02Z

I merged the first commit in b92c6b7. In the end, unless you feel differently, I think we can squash the others when merging. Splitting them out definitely made review easier though.

I think this is good to go, @MarkusH are you happy?

akki · 2016-08-05T02:46:04Z

I share the same rationale. Will squash them in my next push.

Added support for class based indexes in Meta class Other changes * Added the index name to it's deconstruction * Added indexes to sqlite3.schema._remake_table Otherwise indexes made using class based indexes get dropped whenever _remake_table is called. Thanks timgraham & MarkusH for review and advice

timgraham · 2016-08-05T22:16:09Z

merged in 6a8372e, thanks!

akki force-pushed the gsoc-indexes-2 branch from 24a207b to d624e18 Compare July 1, 2016 01:18

timgraham reviewed Jul 1, 2016
View reviewed changes

docs/ref/schema-editor.txt

---------------

.. method:: BaseDatabaseSchemaEditor.add_index(model, index)

Copy link

Member

timgraham Jul 1, 2016

Choose a reason for hiding this comment

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

.. versionadded:: 1.11

akki force-pushed the gsoc-indexes-2 branch 4 times, most recently from a466e32 to 4558449 Compare July 3, 2016 11:32

akki changed the title ~~[WIP] Fixed #26808 -- Introduce Meta.indexes~~ Fixed #26808 -- Introduce Meta.indexes Jul 3, 2016

akki force-pushed the gsoc-indexes-2 branch from 4558449 to e06e3ca Compare July 6, 2016 14:49

timgraham reviewed Jul 7, 2016
View reviewed changes

akki force-pushed the gsoc-indexes-2 branch from e06e3ca to b79ba67 Compare July 7, 2016 16:48

timgraham reviewed Jul 13, 2016
View reviewed changes

akki force-pushed the gsoc-indexes-2 branch from b79ba67 to a7a1342 Compare July 14, 2016 12:23

akki force-pushed the gsoc-indexes-2 branch from a7a1342 to daf9239 Compare July 14, 2016 12:55

timgraham reviewed Jul 14, 2016
View reviewed changes

akki force-pushed the gsoc-indexes-2 branch from daf9239 to aa44813 Compare July 14, 2016 17:20

timgraham reviewed Jul 14, 2016
View reviewed changes

akki force-pushed the gsoc-indexes-2 branch from aa44813 to 668ba24 Compare July 15, 2016 05:52

timgraham reviewed Jul 15, 2016
View reviewed changes

akki force-pushed the gsoc-indexes-2 branch 4 times, most recently from 8027608 to 2496a55 Compare July 18, 2016 15:45

akki mentioned this pull request Jul 19, 2016

Fixed #26180 -- AlterUniqueTogether should stay before RemoveField operation after optimization #6926

Closed

MarkusH reviewed Jul 20, 2016
View reviewed changes

akki force-pushed the gsoc-indexes-2 branch from e07c3b7 to a25385f Compare August 2, 2016 05:54

edmorley mentioned this pull request Aug 2, 2016

Add support for defining MySQL FULLTEXT INDEX in the model adamchainz/django-mysql#314

Closed

timgraham reviewed Aug 4, 2016
View reviewed changes

akki force-pushed the gsoc-indexes-2 branch from a25385f to 08b00a2 Compare August 5, 2016 02:19

akki force-pushed the gsoc-indexes-2 branch 2 times, most recently from 74356ed to adb03e4 Compare August 5, 2016 08:25

akki force-pushed the gsoc-indexes-2 branch from adb03e4 to 9ff3a37 Compare August 5, 2016 15:20

timgraham closed this Aug 5, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #26808 -- Introduce Meta.indexes #6857

Fixed #26808 -- Introduce Meta.indexes #6857

akki commented Jun 30, 2016 •

edited

timgraham Jul 1, 2016

timgraham Jul 7, 2016

akki commented Jul 9, 2016

timgraham Jul 13, 2016 •

edited

timgraham commented Jul 13, 2016

akki commented Jul 14, 2016

timgraham Jul 14, 2016

akki Jul 14, 2016

timgraham commented Jul 14, 2016

timgraham Jul 14, 2016

timgraham Jul 15, 2016

MarkusH Jul 20, 2016

akki Jul 21, 2016 •

edited by MarkusH

timgraham Aug 4, 2016

timgraham commented Aug 4, 2016

akki commented Aug 5, 2016

timgraham commented Aug 5, 2016

		---------------

		.. method:: BaseDatabaseSchemaEditor.add_index(model, index)

Fixed #26808 -- Introduce Meta.indexes #6857

Fixed #26808 -- Introduce Meta.indexes #6857

Conversation

akki commented Jun 30, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akki commented Jul 9, 2016

timgraham Jul 13, 2016 • edited

Choose a reason for hiding this comment

timgraham commented Jul 13, 2016

akki commented Jul 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timgraham commented Jul 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akki Jul 21, 2016 • edited by MarkusH

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timgraham commented Aug 4, 2016

akki commented Aug 5, 2016

timgraham commented Aug 5, 2016

akki commented Jun 30, 2016 •

edited

timgraham Jul 13, 2016 •

edited

akki Jul 21, 2016 •

edited by MarkusH