Add system check for django.contrib.postgres in INSTALLED_APPS when using the fields, indexes, and constraints it provides.

[Seth Yastrov]

Given the following model + index:

from django.db import models
from django.contrib.postgres.indexes import GinIndex, OpClass
from django.db.models.functions import Cast

class MyModel(models.Model):
    class Meta:
        indexes = [
            GinIndex(OpClass(Cast("id", output_field=models.TextField()), name='gin_trgm_ops'), name='foobar')
        ]

After running makemigrations and running the migration, it produces the SQL:

CREATE INDEX "foobar" ON "myapp_mymodel" USING gin ((CAST("id" AS text) gin_trgm_ops));

Which is a syntax error on Postgres (version 12). The reason is the extra parentheses around CAST.

[Mariusz Felisiak]

Extra parentheses shouldn't be an issue on PostgreSQL. Also I cannot reproduce this crash on PostgreSQL 12.6, see the following test.

    def test_trigram_op_class_cast_gin_index(self):
        index_name = 'trigram_op_class_castgin'
        index = GinIndex(OpClass(Cast('id', CharField(max_length=255)), name='gin_trgm_ops'), name=index_name)
        with connection.schema_editor() as editor:
            editor.add_index(Scene, index) 
        with editor.connection.cursor() as cursor:
            cursor.execute(self.get_opclass_query, [index_name])
            self.assertCountEqual(cursor.fetchall(), [('gin_trgm_ops', index_name)])
        constraints = self.get_constraints(Scene._meta.db_table)
        self.assertIn(index_name, constraints)
        self.assertIn(constraints[index_name]['type'], GinIndex.suffix)
        with connection.schema_editor() as editor:
            editor.remove_index(Scene, index)
        self.assertNotIn(index_name, self.get_constraints(Scene._meta.db_table))

CREATE INDEX "trigram_op_class_castgin" ON "postgres_tests_scene" USING gin ((CAST("id" AS varchar(255))) gin_trgm_ops)

[Hannes Ljungberg]

I'm pretty sure that you're seeing this error because of not adding django.contrib.postgres to INSTALLED_APPS, see: ​https://docs.djangoproject.com/en/3.2/ref/contrib/postgres/indexes/#opclass-expressions

[Seth Yastrov]

Mariusz Felisiak, thanks for your comment. If you'll see my example, the extra parentheses are surrounding gin_trgm_ops as well, not just the CAST expression. This seems to be the problem.

Thank you Hannes Ljungberg!

I did not notice that the documentation says to add django.contrib.postgres to INSTALLED_APPS. That fixes the problem.

Would it be an idea to make a check that django.contrib.postgres is in INSTALLED_APPS when making use of OpClass so that instead of encountering a Postgres syntax error and not knowing what to do (and assuming there is a bug in Django), you would get a helpful error message telling how to correct the problem?

Therefore I'm reopening the issue, and hoping that this check can be made, as despite this requirement being mentioned in the docs, the current behavior is quite unintuitive.

[Mariusz Felisiak]

Therefore I'm reopening the issue, and hoping that this check can be made, as despite this requirement being mentioned in the docs, the current behavior is quite unintuitive.

It would be really complicated. First of all, we would need to mix-up logic from a contrib app and the ORM. Secondly OpClass() don't need to be the topmost expression, so the flatten list of expression would be necessary. Thirdly we don't have similar checks for fields from django.contrib.postgres app which also require including 'django.contrib.postgres' in INSTALLED_APPS, e.g. HStoreField. I don't think it's worth complexity, however we can reconsider this decision if someone provides PoC.

[Simon Charette]

In the light of continuous reports of users running into this qwirk (#35431 being the latest one) I wonder if we could

1. Have all fields defined in contrib.postgres have their check method make sure that 'django.contrib.postgres' in INSTALLED_APPS
2. Adjust Model._check_indexes to do something similar to what we did with Constraint check delegation in 0fb104dda287431f5ab74532e45e8471e22b58c8
3. Have both contrib.postgres indexes and constraints perform the same 'django.contrib.postgres' in INSTALLED_APPS check

Thoughts?

[Simon Charette]

I'd like to re-iterate my plea for the above to take place. ​I helped another user who ran into the same issue today and it was far from clear what exactly was to blame.

Now that 2. from comment:5 is accepted under #36273 would there be interest in re-opening this ticket?

[Javier Buzzi]

Hello, "user who ran into this same issue" here, I am in favor of A. reopening this issue, and B. adding this check so it is clear what needs to be done, instead of chasing my tail for a day and a half.

Thanks!

[Tim Graham]

It seems feasible now that Simon outlined a plan.

[Simon Charette]

Clifford note that this ticket won't be completable until #36273 is completed as the check that should be added django.contrib.postgres.PostgresIndex requires that check method be exposed to subclasses to avoid coupling core with contrib.

[Clifford Gama]

Thanks Simon👍. I noticed, and cc'd myself in #36273 so as to stay up-to-date with the goings-on there.