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

CRM-21362: Mailing summary report group by MySQL 5.7 error #11206

Merged
merged 2 commits into from
Oct 29, 2017
Merged

CRM-21362: Mailing summary report group by MySQL 5.7 error #11206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b07c960
CRM-21362: Mailing summary report group by MySQL 5.7 error
monishdeb Oct 27, 2017
b281855
additional fixes
monishdeb Oct 27, 2017
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: 22 additions & 10 deletions CRM/Contact/BAO/Query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4678,16 +4678,7 @@ public static function filterCountryFromValuesIfStateExists(&$formValues) {
* @return string
*/
public static function appendAnyValueToSelect($selectClauses, $groupBy) {
$mysqlVersion = CRM_Core_DAO::singleValueQuery('SELECT VERSION()');
$sqlMode = explode(',', CRM_Core_DAO::singleValueQuery('SELECT @@sql_mode'));

// Disable only_full_group_by mode for lower sql versions.
if (version_compare($mysqlVersion, '5.7', '<') || (!empty($sqlMode) && !in_array('ONLY_FULL_GROUP_BY', $sqlMode))) {
$key = array_search('ONLY_FULL_GROUP_BY', $sqlMode);
unset($sqlMode[$key]);
CRM_Core_DAO::executeQuery("SET SESSION sql_mode = '" . implode(',', $sqlMode) . "'");
}
else {
if (!CRM_Utils_SQL::disableFullGroupByMode()) {
$groupBy = array_map('trim', (array) $groupBy);
$aggregateFunctions = '/(ROUND|AVG|COUNT|GROUP_CONCAT|SUM|MAX|MIN)\(/i';
foreach ($selectClauses as $key => &$val) {
Expand All @@ -4702,6 +4693,27 @@ public static function appendAnyValueToSelect($selectClauses, $groupBy) {
return "SELECT " . implode(', ', $selectClauses) . " ";
}

/**
* For some special cases, where if non-aggregate ORDER BY columns are not present in GROUP BY
* 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 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
// 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}";
}
}
}
}

/**
* Include Select columns in groupBy clause.
*
Expand Down
11 changes: 7 additions & 4 deletions CRM/Report/Form/Mailing/Summary.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,13 @@ public function __construct() {
'order_bys' => array(
'start_date' => array(
'title' => ts('Start Date'),
'dbAlias' => 'MIN(mailing_job_civireport.start_date)',
),
'end_date' => array(
'title' => ts('End Date'),
'default_weight' => '1',
'default_order' => 'DESC',
'dbAlias' => 'MAX(mailing_job_civireport.end_date)',
),
),
'grouping' => 'mailing-fields',
Expand Down Expand Up @@ -500,10 +502,6 @@ public function where() {
else {
$this->_where = "WHERE " . implode(' AND ', $clauses);
}

// if ( $this->_aclWhere ) {
// $this->_where .= " AND {$this->_aclWhere} ";
// }
}

public function groupBy() {
Expand All @@ -513,6 +511,11 @@ public function groupBy() {
$this->_groupBy = CRM_Contact_BAO_Query::getGroupByFromSelectColumns($this->_selectClauses, $groupBy);
}

public function orderBy() {
parent::orderBy();
CRM_Contact_BAO_Query::getGroupByFromOrderBy($this->_groupBy, $this->_orderByArray);
}

public function postProcess() {

$this->beginPostProcess();
Expand Down
20 changes: 20 additions & 0 deletions CRM/Utils/SQL.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,24 @@ public static function supportsFullGroupBy() {
return version_compare(CRM_Core_DAO::singleValueQuery('SELECT VERSION()'), '5.7', '>=');
}

/**
* Disable ONLY_FULL_GROUP_BY for MySQL versions lower then 5.7
*
* @return bool
*/
public static function disableFullGroupByMode() {
$sqlModes = self::getSqlModes();

// Disable only_full_group_by mode for lower sql versions.
if (!self::supportsFullGroupBy() || (!empty($sqlModes) && !in_array('ONLY_FULL_GROUP_BY', $sqlModes))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@monishdeb should the 2nd half of that 2nd test be && in_array not && !in_array as we want it if ONLY_FULL_GROUP_BY is in the array right?

Copy link
Member Author

Choose a reason for hiding this comment

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

@seamuslee001 no because the second part (!empty($sqlModes) && !in_array('ONLY_FULL_GROUP_BY', $sqlModes)) is for Mysql 5.7 where if ONLY_FULL_GROUP_BY is intentionally not present (i.e. !in_array) in sql modes then return TRUE which means GroupByMode is disabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

so you expect then in would just jump to the return in L93 because the array_search in 89 won't find it then right?

Copy link
Member Author

@monishdeb monishdeb Oct 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

yes .. exactly. Also this logic is already written and tested earlier. See above in Contact/BAO/Query.php from where this condition was moved

Copy link
Contributor

Choose a reason for hiding this comment

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

ok no worries makes sense then

if ($key = array_search('ONLY_FULL_GROUP_BY', $sqlModes)) {
unset($sqlModes[$key]);
CRM_Core_DAO::executeQuery("SET SESSION sql_mode = '" . implode(',', $sqlModes) . "'");
}
return TRUE;
}

return FALSE;
}

}