Skip to content

Commit

Permalink
Merge pull request #11732 from twomice/CRM-21811_optimize_search_reci…
Browse files Browse the repository at this point in the history 
…procal_relationship_group

CRM-21811: Optimize advanced search by relationship with target g…
  • Loading branch information
@seamuslee001
seamuslee001 authored May 22, 2018
2 parents 32a12b5 + 72a2eea commit 0e4e454
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 17 deletions.
38 changes: 21 additions & 17 deletions CRM/Contact/BAO/Query.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3078,14 +3078,18 @@ public function getGroupsFromTypeCriteria($value) {
}

/**
* @param array $groups
* @param string $tableAlias
* @param string $joinTable
* @param string $op
* Prime smart group cache for smart groups in the search, and join
* civicrm_group_contact_cache table into the query.
*
* @param array $groups IDs of groups specified in search criteria.
* @param string $tableAlias Alias to use for civicrm_group_contact_cache table.
* @param string $joinTable Table on which to join civicrm_group_contact_cache
* @param string $op SQL comparison operator (NULL, IN, !=, IS NULL, etc.)
* @param string $joinColumn Column in $joinTable on which to join civicrm_group_contact_cache.contact_id
*
* @return null|string
* @return string WHERE clause component for smart group criteria.
*/
public function addGroupContactCache($groups, $tableAlias = NULL, $joinTable = "contact_a", $op) {
public function addGroupContactCache($groups, $tableAlias = NULL, $joinTable = "contact_a", $op, $joinColumn = 'id') {
$isNullOp = (strpos($op, 'NULL') !== FALSE);
$groupsIds = $groups;
if (!$isNullOp && !$groups) {
Expand Down Expand Up @@ -3126,7 +3130,7 @@ public function addGroupContactCache($groups, $tableAlias = NULL, $joinTable = "
$tableAlias .= ($isNullOp) ? "a`" : implode(',', (array) $groupsIds) . "`";
}

$this->_tables[$tableAlias] = $this->_whereTables[$tableAlias] = " LEFT JOIN civicrm_group_contact_cache {$tableAlias} ON {$joinTable}.id = {$tableAlias}.contact_id ";
$this->_tables[$tableAlias] = $this->_whereTables[$tableAlias] = " LEFT JOIN civicrm_group_contact_cache {$tableAlias} ON {$joinTable}.{$joinColumn} = {$tableAlias}.contact_id ";
return self::buildClause("{$tableAlias}.group_id", $op, $groups, 'Int');
}

Expand Down Expand Up @@ -4078,25 +4082,28 @@ public function relationship(&$values) {
$rTypeValue = (array) $rTypeValue;
if ($rTypeValue['name_a_b'] == $rTypeValue['name_b_a']) {
// if we don't know which end of the relationship we are dealing with we'll create a temp table
//@todo unless we are dealing with a target group
self::$_relType = 'reciprocal';
}
}
}
// if we are creating a temp table we build our own where for the relationship table
$relationshipTempTable = NULL;
if (self::$_relType == 'reciprocal' && empty($targetGroup)) {
if (self::$_relType == 'reciprocal') {
$where = array();
self::$_relationshipTempTable = $relationshipTempTable = CRM_Core_DAO::createTempTableName('civicrm_rel');
if ($nameClause) {
$where[$grouping][] = " sort_name $nameClause ";
}
$groupJoinTable = "civicrm_relationship";
$groupJoinColumn = "contact_id_alt";
}
else {
$where = &$this->_where;
if ($nameClause) {
$where[$grouping][] = "( contact_b.sort_name $nameClause AND contact_b.id != contact_a.id )";
}
$groupJoinTable = "contact_b";
$groupJoinColumn = "id";
}
$allRelationshipType = CRM_Contact_BAO_Relationship::getContactRelationshipType(NULL, 'null', NULL, NULL, TRUE);
if ($nameClause || !$targetGroup) {
Expand All @@ -4119,12 +4126,12 @@ public function relationship(&$values) {
if ($targetGroup) {
//add contacts from static groups
$this->_tables['civicrm_relationship_group_contact'] = $this->_whereTables['civicrm_relationship_group_contact']
= " LEFT JOIN civicrm_group_contact civicrm_relationship_group_contact ON civicrm_relationship_group_contact.contact_id = contact_b.id AND civicrm_relationship_group_contact.status = 'Added'";
= " LEFT JOIN civicrm_group_contact civicrm_relationship_group_contact ON civicrm_relationship_group_contact.contact_id = {$groupJoinTable}.{$groupJoinColumn} AND civicrm_relationship_group_contact.status = 'Added'";
$groupWhere[] = "( civicrm_relationship_group_contact.group_id IN (" .
implode(",", $targetGroup[2]) . ") ) ";

//add contacts from saved searches
$ssWhere = $this->addGroupContactCache($targetGroup[2], "civicrm_relationship_group_contact_cache", "contact_b", $op);
$ssWhere = $this->addGroupContactCache($targetGroup[2], "civicrm_relationship_group_contact_cache", $groupJoinTable, $op, $groupJoinColumn);

//set the group where clause
if ($ssWhere) {
Expand All @@ -4146,7 +4153,7 @@ public function relationship(&$values) {
if (!empty($relQill)) {
$relQill .= ' OR ';
}
$relQill .= $allRelationshipType[$rel];
$relQill .= CRM_Utils_Array::value($rel, $allRelationshipType);
}
$this->_qill[$grouping][] = 'Relationship Type(s) ' . $relQill . " ( " . implode(", ", $qillNames) . " )";
}
Expand Down Expand Up @@ -4206,9 +4213,6 @@ public function relationship(&$values) {
$this->_relationshipValuesAdded = TRUE;
// it could be a or b, using an OR creates an unindexed join - better to create a temp table &
// join on that,
// @todo creating a temp table could be expanded to group filter
// as even creating a temp table of all relationships is much much more efficient than
// an OR in the join
if ($relationshipTempTable) {
$whereClause = '';
if (!empty($where[$grouping])) {
Expand All @@ -4217,12 +4221,12 @@ public function relationship(&$values) {
}
$sql = "
CREATE TEMPORARY TABLE {$relationshipTempTable}
(SELECT contact_id_b as contact_id, civicrm_relationship.id
(SELECT contact_id_b as contact_id, contact_id_a as contact_id_alt, civicrm_relationship.id
FROM civicrm_relationship
INNER JOIN civicrm_contact c ON civicrm_relationship.contact_id_a = c.id
$whereClause )
UNION
(SELECT contact_id_a as contact_id, civicrm_relationship.id
(SELECT contact_id_a as contact_id, contact_id_b as contact_id_alt, civicrm_relationship.id
FROM civicrm_relationship
INNER JOIN civicrm_contact c ON civicrm_relationship.contact_id_b = c.id
$whereClause )
Expand Down
122 changes: 122 additions & 0 deletions tests/phpunit/CRM/Contact/BAO/QueryTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,128 @@ public function testNonNumericEqualsPostal() {

}

public function testNonReciprocalRelationshipTargetGroupIsCorrectResults() {
$contactID_a = $this->individualCreate();
$contactID_b = $this->individualCreate();
$this->callAPISuccess('Relationship', 'create', array(
'contact_id_a' => $contactID_a,
'contact_id_b' => $contactID_b,
'relationship_type_id' => 1,
'is_active' => 1,
));
// Create a group and add contact A to it.
$groupID = $this->groupCreate();
$this->callAPISuccess('GroupContact', 'create', array('group_id' => $groupID, 'contact_id' => $contactID_a, 'status' => 'Added'));

// Add another (sans-relationship) contact to the group,
$contactID_c = $this->individualCreate();
$this->callAPISuccess('GroupContact', 'create', array('group_id' => $groupID, 'contact_id' => $contactID_c, 'status' => 'Added'));

$params = array(
array(
0 => 'relation_type_id',
1 => 'IN',
2 =>
array(
0 => '1_b_a',
),
3 => 0,
4 => 0,
),
array(
0 => 'relation_target_group',
1 => 'IN',
2 =>
array(
0 => $groupID,
),
3 => 0,
4 => 0,
),
);

$query = new CRM_Contact_BAO_Query($params);
$dao = $query->searchQuery();
$this->assertEquals('1', $dao->N, "Search query returns exactly 1 result?");
$this->assertTrue($dao->fetch(), "Search query returns success?");
$this->assertEquals($contactID_b, $dao->contact_id, "Search query returns parent of contact A?");
}

public function testReciprocalRelationshipTargetGroupIsCorrectResults() {
$contactID_a = $this->individualCreate();
$contactID_b = $this->individualCreate();
$this->callAPISuccess('Relationship', 'create', array(
'contact_id_a' => $contactID_a,
'contact_id_b' => $contactID_b,
'relationship_type_id' => 2,
'is_active' => 1,
));
// Create a group and add contact A to it.
$groupID = $this->groupCreate();
$this->callAPISuccess('GroupContact', 'create', array('group_id' => $groupID, 'contact_id' => $contactID_a, 'status' => 'Added'));

// Add another (sans-relationship) contact to the group,
$contactID_c = $this->individualCreate();
$this->callAPISuccess('GroupContact', 'create', array('group_id' => $groupID, 'contact_id' => $contactID_c, 'status' => 'Added'));

$params = array(
array(
0 => 'relation_type_id',
1 => 'IN',
2 =>
array(
0 => '2_a_b',
),
3 => 0,
4 => 0,
),
array(
0 => 'relation_target_group',
1 => 'IN',
2 =>
array(
0 => $groupID,
),
3 => 0,
4 => 0,
),
);

$query = new CRM_Contact_BAO_Query($params);
$dao = $query->searchQuery();
$this->assertEquals('1', $dao->N, "Search query returns exactly 1 result?");
$this->assertTrue($dao->fetch(), "Search query returns success?");
$this->assertEquals($contactID_b, $dao->contact_id, "Search query returns spouse of contact A?");
}

public function testReciprocalRelationshipTargetGroupUsesTempTable() {
$groupID = $this->groupCreate();
$params = array(
array(
0 => 'relation_type_id',
1 => 'IN',
2 =>
array(
0 => '2_a_b',
),
3 => 0,
4 => 0,
),
array(
0 => 'relation_target_group',
1 => 'IN',
2 =>
array(
0 => $groupID,
),
3 => 0,
4 => 0,
),
);
$sql = CRM_Contact_BAO_Query::getQuery($params);
$this->assertContains('INNER JOIN civicrm_rel_temp_', $sql, "Query appears to use temporary table of compiled relationships?", TRUE);
}

/**
* Test Relationship Clause
*/
Expand Down

0 comments on commit 0e4e454

Please sign in to comment.