Opened 7 years ago

Last modified 7 years ago

#28888 new Cleanup/optimization

Index added to _meta.indexes with Meta.indexes=[] yields two equal addIndex() operations.

Reported by: Jan Pieter Waagmeester Owned by: nobody
Component: Database layer (models, ORM) Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Someday/Maybe
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no
Pull Requests:How to create a pull request

Description (last modified by Jan Pieter Waagmeester)

I use a custom field derived from django.contrib.postgres.fields.JSONField, which adds an GinIndex to model._meta.indexes:

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)

When used in a model like this,

class Blog(models.Model):
    title = models.CharField(max_length=100)
    json = CustomJSONField()

Migrations for model and index are created as expected:

./manage.py --version
1.11.8
./manage.py makemigrations app
Migrations for 'app':
  app/migrations/0001_initial.py
    - Create model Blog
    - Create index app_blog_json_2cf556_gin on field(s) json of model blog

But when I add an empty list of indexes to class Meta like this:

class Blog(models.Model):
    title = models.CharField(max_length=100)
    json = CustomJSONField()

    class Meta:
        indexes = []

two indexes are created:

rm -rf app/migrations
./manage.py --version
1.11.8
./manage.py makemigrations app
Migrations for 'app':
  app/migrations/0001_initial.py
    - Create model Blog
    - Create index app_blog_json_2cf556_gin on field(s) json of model blog
    - Create index app_blog_json_2cf556_gin on field(s) json of model blog

Which of course results in django.db.utils.ProgrammingError: relation "app_blog_json_2cf556_gin" already exists.

According to the ticket's flags, the next step(s) to move this issue forward are:

  • Unknown. The Someday/Maybe triage stage is used to keep track of high-level ideas or long term feature requests.

    It could be an issue that's blocked until a future version of Django (if so, Keywords will contain that version number). It could also be an enhancement request that we might consider adding someday to the framework if an excellent patch is submitted.

    If you're interested in contributing to the issue, raising your ideas on the Django Forum would be a great place to start.

Change History (5)

comment:1 by Jan Pieter Waagmeester, 7 years ago

Description: modified (diff)

comment:2 by Jan Pieter Waagmeester, 7 years ago

Possibly unrelated, but:

When an initial migration is added without the index added to cls._meta.indexes, resulting in the output

./manage.py makemigrations app
Migrations for 'app':
  app/migrations/0001_initial.py
    - Create model Blog

No index is added, as expected.

However, adding the index to cls._meta.indexes has no effect: No changes detected in app 'app'.

comment:3 by Tim Graham, 7 years ago

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?

comment:4 by Jan Pieter Waagmeester, 7 years ago

I understand that "there's no documented support for adding indexes in Field.contribute_to_class()" and I'm open to suggestions to other solutions to add indexes with a field.

But this seems the cleanest way to do it, and support seems to be almost there/halfway there. I'm willing to invest time to create a patch to fix this, but I'd like to have some degree of confidence that such a solution would be desirable functionality.

comment:5 by Tim Graham, 7 years ago

Component: MigrationsDatabase layer (models, ORM)
Triage Stage: UnreviewedSomeday/Maybe
Type: UncategorizedCleanup/optimization

Usually it's more effective to use the DevelopersMailingList to get feedback and consensus on design decisions as it reaches a wider audience than the ticket tracker.

Note: See TracTickets for help on using tickets.
Back to Top