Skip to content

Commit

Permalink
Add check for existing key index in table
Browse files Browse the repository at this point in the history
  • Loading branch information
Jitendra Purohit committed Jun 26, 2017
1 parent 5a80680 commit 5cff2b6
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 5 deletions.
23 changes: 22 additions & 1 deletion CRM/Core/BAO/SchemaHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,27 @@ public static function getMissingIndices() {

// Compare
$missingSigs = array_diff($requiredSigs, $existingSigs);

//This gets the index key which do exist in the table but the value varies.
//Updating this key would produce duplicate error and eventually skips
//further updates of indices in other tables. Hence remove this from $missingSigs
//while updating indices from sytem status page.
$existingKeyIndices = array();
$existingKeySigs = array_intersect_key($missingSigs, $existingSigs);
if (!empty($existingKeySigs)) {
$missingSigs = array_diff_key($missingSigs, $existingKeySigs);

foreach ($existingKeySigs as $sig) {
$sigParts = explode('::', $sig);
foreach ($requiredIndices[$sigParts[0]] as $index) {
if ($index['sig'] == $sig) {
$existingKeyIndices[$sigParts[0]][] = $index;
continue;
}
}
}
}

// Get missing indices
$missingIndices = array();
foreach ($missingSigs as $sig) {
Expand All @@ -734,7 +755,7 @@ public static function getMissingIndices() {
}
}
}
return $missingIndices;
return array($missingIndices, $existingKeyIndices);
}

/**
Expand Down
24 changes: 21 additions & 3 deletions CRM/Utils/Check/Component/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,29 @@ class CRM_Utils_Check_Component_Schema extends CRM_Utils_Check_Component {
*/
public function checkIndices() {
$messages = array();
$missingIndices = CRM_Core_BAO_SchemaHandler::getMissingIndices();
if ($missingIndices) {
list($missingIndices, $existingKeyIndices) = CRM_Core_BAO_SchemaHandler::getMissingIndices();
if ($existingKeyIndices) {
$html = '';
foreach ($existingKeyIndices as $tableName => $indices) {
foreach ($indices as $index) {
$fields = implode(', ', $index['field']);
$html .= "<tr><td>{$tableName}</td><td>{$index['name']}</td><td>$fields</td>";

This comment has been minimized.

Copy link
@xurizaemon

xurizaemon Jun 27, 2017

Member

Feels wrong to be generating complex HTML here, but that's what we have, and at least this output helps people running into the message. I'm OK with this until we have a better separation of display logic and checks.

ie if this check was being called from cv or API, this wouldn't be the right place to make a CRM_Utils_Check_Message() either

}
}
$keyMessage = "<p>The following tables have an index key with a mismatch in value. Please delete the key indices listed from the below table and then click on 'Update Indices' button. <p>
<p><table><thead><tr><th>Table Name</th><th>Key Name</th><th>Fields</th>
</tr></thead><tbody>
$html
</tbody></table></p>";
}
if ($missingIndices || $existingKeyIndices) {
$message = "You have missing indices on some tables. This may cause poor performance.";
if ($keyMessage) {
$message = $keyMessage;
}
$msg = new CRM_Utils_Check_Message(
__FUNCTION__,
ts('You have missing indices on some tables. This may cause poor performance.'),
ts($message),
ts('Performance warning: Missing indices'),
\Psr\Log\LogLevel::WARNING,
'fa-server'
Expand Down
3 changes: 2 additions & 1 deletion api/v3/System.php
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ function civicrm_api3_system_updatelogtables() {
* This adds any indexes that exist in the schema but not the database.
*/
function civicrm_api3_system_updateindexes() {
CRM_Core_BAO_SchemaHandler::createMissingIndices(CRM_Core_BAO_SchemaHandler::getMissingIndices());
list($missingIndices) = CRM_Core_BAO_SchemaHandler::getMissingIndices();
CRM_Core_BAO_SchemaHandler::createMissingIndices($missingIndices);
return civicrm_api3_create_success(1);
}

0 comments on commit 5cff2b6

Please sign in to comment.