Problem/Motivation

LocalesLocationAddIndexUpdateTest::testExistingIndex() is failing on PostgreSQL. The array of indexes has unique keys and the keys are sorted.

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/pgsql...
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/pgsql...

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3458922

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

quietone created an issue. See original summary.

quietone opened merge request !8654

quietone’s picture

quietone commented
Issue summary: View changes
Parent issue: #3156439: Add an index on locales_location on type and name »
quietone’s picture

quietone commented
Status: Active » Needs review
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I think given we have to sort the index on Postgres the assertions are largely pointless.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Priority: Major » Critical

catch made their first commit to this issue’s fork.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Reviewed & tested by the community

This looks great now.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Why not just check that the index exists after the update? We should trust that the database driver has tests that ensure an index is actually structured correctly, we don't need to re-assert that here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Opened #3458971: Postrges implementation of introspectIndexSchema is incorrect to fix the postgres schema inspection issue - I think the postgres implementation is incorrect and sorting by column name makes the method less valuable.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@longwave pointed out we should be able to trust our APIs and not assrt against the internals - so removed the assertions on index structure.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Looks good to me.

catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked back through to 10.3.x, thanks!

  • catch committed 7d0e0674 on 10.3.x 
    Issue #3458922 by quietone, alexpott, longwave: Fix index test in...

  • catch committed 502d772d on 10.4.x 
    Issue #3458922 by quietone, alexpott, longwave: Fix index test in...

  • catch committed 2cce2e8f on 11.0.x 
    Issue #3458922 by quietone, alexpott, longwave: Fix index test in...

  • catch committed 11c4be53 on 11.x 
    Issue #3458922 by quietone, alexpott, longwave: Fix index test in...
catch’s picture

catch
he/him
Primary language English
commented

catch closed merge request !8654

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.