Adding an Index to a model with a custom Field.

250 views
Skip to first unread message

Jan Pieter Waagmeester

unread,
Dec 5, 2017, 2:33:42 PM12/5/17
to Django developers (Contributions to Django itself)
As part of django-modeltrans, I'm trying to add a GinIndex to the model when a custom Field is added. The simplified version of my naive implementation looks like this:

from django.contrib.postgres.fields import JSONField
from django.contrib.postgres.indexes import GinIndex

class
CustomJSONField(JSONField):
   
def contribute_to_class(self, cls, name):
       
super(CustomJSONField, self).contribute_to_class(cls, name)

        index
= GinIndex(fields=[name])
        index
.set_name_with_model(cls)
       
cls._meta.indexes.append(index)


This works fine when an initial migration is created for the model, but becomes a bit undefined when the custom field is added to an existing model:

1. When I add the custom field, with class Meta: indexes = [] present (can also be non-empty), the index is added twice (with the same name).
2. When I add the custom field with the index, the index is correctly added
3. When I remove the index from the custom field, the index is correctly removed.
4. When I re-add the index to the custom field, the change is not detected.

When I reported 1 (and 4): https://code.djangoproject.com/ticket/28888, Tim commented:

As there's no documented support for adding indexes in Field.contribute_to_class(), can you explain why Django is at fault and/or propose a patch? 

Which I understand. I'm open to suggestions for other solutions to add an Index to the model along with the custom field.

But using contribute_to_class seems the cleanest way to do it, and support seems to be almost/halfway there. Is it reasonable to expect this to work? If so, I'm willing to create a patch, but any help/pointers from someone with more in depth knowledge of migrations is appreciated.

Jan Pieter.

James Bennett

unread,
Dec 5, 2017, 3:28:29 PM12/5/17
to django-d...@googlegroups.com
I can think of several cases where I'd want a custom field to be able to add an index, and contribute_to_class() is, as you say, the natural place to do it -- it's the hook for everything else fields want to add as part of their setup.

So I'd be +1 on making sure it's possible to add indexes through contribute_to_class(), and figuring out what we need to document/guarantee in terms of API for it.

charettes

unread,
Dec 5, 2017, 6:45:09 PM12/5/17
to Django developers (Contributions to Django itself)
Hello Jan,

I'm a bit surprised the `contribute_to_class` approach isn't working but I remember
there was previous discussions to support passing an `Index` instance to the `Field.db_index`
option or to allow an `index_class` attribute to be defined on `Field` subclasses to
determine what kind of index should be used when passing `db_index=True`.

From what I remember this feature was planned to be used to replace `BaseSpatialField`
special handling of `spatial_index` and allow custom fields to define

I'm not sure if there's still plan to support such features but I guess it would solve your
problem by exposing a more convenient API.

Cheers,
Simon

Jan Pieter Waagmeester

unread,
Dec 8, 2017, 5:10:18 AM12/8/17
to Django developers (Contributions to Django itself)

On Wednesday, 6 December 2017 00:45:09 UTC+1, charettes wrote:
From what I remember this feature was planned to be used to replace `BaseSpatialField`
special handling of `spatial_index` and allow custom fields to define

This lead me to some DEP drafts on indexes here:  

The latter seems to be more complete, quite some concepts of it seem to be implemented. It also notes the way a Field would specify the index it wants:

Field will gain a method get_default_index which will be called when db_index=True is passed to the __init__. The default value would be the SpatialIndex in gis, which will mark the start of deprecation of spatial_index which would be supported with deprecation warnings.

I am not sure why this was never implemented. The DEP also proposes the db_index argument to fields to accept Index instances, which is also not implemented.

While those are all interesting proposals to provide a nicer API, I still think the behavior described in my original post needs fixing. I'll post any progress here.

Jan Pieter Waagmeester

unread,
May 26, 2018, 10:17:24 AM5/26/18
to Django developers (Contributions to Django itself)
I just discussed with Markus H here at DjangoCon Europe, apparently he did a presentation last year about this, which reflects some of the thoughts from above: https://speakerdeck.com/markush/to-index-or-not-thats-not-the-question-djangocon-europe-2017?slide=35

Reply all
Reply to author
Forward
0 new messages