Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(dev/core/68) Fix DB Error on 'Find Participant' page when MySQL FULL_GROUP_BY_MODE is enabled #11996

Merged
merged 2 commits into from
May 10, 2018
Merged

(dev/core/68) Fix DB Error on 'Find Participant' page when MySQL FULL_GROUP_BY_MODE is enabled #11996

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4fa404b
(dev/core/68) Fix DB Error on 'Find Participant' page when MySQL FULL…
monishdeb Apr 18, 2018
e516755
revert some change and other fixes
monishdeb May 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions CRM/Contact/BAO/Query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4734,14 +4734,14 @@ public static function appendAnyValueToSelect($selectClauses, $groupBy, $aggrega
* on full_group_by mode, then append the those missing columns to GROUP BY clause
* keyword to select fields not present in groupBy
*
* @param string $groupBy - GROUP BY clause where missing ORDER BY columns will be appended
* @param string $groupBy - GROUP BY clause where missing ORDER BY columns will be appended if not present
* @param array $orderBys - ORDER BY sub-clauses
*
*/
public static function getGroupByFromOrderBy(&$groupBy, $orderBys) {
if (!CRM_Utils_SQL::disableFullGroupByMode()) {
foreach ($orderBys as $orderBy) {
$orderBy = str_replace(array(' DESC', ' ASC', '`'), '', $orderBy); // remove sort syntax from ORDER BY clauses if present
$orderBy = str_ireplace(array(' DESC', ' ASC', '`'), '', $orderBy); // remove sort syntax from ORDER BY clauses if present
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton so this change is about making it so that its not case sensitive iirc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so - this seems totally safe IMHO - if it's OK for non sensitive then case sensitive is ok

// if ORDER BY column is not present in GROUP BY then append it to end
if (preg_match('/(MAX|MIN)\(/i', trim($orderBy)) !== 1 && !strstr($groupBy, $orderBy)) {
$groupBy .= ", {$orderBy}";
Expand Down Expand Up @@ -4841,13 +4841,6 @@ public function searchQuery(
$additionalFromClause = NULL, $skipOrderAndLimit = FALSE
) {

// In this case we are expecting the search query to return all the first single letter characters of contacts ONLY,
// but when FGB (FULL_GROUP_BY_MODE) is enabled MySQL expect the columns present in GROUP BY must be present in SELECT clause
// and that results into error and needless to have other columns. In order to resolve this disable FGB to fulfill both the cases
if ($sortByChar) {
CRM_Core_DAO::disableFullGroupByMode();
}

if ($includeContactIds) {
$this->_includeContactIds = TRUE;
$this->_whereClause = $this->whereClause();
Expand Down Expand Up @@ -4895,6 +4888,16 @@ public function searchQuery(
$limit = " LIMIT $offset, $rowCount ";
}
}
// Two cases where we are disabling FGB (FULL_GROUP_BY_MODE):
// 1. Expecting the search query to return all the first single letter characters of contacts ONLY, but when FGB is enabled
// MySQL expect the columns present in GROUP BY, must be present in SELECT clause and that results into error, needless to have other columns.
// 2. When GROUP BY columns are present then disable FGB otherwise it demands to add ORDER BY columns in GROUP BY and eventually in SELECT
// clause. This will impact the search query output.
$disableFullGroupByMode = ($sortByChar || !empty($groupByCols));

if ($disableFullGroupByMode) {
CRM_Core_DAO::disableFullGroupByMode();
}

// CRM-15231
$this->_sort = $sort;
Expand All @@ -4905,16 +4908,7 @@ public function searchQuery(
list($select, $from, $where, $having) = $this->query($count, $sortByChar, $groupContacts, $onlyDeleted);

if (!empty($groupByCols)) {
// It doesn't matter to include columns in SELECT clause, which are present in GROUP BY when we just want the contact IDs
if (!$groupContacts && !$sortByChar) {
$select = self::appendAnyValueToSelect($this->_select, $groupByCols, 'GROUP_CONCAT');
}
$groupBy = " GROUP BY " . implode(', ', $groupByCols);
if (!empty($order)) {
// retrieve order by columns from ORDER BY clause
$orderBys = explode(",", str_replace('ORDER BY ', '', $order));
self::getGroupByFromOrderBy($groupBy, $orderBys);
}
}

if ($additionalWhereClause) {
Expand Down Expand Up @@ -4948,7 +4942,7 @@ public function searchQuery(

$dao = CRM_Core_DAO::executeQuery($query);

if ($sortByChar) {
if ($disableFullGroupByMode) {
CRM_Core_DAO::reenableFullGroupByMode();
}

Expand Down