From 90dcf8f89fa6c89c18db6de766efe1f14761ecf8 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 7 May 2021 12:11:58 +1200 Subject: [PATCH] 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 | 21 ++++++--------------- CRM/Financial/DAO/FinancialType.php | 14 ++------------ CRM/Price/DAO/LineItem.php | 17 ++++------------- xml/schema/Core/EntityFile.xml | 10 ++-------- xml/schema/Financial/FinancialType.xml | 6 ------ xml/schema/Price/LineItem.xml | 10 ++-------- 6 files changed, 16 insertions(+), 62 deletions(-) diff --git a/CRM/Core/DAO/EntityFile.php b/CRM/Core/DAO/EntityFile.php index 9b5240bfe9..d26286a44b 100644 --- a/CRM/Core/DAO/EntityFile.php +++ b/CRM/Core/DAO/EntityFile.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Core/EntityFile.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:76720cd19bd7b5bbbf1461184198acc5) + * (GenCodeChecksum:37cc0051ec4b2d54eeeefe4c4d004b6d) */ /** @@ -231,25 +231,16 @@ class CRM_Core_DAO_EntityFile extends CRM_Core_DAO { */ public static function indices($localize = TRUE) { $indices = [ - 'index_entity' => [ - 'name' => 'index_entity', + 'UI_entity_id_entity_table_file_id' => [ + 'name' => 'UI_entity_id_entity_table_file_id', 'field' => [ - 0 => 'entity_table', - 1 => 'entity_id', - ], - 'localizable' => FALSE, - 'sig' => 'civicrm_entity_file::0::entity_table::entity_id', - ], - 'UI_entity_table_entity_id_file_id' => [ - 'name' => 'UI_entity_table_entity_id_file_id', - 'field' => [ - 0 => 'entity_table', - 1 => 'entity_id', + 0 => 'entity_id', + 1 => 'entity_table', 2 => 'file_id', ], 'localizable' => FALSE, 'unique' => TRUE, - 'sig' => 'civicrm_entity_file::1::entity_table::entity_id::file_id', + 'sig' => 'civicrm_entity_file::1::entity_id::entity_table::file_id', ], ]; return ($localize && !empty($indices)) ? CRM_Core_DAO_AllCoreTables::multilingualize(__CLASS__, $indices) : $indices; diff --git a/CRM/Financial/DAO/FinancialType.php b/CRM/Financial/DAO/FinancialType.php index 8a54a20478..a5db19a8dc 100644 --- a/CRM/Financial/DAO/FinancialType.php +++ b/CRM/Financial/DAO/FinancialType.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Financial/FinancialType.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:4f25c751d874b9438009c8685f8d6843) + * (GenCodeChecksum:035576414ddb721b2607522dc5d874e4) */ /** @@ -287,17 +287,7 @@ class CRM_Financial_DAO_FinancialType extends CRM_Core_DAO { * @return array */ public static function indices($localize = TRUE) { - $indices = [ - 'UI_id' => [ - 'name' => 'UI_id', - 'field' => [ - 0 => 'id', - ], - 'localizable' => FALSE, - 'unique' => TRUE, - 'sig' => 'civicrm_financial_type::1::id', - ], - ]; + $indices = []; return ($localize && !empty($indices)) ? CRM_Core_DAO_AllCoreTables::multilingualize(__CLASS__, $indices) : $indices; } diff --git a/CRM/Price/DAO/LineItem.php b/CRM/Price/DAO/LineItem.php index 4649837b1c..c226921b4b 100644 --- a/CRM/Price/DAO/LineItem.php +++ b/CRM/Price/DAO/LineItem.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Price/LineItem.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:ce105cf63ee265d470ed263f959a7706) + * (GenCodeChecksum:1c10b6b406bb73014b2fbe6ceb95fda3) */ /** @@ -511,27 +511,18 @@ class CRM_Price_DAO_LineItem extends CRM_Core_DAO { */ public static function indices($localize = TRUE) { $indices = [ - 'index_entity' => [ - 'name' => 'index_entity', - 'field' => [ - 0 => 'entity_table', - 1 => 'entity_id', - ], - 'localizable' => FALSE, - 'sig' => 'civicrm_line_item::0::entity_table::entity_id', - ], 'UI_line_item_value' => [ 'name' => 'UI_line_item_value', 'field' => [ - 0 => 'entity_table', - 1 => 'entity_id', + 0 => 'entity_id', + 1 => 'entity_table', 2 => 'contribution_id', 3 => 'price_field_value_id', 4 => 'price_field_id', ], 'localizable' => FALSE, 'unique' => TRUE, - 'sig' => 'civicrm_line_item::1::entity_table::entity_id::contribution_id::price_field_value_id::price_field_id', + 'sig' => 'civicrm_line_item::1::entity_id::entity_table::contribution_id::price_field_value_id::price_field_id', ], ]; return ($localize && !empty($indices)) ? CRM_Core_DAO_AllCoreTables::multilingualize(__CLASS__, $indices) : $indices; diff --git a/xml/schema/Core/EntityFile.xml b/xml/schema/Core/EntityFile.xml index 37ac0882c1..e2dc7bbd6b 100644 --- a/xml/schema/Core/EntityFile.xml +++ b/xml/schema/Core/EntityFile.xml @@ -43,12 +43,6 @@ entity_table 1.5 - - index_entity - entity_table - entity_id - 1.5 - file_id File ID @@ -67,9 +61,9 @@ 1.5 - UI_entity_table_entity_id_file_id - entity_table + UI_entity_id_entity_table_file_id entity_id + entity_table file_id true 1.1 diff --git a/xml/schema/Financial/FinancialType.xml b/xml/schema/Financial/FinancialType.xml index c6850f15ce..cc158141b0 100644 --- a/xml/schema/Financial/FinancialType.xml +++ b/xml/schema/Financial/FinancialType.xml @@ -97,10 +97,4 @@ - - UI_id - id - true - 4.3 - diff --git a/xml/schema/Price/LineItem.xml b/xml/schema/Price/LineItem.xml index 98b7e38e78..6b2733e717 100644 --- a/xml/schema/Price/LineItem.xml +++ b/xml/schema/Price/LineItem.xml @@ -138,16 +138,10 @@ 3.2 - - index_entity - entity_table - entity_id - 1.7 - - + UI_line_item_value - entity_table entity_id + entity_table contribution_id price_field_value_id price_field_id -- 2.25.1