From 43c1fa192f5ef4e0b34877a49bf0bc5bfe9e307c Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Aug 2016 20:32:07 +1200 Subject: [PATCH] CRM-19170 optimise group filter on contribution summary report. This reduced the time in testing from 190+ seconds (killed at that point) to ~1 second for a report with a small result set on a large DB --- CRM/Report/Form.php | 176 ++++++++++++++++++-- CRM/Report/Form/Contribute/Summary.php | 14 +- tests/phpunit/api/v3/ReportTemplateTest.php | 2 +- 3 files changed, 177 insertions(+), 15 deletions(-) diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index 722ad70f0c..5e278fdd08 100644 --- a/CRM/Report/Form.php +++ b/CRM/Report/Form.php @@ -152,6 +152,26 @@ class CRM_Report_Form extends CRM_Core_Form { */ protected $_groupFilter = FALSE; + /** + * Has the report been optimised for group filtering. + * + * The functionality for group filtering has been improved but not + * all reports have been adjusted to take care of it. + * + * This property exists to highlight the reports which are still using the + * slow method & allow group filtering to still work for them until they + * can be migrated. + * + * In order to protect extensions we have to default to TRUE - but I have + * separately marked every class with a groupFilter in the hope that will trigger + * people to fix them as they touch them. + * + * CRM-19170 + * + * @var bool + */ + protected $groupFilterNotOptimised = TRUE; + /** * Navigation fields * @@ -219,6 +239,13 @@ class CRM_Report_Form extends CRM_Core_Form { protected $_selectAliases = array(); protected $_rollup = NULL; + /** + * Table containing list of contact IDs within the group filter. + * + * @var string + */ + protected $groupTempTable = ''; + /** * @var array */ @@ -1304,15 +1331,16 @@ class CRM_Report_Form extends CRM_Core_Form { $this->assignTabs(); $this->sqlArray[] = $sql; - foreach (array('LEFT JOIN') as $term) { - $sql = str_replace($term, '
  ' . $term, $sql); - } - foreach (array('FROM', 'WHERE', 'GROUP BY', 'ORDER BY', 'LIMIT', ';') as $term) { - $sql = str_replace($term, '

' . $term, $sql); + foreach ($this->sqlArray as $sql) { + foreach (array('LEFT JOIN') as $term) { + $sql = str_replace($term, '
  ' . $term, $sql); + } + foreach (array('FROM', 'WHERE', 'GROUP BY', 'ORDER BY', 'LIMIT', ';') as $term) { + $sql = str_replace($term, '

' . $term, $sql); + } + $this->sqlFormattedArray[] = $sql; + $this->assign('sql', implode(';



', $this->sqlFormattedArray)); } - $this->sql .= $sql . "
"; - - $this->assign('sql', $this->sql); } /** @@ -1847,7 +1875,7 @@ class CRM_Report_Form extends CRM_Core_Form { case 'in': case 'notin': - if (is_string($value) && strlen($value)) { + if ((is_string($value) || is_numeric($value)) && strlen($value)) { $value = explode(',', $value); } if ($value !== NULL && is_array($value) && count($value) > 0) { @@ -2620,6 +2648,7 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND * @return string */ public function buildQuery($applyLimit = TRUE) { + $this->buildGroupTempTable(); $this->select(); $this->from(); $this->customDataFrom(); @@ -3354,7 +3383,12 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND } /** - * Build where clause for groups. + * Build a group filter with contempt for large data sets. + * + * This function has been retained as it takes time to migrate the reports over + * to the new method which will not crash on large datasets. + * + * @deprecated * * @param string $field * @param mixed $value @@ -3362,8 +3396,7 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND * * @return string */ - public function whereGroupClause($field, $value, $op) { - + public function legacySlowGroupFilterClause($field, $value, $op) { $smartGroupQuery = ""; $group = new CRM_Contact_DAO_Group(); @@ -3406,6 +3439,83 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND {$smartGroupQuery} ) "; } + /** + * Build where clause for groups. + * + * @param string $field + * @param mixed $value + * @param string $op + * + * @return string + */ + public function whereGroupClause($field, $value, $op) { + if ($this->groupFilterNotOptimised) { + return $this->legacySlowGroupFilterClause($field, $value, $op); + } + if ($op === 'notin') { + return " group_temp_table.id IS NULL "; + } + // We will have used an inner join instead. + return "1"; + } + + + /** + * Create a table of the contact ids included by the group filter. + * + * This function is called by both the api (tests) and the UI. + */ + public function buildGroupTempTable() { + if (!empty($this->groupTempTable) || empty ($this->_params['gid_value']) || $this->groupFilterNotOptimised) { + return; + } + $filteredGroups = (array) $this->_params['gid_value']; + + $groups = civicrm_api3('Group', 'get', array( + 'is_active' => 1, + 'id' => array('IN' => $filteredGroups), + 'saved_search_id' => array('>' => 0), + 'return' => 'id', + )); + $smartGroups = array_keys($groups['values']); + + $query = " + SELECT group_contact.contact_id as id + FROM civicrm_group_contact group_contact + WHERE group_contact.group_id IN (" . implode(', ', $filteredGroups) . ") + AND group_contact.status = 'Added' "; + + if (!empty($smartGroups)) { + CRM_Contact_BAO_GroupContactCache::check($smartGroups); + $smartGroups = implode(',', $smartGroups); + $query .= " + UNION DISTINCT + SELECT smartgroup_contact.contact_id as id + FROM civicrm_group_contact_cache smartgroup_contact + WHERE smartgroup_contact.group_id IN ({$smartGroups}) "; + } + + $this->groupTempTable = 'civicrm_report_temp_group_' . date('Ymd_') . uniqid(); + $this->executeReportQuery(" + CREATE TEMPORARY TABLE $this->groupTempTable + $query + "); + CRM_Core_DAO::executeQuery("ALTER TABLE $this->groupTempTable ADD INDEX i_id(id)"); + } + + /** + * Execute query and add it to the developer tab. + * + * @param string $query + * @param array $params + * + * @return \CRM_Core_DAO|object + */ + protected function executeReportQuery($query, $params = array()) { + $this->addToDeveloperTab($query); + return CRM_Core_DAO::executeQuery($query, $params); + } + /** * Build where clause for tags. * @@ -4708,4 +4818,46 @@ LEFT JOIN civicrm_contact {$field['alias']} ON {$field['alias']}.id = {$this->_a } } + /** + * Set the base table for the FROM clause. + * + * Sets up the from clause, allowing for the possibility it might be a + * temp table pre-filtered by groups if a group filter is in use. + * + * @param string $baseTable + * @param string $field + * @param null $tableAlias + */ + public function setFromBase($baseTable, $field = 'id', $tableAlias = NULL) { + if (!$tableAlias) { + $tableAlias = $this->_aliases[$baseTable]; + } + $this->_from = $this->_from = " FROM $baseTable $tableAlias "; + $this->joinGroupTempTable($baseTable, $field, $tableAlias); + $this->_from .= " {$this->_aclFrom} "; + } + + /** + * Join the temp table contacting contacts who are members of the filtered groups. + * + * If we are using an IN filter we use an inner join, otherwise a left join. + * + * @param string $baseTable + * @param string $field + * @param string $tableAlias + */ + public function joinGroupTempTable($baseTable, $field, $tableAlias) { + if ($this->groupTempTable) { + if ($this->_params['gid_op'] == 'in') { + $this->_from = " FROM $this->groupTempTable group_temp_table INNER JOIN $baseTable $tableAlias + ON group_temp_table.id = $tableAlias.{$field} "; + } + else { + $this->_from .= " + LEFT JOIN $this->groupTempTable group_temp_table + ON $tableAlias.{$field} = group_temp_table.id "; + } + } + } + } diff --git a/CRM/Report/Form/Contribute/Summary.php b/CRM/Report/Form/Contribute/Summary.php index 564ed1ac1e..b70a3f4f53 100644 --- a/CRM/Report/Form/Contribute/Summary.php +++ b/CRM/Report/Form/Contribute/Summary.php @@ -57,6 +57,15 @@ class CRM_Report_Form_Contribute_Summary extends CRM_Report_Form { 'FISCALYEAR' => 'Fiscal Year', ); + /** + * This report has been optimised for group filtering. + * + * CRM-19170 + * + * @var bool + */ + protected $groupFilterNotOptimised = FALSE; + /** * Class constructor. */ @@ -462,8 +471,9 @@ class CRM_Report_Form_Contribute_Summary extends CRM_Report_Form { $softCreditJoin .= " AND {$this->_aliases['civicrm_contribution_soft']}.id = (SELECT MIN(id) FROM civicrm_contribution_soft cs WHERE cs.contribution_id = {$this->_aliases['civicrm_contribution']}.id) "; } - $this->_from = " - FROM civicrm_contact {$this->_aliases['civicrm_contact']} {$this->_aclFrom} + $this->setFromBase('civicrm_contact'); + + $this->_from .= " INNER JOIN civicrm_contribution {$this->_aliases['civicrm_contribution']} ON {$this->_aliases['civicrm_contact']}.id = {$this->_aliases['civicrm_contribution']}.contact_id AND {$this->_aliases['civicrm_contribution']}.is_test = 0 diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index 2e26a2710d..f20a713b51 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/tests/phpunit/api/v3/ReportTemplateTest.php @@ -309,7 +309,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase { $rows = $this->callAPISuccess('report_template', 'getrows', array( 'report_id' => 'contribute/summary', 'gid_value' => $groupID, - 'gid_op' => 'not in', + 'gid_op' => 'notin', 'options' => array('metadata' => array('sql')), )); $this->assertEquals(2, $rows['values'][0]['civicrm_contribution_total_amount_count']); -- 2.25.1