Adding an Index to a model with a custom Field.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Adding an Index to a model with a custom Field.

Jan Pieter Waagmeester
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.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/d4366ffd-bf1d-468b-82fd-def6efb49ac8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Adding an Index to a model with a custom Field.

James Bennett
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.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAL13Cg_uCe1kK22h8rXEssFuL6f7cBTZdU1qubA4xVnevsTryg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Adding an Index to a model with a custom Field.

charettes
In reply to this post by Jan Pieter Waagmeester
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

Le mardi 5 décembre 2017 14:33:42 UTC-5, Jan Pieter Waagmeester a écrit :
As part of <a href="https://github.com/zostera/django-modeltrans" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fzostera%2Fdjango-modeltrans\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFFEsfYHWMRqm8mv_RTXck25wkvhg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fzostera%2Fdjango-modeltrans\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFFEsfYHWMRqm8mv_RTXck25wkvhg&#39;;return true;">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): <a href="https://code.djangoproject.com/ticket/28888" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F28888\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNE9b6xdtIApS5t8xoyh_OfznO_mLQ&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F28888\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNE9b6xdtIApS5t8xoyh_OfznO_mLQ&#39;;return true;">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.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/0d580017-d503-433b-89f8-16bf6af6c463%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Adding an Index to a model with a custom Field.

Jan Pieter Waagmeester

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:  
 - https://github.com/django/deps/pull/6
 - https://github.com/django/deps/pull/19

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.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/afce7b2b-7c33-4fe0-bb7f-784a780a580f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Adding an Index to a model with a custom Field.

Jan Pieter Waagmeester
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

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/274f1fca-a07c-47b0-b131-7d824c7557f3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.