Skip to content
Snippets Groups Projects
New look: Off

Extraneous queries - activiies

    • Closed Issue created by eileen @eileen

    Per #2033 (closed) when doing query analysis on creating 7 contacts with 7 contributions I found that around 10% of the queries were looking up activity contacts.

    In other words 2 queries for each record type for each contribution. In fact these are new contributions so no existing records exist & that can hopefully be pre-determined

    Examples

    timestamp query seconds rows found columns requested
    15/09/20 2:54 SELECT * FROM civicrm_activity_contact WHERE ( civicrm_activity_contact.activity_id = 112183025 ) AND ( civicrm_activity_contact.contact_id = 46011835 ) AND ( civicrm_activity_contact.record_type_id = 2 ) 0.000549 0 0
    15/09/20 2:54 SELECT * FROM civicrm_activity_contact WHERE ( civicrm_activity_contact.activity_id = 112183025 ) AND ( civicrm_activity_contact.record_type_id = 1 ) 0.000606 0 0
    15/09/20 2:54 SELECT * FROM civicrm_activity_contact WHERE ( civicrm_activity_contact.activity_id = 112183025 ) AND ( civicrm_activity_contact.record_type_id = 2 ) 0.000725 0 0
    15/09/20 2:54 SELECT * FROM civicrm_activity_contact WHERE ( civicrm_activity_contact.activity_id = 112183025 ) AND ( civicrm_activity_contact.record_type_id = 2 ) 0.000605 1 1
    15/09/20 2:54 SELECT * FROM civicrm_activity_contact WHERE ( civicrm_activity_contact.activity_id = 112183025 ) AND ( civicrm_activity_contact.record_type_id = 3 ) 0.000636 0 0
    To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information

    Child items ...

    • Activity

    • eileen added sig:performance label

      added sig:performance label

    • eileen mentioned in issue #2033 (closed)

      mentioned in issue #2033 (closed)

    • jaapjansma added triaged label

      added triaged label

    • totten mentioned in commit 8293f7c2

      mentioned in commit 8293f7c2

    • colemanw mentioned in commit ef271804

      mentioned in commit ef271804

    • totten mentioned in commit 8b1f38dc

      mentioned in commit 8b1f38dc

    • totten mentioned in commit 1acaddb8

      mentioned in commit 1acaddb8

    • totten mentioned in commit 36fa1940

      mentioned in commit 36fa1940

    • DaveD
      DaveD @DaveD ·
      Maintainer

      While looking at https://github.com/civicrm/civicrm-core/pull/18609#issuecomment-699673757 some preexisting issues with passing scalar target/assignee parameters to Activity::create() came up. They appear fixed in https://github.com/civicrm/civicrm-core/pull/18625, so documenting the fails for fun:

      .........................F...F..E............F.FF.E              52 / 52 (100%)

There were 2 errors:

1) CRM_Activity_BAO_ActivityTest::testTargetAssigneeVariations with data set 33 (array(array(1, 2), array()), array(array(2), array()))
Exception: PEAR_Exception: "DB Error: already exists"
 * ERROR TYPE: DB_Error
 * ERROR CODE: -5
 * ERROR MESSAGE: DB Error: already exists
 * ERROR MODE: 16
 * ERROR USERINFO: UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 5 , `record_type_id` = 3   WHERE (  `civicrm_activity_contact`.`id` = 2 )   [nativecode=1062 ** Duplicate entry '5-1-3' for key 'UI_activity_contact']
 * ERROR DEBUGINFO: UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 5 , `record_type_id` = 3   WHERE (  `civicrm_activity_contact`.`id` = 2 )   [nativecode=1062 ** Duplicate entry '5-1-3' for key 'UI_activity_contact']
0 ...\sites\all\modules\civicrm\vendor\pear\pear-core-minimal\src\PEAR.php(922): CRM_Core_Error::exceptionHandler(Object(DB_Error))
1 ...\sites\all\modules\civicrm\vendor\pear\db\DB.php(997): PEAR_Error->__construct("DB Error: already exists", -5, 16, (Array:2), "UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 5 ...")
2 ...\sites\all\modules\civicrm\vendor\pear\pear-core-minimal\src\PEAR.php(575): DB_Error->__construct(-5, 16, (Array:2), "UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 5 ...")
3 ...\sites\all\modules\civicrm\vendor\pear\pear-core-minimal\src\PEAR.php(223): PEAR->_raiseError(Object(DB_mysqli), NULL, -5, 16, (Array:2), "UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 5 ...", "DB_Error", TRUE)
4 ...\sites\all\modules\civicrm\vendor\pear\db\DB\common.php(1928): PEAR->__call("raiseError", (Array:7))
5 ...\sites\all\modules\civicrm\vendor\pear\db\DB\mysqli.php(936): DB_common->raiseError(-5, NULL, NULL, "UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 5 ...", "1062 ** Duplicate entry '5-1-3' for key 'UI_activity_contact'")
6 ...\sites\all\modules\civicrm\vendor\pear\db\DB\mysqli.php(406): DB_mysqli->mysqliRaiseError()
7 ...\sites\all\modules\civicrm\vendor\pear\db\DB\common.php(1234): DB_mysqli->simpleQuery("UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 5 ...")
8 ...\sites\all\modules\civicrm\packages\DB\DataObject.php(2696): DB_common->query("UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 5 ...")
9 ...\sites\all\modules\civicrm\packages\DB\DataObject.php(1539): DB_DataObject->_query("UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 5 ...")
10 ...\sites\all\modules\civicrm\CRM\Core\DAO.php(625): DB_DataObject->update()
11 ...\sites\all\modules\civicrm\CRM\Activity\BAO\ActivityContact.php(44): CRM_Core_DAO->save()
12 ...\sites\all\modules\civicrm\CRM\Activity\BAO\Activity.php(456): CRM_Activity_BAO_ActivityContact::create((Array:4))
13 ...\sites\all\modules\civicrm\tests\phpunit\CRM\Activity\BAO\ActivityTest.php(1687): CRM_Activity_BAO_Activity::create((Array:8))
14 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestCase.php(1154): CRM_Activity_BAO_ActivityTest->testTargetAssigneeVariations((Array:2), (Array:2))
15 ...\sites\all\modules\civicrm\tests\phpunit\CiviTest\CiviUnitTestCase.php(226): PHPUnit\Framework\TestCase->runTest()
16 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestCase.php(842): CiviUnitTestCase->runTest()
17 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestResult.php(693): PHPUnit\Framework\TestCase->runBare()
18 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestCase.php(796): PHPUnit\Framework\TestResult->run(Object(CRM_Activity_BAO_ActivityTest))
19 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestSuite.php(746): PHPUnit\Framework\TestCase->run(Object(PHPUnit\Framework\TestResult))
20 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestSuite.php(746): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
21 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/TextUI/TestRunner.php(652): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
22 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/TextUI/Command.php(206): PHPUnit\TextUI\TestRunner->doRun(Object(PHPUnit\Framework\TestSuite), (Array:63), TRUE)
23 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/TextUI/Command.php(162): PHPUnit\TextUI\Command->run((Array:4), TRUE)
24 ...\sites\all\modules\civicrm\phpunit7(620): PHPUnit\TextUI\Command::main()
25 {main}


...\sites\all\modules\civicrm\tests\phpunit\CiviTest\CiviUnitTestCase.php:230
...\sites\all\modules\civicrm\phpunit7:620

2) CRM_Activity_BAO_ActivityTest::testTargetAssigneeVariations with data set 51 (array(array(), array(3, 4)), array(array(), array(4)))
Exception: PEAR_Exception: "DB Error: already exists"
 * ERROR TYPE: DB_Error
 * ERROR CODE: -5
 * ERROR MESSAGE: DB Error: already exists
 * ERROR MODE: 16
 * ERROR USERINFO: UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 7 , `record_type_id` = 1   WHERE (  `civicrm_activity_contact`.`id` = 2 )   [nativecode=1062 ** Duplicate entry '7-1-1' for key 'UI_activity_contact']
 * ERROR DEBUGINFO: UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 7 , `record_type_id` = 1   WHERE (  `civicrm_activity_contact`.`id` = 2 )   [nativecode=1062 ** Duplicate entry '7-1-1' for key 'UI_activity_contact']
0 ...\sites\all\modules\civicrm\vendor\pear\pear-core-minimal\src\PEAR.php(922): CRM_Core_Error::exceptionHandler(Object(DB_Error))
1 ...\sites\all\modules\civicrm\vendor\pear\db\DB.php(997): PEAR_Error->__construct("DB Error: already exists", -5, 16, (Array:2), "UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 7 ...")
2 ...\sites\all\modules\civicrm\vendor\pear\pear-core-minimal\src\PEAR.php(575): DB_Error->__construct(-5, 16, (Array:2), "UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 7 ...")
3 ...\sites\all\modules\civicrm\vendor\pear\pear-core-minimal\src\PEAR.php(223): PEAR->_raiseError(Object(DB_mysqli), NULL, -5, 16, (Array:2), "UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 7 ...", "DB_Error", TRUE)
4 ...\sites\all\modules\civicrm\vendor\pear\db\DB\common.php(1928): PEAR->__call("raiseError", (Array:7))
5 ...\sites\all\modules\civicrm\vendor\pear\db\DB\mysqli.php(936): DB_common->raiseError(-5, NULL, NULL, "UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 7 ...", "1062 ** Duplicate entry '7-1-1' for key 'UI_activity_contact'")
6 ...\sites\all\modules\civicrm\vendor\pear\db\DB\mysqli.php(406): DB_mysqli->mysqliRaiseError()
7 ...\sites\all\modules\civicrm\vendor\pear\db\DB\common.php(1234): DB_mysqli->simpleQuery("UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 7 ...")
8 ...\sites\all\modules\civicrm\packages\DB\DataObject.php(2696): DB_common->query("UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 7 ...")
9 ...\sites\all\modules\civicrm\packages\DB\DataObject.php(1539): DB_DataObject->_query("UPDATE  `civicrm_activity_contact`  SET `activity_id` = 1 , `contact_id` = 7 ...")
10 ...\sites\all\modules\civicrm\CRM\Core\DAO.php(625): DB_DataObject->update()
11 ...\sites\all\modules\civicrm\CRM\Activity\BAO\ActivityContact.php(44): CRM_Core_DAO->save()
12 ...\sites\all\modules\civicrm\CRM\Activity\BAO\Activity.php(414): CRM_Activity_BAO_ActivityContact::create((Array:4))
13 ...\sites\all\modules\civicrm\tests\phpunit\CRM\Activity\BAO\ActivityTest.php(1687): CRM_Activity_BAO_Activity::create((Array:8))
14 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestCase.php(1154): CRM_Activity_BAO_ActivityTest->testTargetAssigneeVariations((Array:2), (Array:2))
15 ...\sites\all\modules\civicrm\tests\phpunit\CiviTest\CiviUnitTestCase.php(226): PHPUnit\Framework\TestCase->runTest()
16 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestCase.php(842): CiviUnitTestCase->runTest()
17 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestResult.php(693): PHPUnit\Framework\TestCase->runBare()
18 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestCase.php(796): PHPUnit\Framework\TestResult->run(Object(CRM_Activity_BAO_ActivityTest))
19 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestSuite.php(746): PHPUnit\Framework\TestCase->run(Object(PHPUnit\Framework\TestResult))
20 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/Framework/TestSuite.php(746): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
21 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/TextUI/TestRunner.php(652): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
22 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/TextUI/Command.php(206): PHPUnit\TextUI\TestRunner->doRun(Object(PHPUnit\Framework\TestSuite), (Array:63), TRUE)
23 phar://.../sites/all/modules/civicrm/phpunit7/phpunit/TextUI/Command.php(162): PHPUnit\TextUI\Command->run((Array:4), TRUE)
24 ...\sites\all\modules\civicrm\phpunit7(620): PHPUnit\TextUI\Command::main()
25 {main}


...\sites\all\modules\civicrm\tests\phpunit\CiviTest\CiviUnitTestCase.php:230
...\sites\all\modules\civicrm\phpunit7:620

--

There were 5 failures:

1) CRM_Activity_BAO_ActivityTest::testTargetAssigneeVariations with data set 26 (array(array(1, 2), array()), array(array(1), array()))
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 4
+    0 => '4'
+    1 => '5'
 )

...\sites\all\modules\civicrm\tests\phpunit\CRM\Activity\BAO\ActivityTest.php:1694
...\sites\all\modules\civicrm\tests\phpunit\CiviTest\CiviUnitTestCase.php:226
...\sites\all\modules\civicrm\phpunit7:620

2) CRM_Activity_BAO_ActivityTest::testTargetAssigneeVariations with data set 30 (array(array(1, 2), array()), array(array(1), array(3)))
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 4
+    0 => '4'
+    1 => '5'
 )

...\sites\all\modules\civicrm\tests\phpunit\CRM\Activity\BAO\ActivityTest.php:1694
...\sites\all\modules\civicrm\tests\phpunit\CiviTest\CiviUnitTestCase.php:226
...\sites\all\modules\civicrm\phpunit7:620

3) CRM_Activity_BAO_ActivityTest::testTargetAssigneeVariations with data set 46 (array(array(), array(3, 4)), array(array(), array(3)))
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 6
+    0 => '6'
+    1 => '7'
 )

...\sites\all\modules\civicrm\tests\phpunit\CRM\Activity\BAO\ActivityTest.php:1701
...\sites\all\modules\civicrm\tests\phpunit\CiviTest\CiviUnitTestCase.php:226
...\sites\all\modules\civicrm\phpunit7:620

4) CRM_Activity_BAO_ActivityTest::testTargetAssigneeVariations with data set 48 (array(array(), array(3, 4)), array(array(1), array(3)))
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 6
+    0 => '6'
+    1 => '7'
 )

...\sites\all\modules\civicrm\tests\phpunit\CRM\Activity\BAO\ActivityTest.php:1701
...\sites\all\modules\civicrm\tests\phpunit\CiviTest\CiviUnitTestCase.php:226
...\sites\all\modules\civicrm\phpunit7:620

5) CRM_Activity_BAO_ActivityTest::testTargetAssigneeVariations with data set 49 (array(array(), array(3, 4)), array(array(1, 2), array(3)))
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 6
+    0 => '6'
+    1 => '7'
 )

...\sites\all\modules\civicrm\tests\phpunit\CRM\Activity\BAO\ActivityTest.php:1701
...\sites\all\modules\civicrm\tests\phpunit\CiviTest\CiviUnitTestCase.php:226
...\sites\all\modules\civicrm\phpunit7:620

ERRORS!
Tests: 52, Assertions: 530, Errors: 2, Failures: 5.
    • totten mentioned in commit 66cbbfb6

      mentioned in commit 66cbbfb6

    • totten mentioned in commit 186f87a3

      mentioned in commit 186f87a3

    • totten mentioned in commit 729e12ba

      mentioned in commit 729e12ba

    • eileen
      eileen @eileen ·
      Author Owner

      A bunch of things have been merged https://github.com/civicrm/civicrm-core/pulls?q=is%3Apr+%232057+is%3Aclosed+ to reduce queries on the principles

      1. don't look up activityContact right before to check if it exists - running the tests without this check did not find any instances where it found something, instead respond if it fails to insert (try-catch)
      2. don't look up activity contacts on new activities
      3. for edits - do just one query to look up if any of the activity contacts to be created already exist, cull these so no attempt is made to re-add any that are removed
      4. for edits - do one query with LIMIT 1 to find if there are any to remove (the idea being that deletes are more locking than selects so if there is zero skip the select - note the risk of there being many was the reason I didn't grab them all in step 3)
      5. if necessary do a delete

      OUTSTANDING

      @colemanw I have used

          if (!empty($activityContactApiValues)) {
      ActivityContact::save($params['check_permissions'] ?? FALSE)->addDefault('activity_id', $activityId)
        ->setRecords($activityContactApiValues)->execute();
    }

      because it was easier than

          if (!empty($activityContactApiValues)) {
     if ($activity->id) {
       ActivityContact::update....
     }  
     else {
      ActivityContact::create.....

      But it looks like this adds a unnecessary query on create?

    • ****
      **** @support_bot_dev ·
      Reporter

      Hi,

      Thank you for raising an issue to help improve CiviCRM. As you may know, this issue has not had any activity for quite some time, so we have closed it.

      We would like you to help us to determine if this issue should be re-opened:

      • If this issue was reporting a bug, can you attempt to reproduce it on a latest version of CiviCRM?
      • If this issue was proposing a new feature, can you verify if the feature proposal is still relevant? Did it get the concept-approved label? Have other people also shown interest? Could it be implemented as an extension?

      If the answer to either question is yes, please feel free to comment or re-open the issue. Please also consider:

      • Is it something that you could help implement, either by sending a patch or hiring someone who can?
      • Would you be willing to fund the work, either through a CiviCRM partner, or the Core Team?

      Thank you for your help and contributions to CiviCRM.

      P.S. This is an automated message, see infra/gitlab issue 20. We understand that automatic responses are annoying, but given the number of open issues as the project evolves, we need a bit of help to triage and prioritise the most relevant issues.

    • **** closed

      closed

    Please register or sign in to reply