dev/core#2575 remove / improve indexes
authoreileen <emcnaughton@wikimedia.org>
Fri, 7 May 2021 00:11:58 +0000 (12:11 +1200)
committerYour Name <you@example.com>
Fri, 7 May 2021 00:33:10 +0000 (12:33 +1200)
commit90dcf8f89fa6c89c18db6de766efe1f14761ecf8
tree1c631eee0fe9817674a0f520e738ce5854cefdcf
parentf6c8032e05799d8a844604810866a41ffdb4c9a9
dev/core#2575 remove / improve indexes

This removes a couple of indexes on the id field as they duplicate the primary key
as pointed out in https://lab.civicrm.org/dev/core/-/issues/2575

In addition a couple of other indexes are removed as duplicates but the
originals are fixed to be more meaningful. It's important that in a combined
key the field with the lowest cardinality is the first as the combined key is
searched for a complete match so on something like line_item we see the
key is

civicrm_contribution1
civicrm_contribution10
civicrm_contribution1111
civicrm_membership1

In this case there are only 3 possible variants for the first chunk of
characters so we get a much more effective key by reversing them.
(arguably the entity_table actually adds no value due to it's low
cardinality).

The original post
https://civicrm.stackexchange.com/questions/39409/redundant-indexes
also suggests that

is_deleted_sort_name_id duplicates the id index

That is not the case - this query would only ever be
used if is_deleted is a filter. If it IS used then the id
index would NOT be used as only one index is used per table.

However, note that the id part of the filter would only be used if
is_deleted and sort_name were too - so the id part is of little value
as the last parameter & it's likely mysql would chose to use the
primary key instead.

However, I would argue the index as a whole is of little value. The
cardinality of is_deleted is super low and so preceding sort_name
with it is probably inferior to just leaving the
sort_name index to be used. However, this would need testing on
a site with enough data to get results.

On the queue index - it's hard to imagine that having more than the
queue name in the index adds any value at all. However, this is also such a small
table that probably the index has almost no size and the query probably takes
almost no time to run so the time we would spend testing index changes on
this table we would likely never get back.

Note changing these indexes will only affect new sites - or existing sites if they
run the System.updateIndexes api. We do have a ticket open
https://lab.civicrm.org/dev/core/-/issues/2279 with a proposal to
allow sites to define their own indexes which would work in conjunction with the
above api call (since indexes are not 1 size fits all - for example we
have removed indexes on things like contribution_status_id due to
low cardinality). We don't pro-actively run index updates on upgrade
due to not wanting to alter custom index config until 2279 is resolved
and also because some old DBs hit issues that we never diagnosed (and stopped
trying to once we stopped pro-actively notifying them about them)
CRM/Core/DAO/EntityFile.php
CRM/Financial/DAO/FinancialType.php
CRM/Price/DAO/LineItem.php
xml/schema/Core/EntityFile.xml
xml/schema/Financial/FinancialType.xml
xml/schema/Price/LineItem.xml