Problem/Motivation

While working on #3397622: Adding GIN and GIST indexes to PostgreSQL databases I discovered rarely-used syntax for creating an index on a table that allows for indexing on a substring'ed prefix. Support by drivers is optional and it appears the only core DB driver which does so is Postgres. While this works, there is no test coverage.

Steps to reproduce

Not a bug, just no test coverage. The new default test coverage in DriverSpecificSchemaTestBase::assertPrefixedColumnIndex() passes on drivers which do not support this syntax (e.g., MySQL) and fails on pgsql. A Postgres-specific test method override is included in the MR.

Proposed resolution

Interestingly, at least in the case of Postgres this also reveals that pgsql module's implementation of Schema::introspectIndexSchema() won't capture these types of indexes. I messed around with variations on the existing SQL in that method and it seems that functional indexes are just a different beast. As that method's docblock says it will return a list of columns that are indexed, I think it's OK to keep things as they are in that method and not get too cute about trying to also figure out which columns contribute to a functional index.

Remaining tasks

User interface changes

API changes

None.

Data model changes

Release notes snippet

Issue fork drupal-3407073

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bradjones1 created an issue. See original summary.

bradjones1 opened merge request !5742

bradjones1’s picture

bradjones1
Primary language English
commented
Status: Active » Needs review

The test-only CI run skips because it's configured for MySQL...but the only "actual" test class changed here is pgsql. Raised a question in Slack. Don't think that should necessarily hold this up, however.

bradjones1’s picture

bradjones1
Primary language English
commented

Test-only changes here aren't going to be helpful because we're adding test coverage, not demonstrating a bug... 🙃

I think this should be ready for review. Some initial test failures but they look to be unrelated/related to new performance testing.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Additional test coverage is always a plus.

See you ran tests for all the different database types and no failures.

The comments read fine to me.

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs work

I'm triaging RTBC issues.

I agree with @smustgrave that additional test coverage is always welcome.

I read the IS, the comments and the MR. I didn't see any unanswered questions. I then tried to apply the diff but it no longer applies.

While reading the MR I was having trouble with the different terms applied to this type of index being tested. It seems that functional, alternative and prefix are being used for the same thing. While I could be wrong it does suggest that consisting in the wording and the method names would help. I also think adding a link to this 'rarely used syntax' would be beneficial.

Setting back to NW for the above.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.