PostgresIndex.create_sql() doesn't respect the using argument.

[Alexandru Mărășteanu]

django.contrib.postgres.indexes.PostgresIndex uses the suffix attribute for both generating the index name (in Index.set_name_with_model see ​https://github.com/django/django/blob/fac662f4798f7e4e0ed9be6b4fb4a87a80810a68/django/db/models/indexes.py#L153-L186) and for deciding the indexing method (in PostgresIndex.create_sql i.e. CREATE INDEX ... USING {suffix} ... see ​https://github.com/django/django/blob/fac662f4798f7e4e0ed9be6b4fb4a87a80810a68/django/contrib/postgres/indexes.py#L25-L36). A developer using and extending PostgresIndex for use with multiple opclasses is unable to specify a custom suffix for the naming scheme.

For example, given two custom indexes like:

class Foo(GinIndex):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, opclasses=[some opclass], **kwargs)


class Bar(GinIndex):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, opclasses=[some opclass], **kwargs)

If set on the same model fields, both Foo.set_name_with_model and Bar.set_name_with_model would then generate the same index name à la dbtable_field_hash_gin which is not ok because index names need to be unique.

Setting custom suffixes on Foo and Bar is not possible with the current code because PostgresIndex.create_sql uses the suffix as the index method. If the suffix doesn't match any of the supported opclasses, then PostgreSQL complains about it.

To workaround that, an option would be to override the create_sql method:

class Foo(GinIndex):
    suffix = "foo"
    
    def __init__(self, *args, **kwargs):
        super().__init__(*args, opclasses=[some opclass], **kwargs)

    def create_sql(self, model, schema_editor, using="", **kwargs):
        return super().create_sql(model, schema_editor, using="gin", **kwargs)


class Bar(GinIndex):
    suffix = "bar"

    def __init__(self, *args, **kwargs):
        super().__init__(*args, opclasses=[some opclass], **kwargs)

    def create_sql(self, model, schema_editor, using="", **kwargs):
        return super().create_sql(model, schema_editor, using="gin", **kwargs)

The problem is, even though PostgresIndex.create_sql accepts a using argument, it currently ignores it. From the source code:

class PostgresIndex(Index):
    ...

    def create_sql(self, model, schema_editor, using="", **kwargs):
        ...
        statement = super().create_sql(
            model, schema_editor, using=" USING %s" % self.suffix, **kwargs
        )
        ...

So in the end there's no way to extend the PostgresIndex with a custom suffix. I was able to do it by temporarily setting a custom suffix in set_name_with_model but that's more of a hack rather than a solution.

--

That said, I believe there are two things which need to be addressed here:

1. Distinguish between the index name suffix and the index method

PostgresIndex would then declare another property which indicated the method to use when defining the index i.e. using.

2. PostgresIndex.create_sql respects the using argument

E.g. (also considering point 1 above):

class PostgresIndex(Index):
    ...

    def create_sql(self, model, schema_editor, using="", **kwargs):
        ...
        statement = super().create_sql(
            model, schema_editor, using=using or self.using, **kwargs
        )
        ...

---

If this is accepted, I'm available to work on it.

[Alexandru Mărășteanu]

Note I'm aware that the index class requires an explicit name when using opclasses, but I think if these were addressed, then it would be easier in the future to leave the name generation to Django.

[Mariusz Felisiak]

Thanks for the report. PostgresIndex is an internal, undocumented API. However, I agree that create_sql() should respect the using argument, so the following diff should be sufficient:

django/contrib/postgres/indexes.py

@@ -25 +25 @@
     def create_sql(self, model, schema_editor, using="", **kwargs):
         self.check_supported(schema_editor)
         statement = super().create_sql(
-            model, schema_editor, using=" USING %s" % self.suffix, **kwargs
+            model, schema_editor, using=" USING %s" % (using or self.suffix), **kwargs
         )
         with_params = self.get_with_params()
         if with_params:

[Alexandru Mărășteanu]

Replying to Mariusz Felisiak:

I agree that create_sql() should respect the using argument, so the following diff should be sufficient:

Fine, that works for me. What are the next steps?

--

Also, is it possible to make this available to all (2+) Django versions?

[Mariusz Felisiak]

Fine, that works for me. What are the next steps?

You can assign this ticket to yourself and prepare PR (test is required).

Also, is it possible to make this available to all (2+) Django versions?

No, we don't backport cleanups and new features.

[Mariusz Felisiak]

Alexandru, Do you have time to keep working on this?

[Alexandru Mărășteanu]

Replying to Mariusz Felisiak:

Alexandru, Do you have time to keep working on this?

Yes, I'll take care of it this week. Sorry for the delay, I was on vacation.

[Alexandru Mărășteanu]

​https://github.com/django/django/pull/15600

[GitHub <noreply@…>]

In a1e4e86f:

Fixed #33607 -- Made PostgresIndex.create_sql() respect the "using" argument.