From 26b55a9e8c95e96b1dfd849eea9bb9645d605b13 Mon Sep 17 00:00:00 2001 From: eileenmcnaugton Date: Fri, 12 Feb 2016 16:52:01 +1300 Subject: [PATCH] CRM-18022 fix performance on contribution repeat report --- CRM/Report/Form/Contribute/Repeat.php | 271 ++++++++++++-------- tests/phpunit/api/v3/ReportTemplateTest.php | 1 - 2 files changed, 157 insertions(+), 115 deletions(-) diff --git a/CRM/Report/Form/Contribute/Repeat.php b/CRM/Report/Form/Contribute/Repeat.php index c4e844b444..a657fae91d 100644 --- a/CRM/Report/Form/Contribute/Repeat.php +++ b/CRM/Report/Form/Contribute/Repeat.php @@ -29,8 +29,6 @@ * * @package CRM * @copyright CiviCRM LLC (c) 2004-2015 - * $Id$ - * */ class CRM_Report_Form_Contribute_Repeat extends CRM_Report_Form { protected $_amountClauseWithAND = NULL; @@ -43,6 +41,49 @@ class CRM_Report_Form_Contribute_Repeat extends CRM_Report_Form { public $_drilldownReport = array('contribute/detail' => 'Link to Detail Report'); /** + * Temp table for first time frame. + * + * @var int + */ + protected $tempTableRepeat1 = NULL; + + /** + * Temp table for second time frame. + * + * @var int + */ + protected $tempTableRepeat2 = NULL; + + /** + * The table the report is being grouped by. + * + * @var string + */ + protected $groupByTable; + + /** + * The field the report is being grouped by. + * + * @var string + */ + protected $groupByFieldName; + + /** + * The alias of the table the report is being grouped by. + * + * @var string + */ + protected $groupByTableAlias; + + /** + * The column in the contribution table that joins to the temp tables. + * + * @var + */ + protected $contributionJoinTableColumn; + + /** + * Class constructor. */ public function __construct() { $this->_columns = array( @@ -91,16 +132,6 @@ class CRM_Report_Form_Contribute_Repeat extends CRM_Report_Form { 'default' => TRUE, ), ), - ), - 'civicrm_email' => array( - 'dao' => 'CRM_Core_DAO_Email', - 'fields' => array( - 'email' => array( - 'title' => ts('Email'), - 'no_repeat' => TRUE, - ), - ), - 'grouping' => 'contact-fields', 'order_bys' => array( 'sort_name' => array( 'title' => ts('Last Name, First Name'), @@ -127,6 +158,16 @@ class CRM_Report_Form_Contribute_Repeat extends CRM_Report_Form { ), ), ), + 'civicrm_email' => array( + 'dao' => 'CRM_Core_DAO_Email', + 'fields' => array( + 'email' => array( + 'title' => ts('Email'), + 'no_repeat' => TRUE, + ), + ), + 'grouping' => 'contact-fields', + ), 'civicrm_phone' => array( 'dao' => 'CRM_Core_DAO_Phone', 'fields' => array( @@ -238,19 +279,9 @@ class CRM_Report_Form_Contribute_Repeat extends CRM_Report_Form { parent::__construct(); } - public function preProcess() { - parent::preProcess(); - } - /** - * @param bool $freeze - * - * @return array + * Override parent select for reasons someone will someday make sense of & document. */ - public function setDefaultValues($freeze = TRUE) { - return parent::setDefaultValues($freeze); - } - public function select() { $select = array(); $append = NULL; @@ -294,12 +325,9 @@ class CRM_Report_Form_Contribute_Repeat extends CRM_Report_Form { } /** - * @param bool $tableCol - * - * @return array|void + * Inspect the group by params to determine group by information. */ - public function groupBy($tableCol = FALSE) { - $this->_groupBy = ""; + public function setGroupByInformation() { if (!empty($this->_params['group_bys']) && is_array($this->_params['group_bys']) ) { @@ -307,27 +335,40 @@ class CRM_Report_Form_Contribute_Repeat extends CRM_Report_Form { if (array_key_exists('group_bys', $table)) { foreach ($table['group_bys'] as $fieldName => $field) { if (!empty($this->_params['group_bys'][$fieldName])) { - if ($tableCol) { - return array($tableName, $field['alias'], $field['name']); + $this->groupByTable = $tableName; + $this->groupByTableAlias = $field['alias']; + $this->groupByFieldName = $field['name']; + if ($this->groupByTable == 'civicrm_contact') { + $this->contributionJoinTableColumn = "contact_id"; + } + elseif ($this->groupByTable == 'civicrm_contribution_type') { + $this->contributionJoinTableColumn = "contribution_type_id"; } - else { - $this->_groupBy[] = "{$field['dbAlias']}"; + elseif ($this->groupByTable == 'civicrm_contribution') { + $this->contributionJoinTableColumn = $this->groupByFieldName; } + elseif ($this->groupByTable == 'civicrm_address') { + $this->contributionJoinTableColumn = "contact_id"; + } + elseif ($this->groupByTable == 'civicrm_financial_type') { + $this->contributionJoinTableColumn = 'financial_type_id'; + } + return; } } } } - $this->_groupBy = "GROUP BY " . implode(', ', $this->_groupBy); } } public function from() { - list($fromTable, $fromAlias, $fromCol) = $this->groupBy(TRUE); - $from = "$fromTable $fromAlias"; + $this->buildTempTables(); + $fromCol = $this->groupByFieldName; - if ($fromTable == 'civicrm_contact') { - $contriCol = "contact_id"; + $from = "$this->groupByTable $this->groupByTableAlias"; + + if ($this->groupByTable == 'civicrm_contact') { $from .= " LEFT JOIN civicrm_address {$this->_aliases['civicrm_address']} ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_address']}.contact_id LEFT JOIN civicrm_email {$this->_aliases['civicrm_email']} @@ -336,27 +377,22 @@ LEFT JOIN civicrm_phone {$this->_aliases['civicrm_phone']} ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_phone']}.contact_id AND {$this->_aliases['civicrm_phone']}.is_primary = 1"; } - elseif ($fromTable == 'civicrm_financial_type') { - $contriCol = "financial_type_id"; - } - elseif ($fromTable == 'civicrm_contribution') { - $contriCol = $fromCol; - } - elseif ($fromTable == 'civicrm_address') { + elseif ($this->groupByTable == 'civicrm_address') { $from .= " INNER JOIN civicrm_contact {$this->_aliases['civicrm_contact']} ON {$this->_aliases['civicrm_address']}.contact_id = {$this->_aliases['civicrm_contact']}.id"; - $fromAlias = $this->_aliases['civicrm_contact']; + $this->groupByTableAlias = $this->_aliases['civicrm_contact']; $fromCol = "id"; - $contriCol = "contact_id"; } $this->_from = " FROM $from -LEFT JOIN civicrm_temp_civireport_repeat1 {$this->_aliases['civicrm_contribution']}1 - ON $fromAlias.$fromCol = {$this->_aliases['civicrm_contribution']}1.$contriCol -LEFT JOIN civicrm_temp_civireport_repeat2 {$this->_aliases['civicrm_contribution']}2 - ON $fromAlias.$fromCol = {$this->_aliases['civicrm_contribution']}2.$contriCol"; +LEFT JOIN $this->tempTableRepeat1 {$this->_aliases['civicrm_contribution']}1 + ON {$this->groupByTableAlias}.$fromCol = {$this->_aliases['civicrm_contribution']}1 + .{$this->contributionJoinTableColumn} +LEFT JOIN $this->tempTableRepeat2 {$this->_aliases['civicrm_contribution']}2 + ON {$this->groupByTableAlias}.$fromCol = {$this->_aliases['civicrm_contribution']}2.{$this->contributionJoinTableColumn}"; } + /** * @param string $replaceAliasWith * @@ -372,6 +408,7 @@ LEFT JOIN civicrm_temp_civireport_repeat2 {$this->_aliases['civicrm_contribution $this->_where = ''; return $from; } + /** * @param string $replaceAliasWith * @@ -778,70 +815,6 @@ GROUP BY currency public function postProcess() { $this->beginPostProcess(); - $create = $subSelect1 = $subSelect2 = NULL; - list($fromTable, $fromAlias, $fromCol) = $this->groupBy(TRUE); - if ($fromTable == 'civicrm_contact') { - $contriCol = "contact_id"; - } - elseif ($fromTable == 'civicrm_contribution_type') { - $contriCol = "contribution_type_id"; - } - elseif ($fromTable == 'civicrm_contribution') { - $contriCol = $fromCol; - } - elseif ($fromTable == 'civicrm_address') { - $contriCol = "contact_id"; - } - elseif ($fromTable == 'civicrm_financial_type') { - $contriCol = 'financial_type_id'; - $subSelect1 = 'contribution1.contact_id,'; - $subSelect2 = 'contribution2.contact_id,'; - $create = 'contact_id int unsigned,'; - } - - $subWhere = $this->whereContribution(); - $from = $this->fromContribution(); - $subContributionQuery1 = " -SELECT {$subSelect1} contribution1.{$contriCol}, - sum( contribution1.total_amount ) AS total_amount_sum, - count( * ) AS total_amount_count -{$from} -{$subWhere} -GROUP BY contribution1.{$contriCol}"; - - $subWhere = $this->whereContribution('contribution2'); - $from = $this->fromContribution('contribution2'); - $subContributionQuery2 = " -SELECT {$subSelect2} contribution2.{$contriCol}, - sum( contribution2.total_amount ) AS total_amount_sum, - count( * ) AS total_amount_count, - currency -{$from} -{$subWhere} -GROUP BY contribution2.{$contriCol}"; - - $sql = " -CREATE TEMPORARY TABLE civicrm_temp_civireport_repeat1 ( -{$create} -{$contriCol} int unsigned, -total_amount_sum int, -total_amount_count int -) ENGINE=HEAP DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci"; - CRM_Core_DAO::executeQuery($sql); - $sql = "INSERT INTO civicrm_temp_civireport_repeat1 {$subContributionQuery1}"; - CRM_Core_DAO::executeQuery($sql); - - $sql = " -CREATE TEMPORARY TABLE civicrm_temp_civireport_repeat2 ( -{$create} -{$contriCol} int unsigned, -total_amount_sum int, -total_amount_count int, -currency varchar(3) -) ENGINE=HEAP DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci"; - CRM_Core_DAO::executeQuery($sql); - $sql = "INSERT INTO civicrm_temp_civireport_repeat2 {$subContributionQuery2}"; - CRM_Core_DAO::executeQuery($sql); $this->select(); $this->from(); @@ -1006,4 +979,74 @@ currency varchar(3) // foreach ends } + /** + * Build the temp tables for comparison. + */ + protected function buildTempTables() { + $this->setGroupByInformation(); + $create = $subSelect1 = $subSelect2 = NULL; + if ($this->tempTableRepeat1) { + return; + } + + if ($this->groupByTable == 'civicrm_financial_type') { + $subSelect1 = 'contribution1.contact_id,'; + $subSelect2 = 'contribution2.contact_id,'; + $create = 'contact_id int unsigned,'; + } + + $subWhere = $this->whereContribution(); + $from = $this->fromContribution(); + $subContributionQuery1 = " +SELECT {$subSelect1} contribution1.{$this->contributionJoinTableColumn}, + sum( contribution1.total_amount ) AS total_amount_sum, + count( * ) AS total_amount_count +{$from} +{$subWhere} +GROUP BY contribution1.{$this->contributionJoinTableColumn}"; + + $subWhere = $this->whereContribution('contribution2'); + $from = $this->fromContribution('contribution2'); + $subContributionQuery2 = " +SELECT {$subSelect2} contribution2.{$this->contributionJoinTableColumn}, + sum( contribution2.total_amount ) AS total_amount_sum, + count( * ) AS total_amount_count, + currency +{$from} +{$subWhere} +GROUP BY contribution2.{$this->contributionJoinTableColumn}"; + $this->tempTableRepeat1 = 'civicrm_temp_civireport_repeat1' . uniqid(); + $sql = " +CREATE TEMPORARY TABLE $this->tempTableRepeat1 ( +{$create} +{$this->contributionJoinTableColumn} int unsigned, +total_amount_sum int, +total_amount_count int +) ENGINE=HEAP DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci"; + CRM_Core_DAO::executeQuery($sql); + CRM_Core_DAO::executeQuery("INSERT INTO $this->tempTableRepeat1 {$subContributionQuery1}"); + + CRM_Core_DAO::executeQuery(" + ALTER TABLE $this->tempTableRepeat1 ADD INDEX ({$this->contributionJoinTableColumn}) + "); + + $this->tempTableRepeat2 = 'civicrm_temp_civireport_repeat2' . uniqid(); + $sql = " +CREATE TEMPORARY TABLE $this->tempTableRepeat2 ( +{$create} +{$this->contributionJoinTableColumn} int unsigned, +total_amount_sum int, +total_amount_count int, +currency varchar(3) +) ENGINE=HEAP DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci"; + CRM_Core_DAO::executeQuery($sql); + $sql = "INSERT INTO $this->tempTableRepeat2 {$subContributionQuery2}"; + CRM_Core_DAO::executeQuery($sql); + + CRM_Core_DAO::executeQuery(" + ALTER TABLE $this->tempTableRepeat2 ADD INDEX ({$this->contributionJoinTableColumn}) + "); + + } + } diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index a617455584..2a09ff1d96 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/tests/phpunit/api/v3/ReportTemplateTest.php @@ -181,7 +181,6 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase { public static function getReportTemplates() { $reportsToSkip = array( 'activity' => 'does not respect function signature on from clause', - 'contribute/repeat' => 'Reports with important functionality in postProcess are not callable via the api. For variable setting recommend beginPostProcessCommon, for temp table creation recommend From fn', 'contribute/topDonor' => 'construction of query in postProcess makes inaccessible ', 'event/income' => 'I do no understand why but error is Call to undefined method CRM_Report_Form_Event_Income::from() in CRM/Report/Form.php on line 2120', 'logging/contact/summary' => '(likely to be test related) probably logging off Undefined index: Form/Contact/LoggingSummary.php(231): PHP', -- 2.25.1