Raise Index name limit for postgres

[Jarek Glowacki]

Hi!

This line here: ​https://docs.djangoproject.com/en/3.1/ref/models/indexes/#django.db.models.Index.name

Index.name

... For compatibility with different databases, index names cannot be longer than 30 characters and shouldn’t start with a number (0-9) or underscore (_).

Okay, but why is this restriction maintained when using a specific backend's db index class, namely PostgresIndex?
​https://github.com/django/django/blob/8c7992f658e1125f8dc20b19206e8bbbe4187372/django/contrib/postgres/indexes.py#L11-L19

This issue has become more noticeable with django 3.0, now that interpolated names are allowed for index definitions. Nice feature, until it falls over because one of your models has a longer name than the rest. :(

I'd like to propose we change the docs to read:

Index.name

... For compatibility with different databases, index names cannot be longer than 30 characters and shouldn’t start with a number (0-9) or underscore (_). To get around this restriction, you can directly import and use the Index class for your database backend (eg PostgresIndex), or write your own.

And change the code to just:

class PostgresIndex(Index):

    @cached_property
    def max_name_length(self):
        return 63

    ...

Notes:

• This _should_ be backwards compatible, though I'm not entirely sure how django creates indexes automatically and applies name truncation in these cases, if it were to somewhy use this class, we'd start getting different truncation to what we had in the past and the user would need to make migrations to recreate some of their indexes with new names.. surely not?
• I'm not sure whether this change should be limited to indexes only. I had a brief look and found ​/ this, which already does what I'm suggesting elsewhere. We should pull the 63 out into a POSTGRES_IDENTIFIER_MAX_NAME_LENGTH constant and apply it more broadly?

Given the goahead, I'm happy to turn the above into a PR.

Thanks for your time. :)

[Mariusz Felisiak]

And change the code to just:

class PostgresIndex(Index):

    @cached_property
    def max_name_length(self):
        return 63

This will break backwards compatibility for PostgresIndex subclasses with long suffixes, because currently we allow suffixes of any length.

I'm not sure whether this change should be limited to indexes only. I had a brief look and found ​/ this, which already does what I'm suggesting elsewhere. We should pull the 63 out into a POSTGRES_IDENTIFIER_MAX_NAME_LENGTH constant and apply it more broadly?

No, because it will break backwards compatibility for backend subclasses (per the comment, "This implementation returns 63, but can be overridden by a custom database backend that inherits most of its behavior from this one."). There are people doing this, e.g. #25428.

I think it's enough to document suffix and max_name_length attributes in ​Model index reference. What do you think?

[Jarek Glowacki]

Hey sorry for the slow response.

Replying to felixxm:

This will break backwards compatibility for PostgresIndex subclasses with long suffixes, because currently we allow suffixes of any length.

I had a deeper dig and I don't see how that's the case.. max_name_length is only interrogated when:

• an index name has been automatically constructed: ​https://github.com/django/django/blob/8c7992f658e1125f8dc20b19206e8bbbe4187372/django/db/models/indexes.py#L118-L121
• system checks run: ​https://github.com/django/django/blob/8c7992f658e1125f8dc20b19206e8bbbe4187372/django/db/models/base.py#L1604-L1612

The code that automatically generates indexes would remain untouched, and unaffected:
​https://github.com/django/django/blob/8c7992f658e1125f8dc20b19206e8bbbe4187372/django/db/models/indexes.py#L110-L117
It could potentially be relaxed to pad out the hash up to MyIndex.max_name_length, but I'm not suggesting we change this, and I don't think doing so would provide any benefit.

I've gone ahead and implemented the change here: ​https://github.com/django/django/compare/master...jarekwg:ticket_31881?expand=1
Tests all pass

Please note that PostgresIndex isn't used by default for Postgres backends, it's only used when inherited by those special Postgres-specific Indexes. So the solution I'm suggesting would only help in cases where users explicitly specify PostgresIndex when defining their custom Indexes. Users with custom overrides would still be overriding all this, so I don't see how they'd run into regressions.

The only hardcoded shenanigans i could find that would have trouble if suffix length were to change is the one here, but it's oracle-specific code, so wouldn't be affected by our work anyway: ​https://github.com/django/django/blob/8c7992f658e1125f8dc20b19206e8bbbe4187372/django/db/backends/oracle/operations.py#L596

All that said, I agree that documenting suffix and max_name_length would also be quite helpful. I wouldn't've opened this ticket had there been a note in the docs, instructing me to create my own index class with these attributes. I'm happy to offer a PR for that too.