Skip to content

Commit

Permalink
Fix CRM-21811: Optimize advanced search by relationship with target g…
Browse files Browse the repository at this point in the history 
…roup for reciprocal relationship types.
  • Loading branch information
@twomice
twomice committed Mar 17, 2018
1 parent 4e99154 commit 14a01e7
Show file tree
Hide file tree
Showing 2 changed files with 96 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 @@ -3073,14 +3073,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 @@ -3121,7 +3125,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 @@ -4073,25 +4077,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 @@ -4114,12 +4121,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 @@ -4141,7 +4148,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 @@ -4201,9 +4208,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 @@ -4212,12 +4216,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
75 changes: 75 additions & 0 deletions tests/phpunit/CRM/Contact/BAO/QueryTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,81 @@ public function testRelationshipClause() {
$this->assertEquals($where2, $sql4[2]);
}

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 the group contact clause does not contain an OR.
*
Expand Down

0 comments on commit 14a01e7

Please sign in to comment.