This could be tested if running tests for SchemaAPI when main durpal instance user database prefix

Test: 'Node index exists' would be broken because it checks for 'node' table

CommentFileSizeAuthor
#8 720620_indexExists_d7.patch6.12 KBandypost
#6 720620_indexExists_d7.patch5.92 KBandypost
#4 720620_indexExists_d7.patch3.51 KBandypost
#1 720620_indexExists_d7.patch4.76 KBandypost
indexExists_d7.patch1.49 KBandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Priority: Normal » Critical
FileSize
720620_indexExists_d7.patch4.76 KB

Critical because this schema API

This patch with tests, mostly for pgsql, because sqlite schema tests is mostly broken

Status: Needs review » Needs work

The last submitted patch, 720620_indexExists_d7.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs work » Needs review

#1: 720620_indexExists_d7.patch queued for re-testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
FileSize
720620_indexExists_d7.patch3.51 KB

There was hunk to rename indexes from #718016: DatabaseSchema_pgsql::renameTable() needs to rename the indexes

I replace testCheckIndex() with extended tests inside testSchema() for #718846: DatabaseSchema_pgsql should not prefix the constraints names

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Needs review » Needs work

Nice additional coverage. Please move all the patch from #716006: Schema tests fail on SQLite into here: properly fixing SQLite requires a little bit more work then just adding the prefixing into DatabaseSchema::indexExists().

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs work » Needs review
FileSize
720620_indexExists_d7.patch5.92 KB

Cool! with changes from #716006 schema tests works for sqlite!

Josh Waihi’s picture

Josh Waihi CreditAttribution: Josh Waihi commented
Status: Needs review » Needs work 
$n = strlen($this->connection->prefixTables('{' . $table . '}')) + 1;

Can we have some documentation? I don't understand what this is for.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs work » Needs review
FileSize
720620_indexExists_d7.patch6.12 KB

Added comment.

@Josh indexes in pgsql and sqlite are prefixed with table name, to get index name we need remove {table_name}_

Josh Waihi’s picture

Josh Waihi CreditAttribution: Josh Waihi commented
Status: Needs review » Reviewed & tested by the community

OK cool, I'm happy and trust @andypost's patches with docs :)

mynet’s picture

mynet CreditAttribution: mynet commented

indexExists_d7.patch queued for re-testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Looks good. Committed to HEAD.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

Now we need #718016: DatabaseSchema_pgsql::renameTable() needs to rename the indexes

Status: Fixed » Closed (fixed)

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