From: eileen <emcnaughton@wikimedia.org>
Date: Wed, 14 Nov 2018 03:27:30 +0000 (+1300)
Subject: dev/core#518 Fix lybunt performance
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=1679e19cec0e220e83e8bd069db0070b165ce854;p=civicrm-core.git

dev/core#518 Fix lybunt performance

Per analysis on the issue this gave me a substantial performance
increase (from nearly 7 minutes down to less than a quarter of a second)
when performing a lybunt report on a small group wihtin a large database.

In testing this patch we went from WSOD of death to nearly instant responsiveness.

I tried the query unfiltered (ie. against our whole DB) and it performed well
although not well enough I would want our users to run it on prod (~12 minutes)
- so I feel comfortable this doesn't regress performance when a group filter is
NOT in play
---

diff --git a/CRM/Report/Form/Contribute/Lybunt.php b/CRM/Report/Form/Contribute/Lybunt.php
index a7aeaee903..f7f9fca90d 100644
--- a/CRM/Report/Form/Contribute/Lybunt.php
+++ b/CRM/Report/Form/Contribute/Lybunt.php
@@ -364,9 +364,7 @@ class CRM_Report_Form_Contribute_Lybunt extends CRM_Report_Form {
       $this->_from .= " ON {$this->_aliases['civicrm_contribution']}.contact_id = {$this->_aliases['civicrm_contact']}.id
          AND {$this->_aliases['civicrm_contribution']}.is_test = 0
          AND " . $this->whereClauseLastYear("{$this->_aliases['civicrm_contribution']}.receive_date") . "
-       {$this->_aclFrom}
-       LEFT JOIN civicrm_contribution cont_exclude ON cont_exclude.contact_id = {$this->_aliases['civicrm_contact']}.id
-         AND " . $this->whereClauseThisYear('cont_exclude.receive_date');
+       {$this->_aclFrom} ";
       $this->selectivelyAddLocationTablesJoinsToFilterQuery();
     }
 
@@ -399,7 +397,11 @@ class CRM_Report_Form_Contribute_Lybunt extends CRM_Report_Form {
     if ($field['name'] == 'receive_date') {
       $clause = 1;
       if (empty($this->contactTempTable)) {
-        $this->_whereClauses[] = "cont_exclude.id IS NULL";
+        $clause = "{$this->_aliases['civicrm_contact']}.id NOT IN (
+          SELECT cont_exclude.contact_id
+          FROM civicrm_contribution cont_exclude
+          WHERE " . $this->whereClauseThisYear('cont_exclude.receive_date')
+        . ")";
       }
     }
     // Group filtering is already done so skip.
diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php
index 630645dfb0..91dec1b956 100644
--- a/tests/phpunit/api/v3/ReportTemplateTest.php
+++ b/tests/phpunit/api/v3/ReportTemplateTest.php
@@ -431,15 +431,19 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
     $this->assertEquals(2, $rows['count'], "Report failed - the sql used to generate the results was " . print_r($rows['metadata']['sql'], TRUE));
 
-    $expected = 'DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci
+    $expected = preg_replace('/\s+/', ' ', 'DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci
       SELECT SQL_CALC_FOUND_ROWS contact_civireport.id as cid  FROM civicrm_contact contact_civireport    INNER JOIN civicrm_contribution contribution_civireport USE index (received_date) ON contribution_civireport.contact_id = contact_civireport.id
          AND contribution_civireport.is_test = 0
          AND contribution_civireport.receive_date BETWEEN \'20140701000000\' AND \'20150630235959\'
-
-       LEFT JOIN civicrm_contribution cont_exclude ON cont_exclude.contact_id = contact_civireport.id
-         AND cont_exclude.receive_date BETWEEN \'2015-7-1\' AND \'20160630235959\' WHERE cont_exclude.id IS NULL AND 1 AND ( contribution_civireport.contribution_status_id IN (1) )
-      GROUP BY contact_civireport.id';
-    $this->assertContains(preg_replace('/\s+/', ' ', $expected), preg_replace('/\s+/', ' ', $rows['metadata']['sql'][0]));
+         WHERE contact_civireport.id NOT IN (
+      SELECT cont_exclude.contact_id
+          FROM civicrm_contribution cont_exclude
+          WHERE cont_exclude.receive_date BETWEEN \'2015-7-1\' AND \'20160630235959\')
+          AND ( contribution_civireport.contribution_status_id IN (1) )
+      GROUP BY contact_civireport.id');
+    // Exclude whitespace in comparison as we don't care if it changes & this allows us to make the above readable.
+    $whitespacelessSql = preg_replace('/\s+/', ' ', $rows['metadata']['sql'][0]);
+    $this->assertContains($expected, $whitespacelessSql);
   }
 
   /**