From 35c7ed81a72e3af52af4c577a52e4d5bbfd9514f Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Sat, 10 Oct 2015 20:04:04 +0200 Subject: [PATCH 1/9] Fix grouping of host- and servicegroup queries It's a rather quick fix but fixes the problem. We should take another look whether a more sophisticated solution can be found. refs #10316 --- .../library/Monitoring/Backend/Ido/Query/IdoQuery.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php index 5c0cbba82..26e157d3f 100644 --- a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php +++ b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php @@ -1001,9 +1001,9 @@ abstract class IdoQuery extends DbQuery } $groupedTables[$table] = true; } - if ($this->getDatasource()->getDbType() !== 'pgsql') { + /*if ($this->getDatasource()->getDbType() !== 'pgsql') { return $group; - } + }*/ $columnIterator = new AppendIterator(); $columnIterator->append(new ColumnFilterIterator($this->columns)); $columnIterator->append(new ArrayIterator($this->orderColumns)); From 89d81262269a8f546e6f01ba660634724815be39 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 12 Nov 2015 09:26:11 +0100 Subject: [PATCH 2/9] Revert "Fix grouping of host- and servicegroup queries" This reverts commit 35c7ed81a72e3af52af4c577a52e4d5bbfd9514f. --- .../library/Monitoring/Backend/Ido/Query/IdoQuery.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php index e3cbbe9a4..6d373daaa 100644 --- a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php +++ b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php @@ -1023,9 +1023,9 @@ abstract class IdoQuery extends DbQuery } $groupedTables[$table] = true; } - /*if ($this->getDatasource()->getDbType() !== 'pgsql') { + if ($this->getDatasource()->getDbType() !== 'pgsql') { return $group; - }*/ + } $columnIterator = new AppendIterator(); $columnIterator->append(new ColumnFilterIterator($this->columns)); $columnIterator->append(new ArrayIterator($this->orderColumns)); From 140e288c0b8dccaf272500dcdf6c8452223b431e Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 12 Nov 2015 16:02:41 +0100 Subject: [PATCH 3/9] IdoQuery: Fix incorrect GROUP BY for MySQL SELECTs with joined columns refs #10316 --- .../Monitoring/Backend/Ido/Query/IdoQuery.php | 179 +++++++++--------- .../Backend/Ido/Query/ServicestatusQuery.php | 28 ++- 2 files changed, 104 insertions(+), 103 deletions(-) diff --git a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php index 6d373daaa..265543020 100644 --- a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php +++ b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php @@ -3,8 +3,6 @@ namespace Icinga\Module\Monitoring\Backend\Ido\Query; -use AppendIterator; -use ArrayIterator; use Zend_Db_Expr; use Icinga\Application\Icinga; use Icinga\Application\Logger; @@ -14,8 +12,8 @@ use Icinga\Data\Filter\FilterExpression; use Icinga\Exception\IcingaException; use Icinga\Exception\ProgrammingError; use Icinga\Exception\QueryException; -use Icinga\Module\Monitoring\Data\ColumnFilterIterator; use Icinga\Web\Session; +use Icinga\Module\Monitoring\Data\ColumnFilterIterator; /** * Base class for Ido Queries @@ -122,7 +120,7 @@ abstract class IdoQuery extends DbQuery protected $joinedVirtualTables = array(); /** - * Unresolved order columns + * List of column aliases used for sorting the result * * @var array */ @@ -445,7 +443,7 @@ abstract class IdoQuery extends DbQuery return $this; } - $this->orderColumns[$alias] = $alias; + $this->orderColumns[] = $alias; return parent::order($column, $dir); } @@ -993,16 +991,65 @@ abstract class IdoQuery extends DbQuery } /** - * Handle grouping for special columns, e.g. when the column sources more than one table + * Register the GROUP BY columns required for the given alias * - * @param string $column Column - * @param array $groupColumns GROUP BY list - * @param array $groupedTables Already grouped tables - * - * @return bool Whether the column was handled + * @param string $alias The alias to register columns for + * @param string $table The table the given alias is associated with + * @param array $groupedColumns The grouping columns registered so far + * @param array $groupedTables The tables for which columns were registered so far */ - protected function handleGroupColumn($column, &$groupColumns, &$groupedTables) { - return false; + protected function registerGroupColumns($alias, $table, array &$groupedColumns, array &$groupedTables) + { + switch ($table) { + case 'checktimeperiods': + $groupedColumns[] = 'ctp.timeperiod_id'; + break; + case 'contacts': + $groupedColumns[] = 'co.object_id'; + $groupedColumns[] = 'c.contact_id'; + break; + case 'hostobjects': + $groupedColumns[] = 'ho.object_id'; + break; + case 'hosts': + $groupedColumns[] = 'h.host_id'; + break; + case 'hostgroups': + $groupedColumns[] = 'hgo.object_id'; + $groupedColumns[] = 'hg.hostgroup_id'; + break; + case 'hoststatus': + $groupedColumns[] = 'hs.hoststatus_id'; + break; + case 'instances': + $groupedColumns[] = 'i.instance_id'; + break; + case 'servicegroups': + $groupedColumns[] = 'sgo.object_id'; + $groupedColumns[] = 'sg.servicegroup_id'; + break; + case 'serviceobjects': + $groupedColumns[] = 'so.object_id'; + break; + case 'serviceproblemsummary': + $groupedColumns[] = 'sps.unhandled_services_count'; + break; + case 'services': + $groupedColumns[] = 'so.object_id'; + $groupedColumns[] = 's.service_id'; + break; + case 'servicestatus': + $groupedColumns[] = 'ss.servicestatus_id'; + break; + case 'timeperiods': + $groupedColumns[] = 'ht.timeperiod_id'; + $groupedColumns[] = 'st.timeperiod_id'; + break; + default: + return; + } + + $groupedTables[$table] = true; } /** @@ -1014,82 +1061,38 @@ abstract class IdoQuery extends DbQuery if (! is_array($group)) { $group = array($group); } - foreach ($this->groupOrigin as $table) { - if ($this->hasJoinedVirtualTable($table)) { - $groupedTables = array(); - foreach ($this->groupBase as $table => $columns) { - foreach ($columns as $column) { - $group[] = $column; - } - $groupedTables[$table] = true; - } - if ($this->getDatasource()->getDbType() !== 'pgsql') { - return $group; - } - $columnIterator = new AppendIterator(); - $columnIterator->append(new ColumnFilterIterator($this->columns)); - $columnIterator->append(new ArrayIterator($this->orderColumns)); - foreach ($columnIterator as $alias => $column) { - $alias = $this->hasAliasName($alias) ? $alias : $this->customAliasToAlias($alias); - if ($this->handleGroupColumn($alias, $group, $groupedTables) === true) { - continue; - } - $tableName = $this->aliasToTableName($alias); - if (isset($groupedTables[$tableName])) { - continue; - } - switch ($tableName) { - case 'checktimeperiods': - $group[] = 'ctp.timeperiod_id'; - break; - case 'contacts': - $group[] = 'co.object_id'; - $group[] = 'c.contact_id'; - break; - case 'hostobjects': - $group[] = 'ho.object_id'; - break; - case 'hosts': - $group[] = 'h.host_id'; - break; - case 'hostgroups': - $group[] = 'hgo.object_id'; - $group[] = 'hg.hostgroup_id'; - break; - case 'hoststatus': - $group[] = 'hs.hoststatus_id'; - break; - case 'instances': - $group[] = 'i.instance_id'; - break; - case 'servicegroups': - $group[] = 'sgo.object_id'; - $group[] = 'sg.servicegroup_id'; - break; - case 'serviceobjects': - $group[] = 'so.object_id'; - break; - case 'serviceproblemsummary': - $group[] = 'sps.unhandled_services_count'; - break; - case 'services': - $group[] = 'so.object_id'; - $group[] = 's.service_id'; - break; - case 'servicestatus': - $group[] = 'ss.servicestatus_id'; - break; - case 'timeperiods': - $group[] = 'ht.timeperiod_id'; - $group[] = 'st.timeperiod_id'; - break; - default: - continue 2; - } - $groupedTables[$tableName] = true; - } - break; + $joinedOrigins = array_filter($this->groupOrigin, array($this, 'hasJoinedVirtualTable')); + if (empty($joinedOrigins)) { + return $group; + } + + $groupedTables = array(); + foreach ($this->groupBase as $baseTable => $aliasedPks) { + $groupedTables[$baseTable] = true; + foreach ($aliasedPks as $aliasedPk) { + $group[] = $aliasedPk; + } + } + + foreach (new ColumnFilterIterator($this->columns) as $desiredAlias => $desiredColumn) { + $alias = is_string($desiredAlias) ? $this->customAliasToAlias($desiredAlias) : $desiredColumn; + $table = $this->aliasToTableName($alias); + if ($table && !isset($groupedTables[$table]) && ( + in_array($table, $joinedOrigins, true) || $this->getDatasource()->getDbType() === 'pgsql') + ) { + $this->registerGroupColumns($alias, $table, $group, $groupedTables); + } + } + + if (! empty($group) && $this->getDatasource()->getDbType() === 'pgsql') { + foreach (new ColumnFilterIterator($this->orderColumns) as $alias) { + $table = $this->aliasToTableName($alias); + if ($table && !isset($groupedTables[$table]) + && !in_array($this->getMappedField($alias), $this->columns, true) + ) { + $this->registerGroupColumns($alias, $table, $group, $groupedTables); + } } } diff --git a/modules/monitoring/library/Monitoring/Backend/Ido/Query/ServicestatusQuery.php b/modules/monitoring/library/Monitoring/Backend/Ido/Query/ServicestatusQuery.php index df29d14f5..522f2a03c 100644 --- a/modules/monitoring/library/Monitoring/Backend/Ido/Query/ServicestatusQuery.php +++ b/modules/monitoring/library/Monitoring/Backend/Ido/Query/ServicestatusQuery.php @@ -413,22 +413,20 @@ class ServicestatusQuery extends IdoQuery /** * {@inheritdoc} */ - protected function handleGroupColumn($column, &$groupColumns, &$groupedTables) + protected function registerGroupColumns($alias, $table, array &$groupedColumns, array &$groupedTables) { - switch ($column) { - case 'service_handled': - case 'service_severity': - case 'service_unhandled': - if (! isset($groupedTables['hoststatus'])) { - $groupColumns[] = 'hs.hoststatus_id'; - $groupedTables['hoststatus'] = true; - } - if (! isset($groupedTables['servicestatus'])) { - $groupColumns[] = 'ss.servicestatus_id'; - $groupedTables['servicestatus'] = true; - } - return true; + if ($alias === 'service_handled' || $alias === 'service_severity' || $alias === 'service_unhandled') { + if (! isset($groupedTables['hoststatus'])) { + $groupedColumns[] = 'hs.hoststatus_id'; + $groupedTables['hoststatus'] = true; + } + + if (! isset($groupedTables['servicestatus'])) { + $groupedColumns[] = 'ss.servicestatus_id'; + $groupedTables['servicestatus'] = true; + } + } else { + parent::registerGroupColumns($alias, $table, $groupedColumns, $groupedTables); } - return false; } } From b182a31b90a1f073d2746c0fe6ad65edcc927049 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Thu, 12 Nov 2015 16:32:29 +0100 Subject: [PATCH 4/9] DbQuery: Catch exceptions in __toString() --- library/Icinga/Data/Db/DbQuery.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/library/Icinga/Data/Db/DbQuery.php b/library/Icinga/Data/Db/DbQuery.php index bb93fe127..39f8032de 100644 --- a/library/Icinga/Data/Db/DbQuery.php +++ b/library/Icinga/Data/Db/DbQuery.php @@ -3,7 +3,9 @@ namespace Icinga\Data\Db; +use Exception; use Zend_Db_Select; +use Icinga\Application\Logger; use Icinga\Data\Filter\FilterAnd; use Icinga\Data\Filter\FilterChain; use Icinga\Data\Filter\FilterNot; @@ -382,8 +384,13 @@ class DbQuery extends SimpleQuery */ public function __toString() { - $select = (string) $this->getSelectQuery(); - return $this->getIsSubQuery() ? ('(' . $select . ')') : $select; + try { + $select = (string) $this->getSelectQuery(); + return $this->getIsSubQuery() ? ('(' . $select . ')') : $select; + } catch (Exception $e) { + Logger::debug('Failed to render DbQuery. An error occured: %s', $e); + return ''; + } } /** From cc37ca37d90d3bda2d13313cba6b5249a00acd9f Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 13 Nov 2015 11:40:51 +0100 Subject: [PATCH 5/9] ListController: Fix servicegrid grouping when applying group restrictions Quickfix only. refs #10316 --- .../monitoring/application/controllers/ListController.php | 1 + .../library/Monitoring/Backend/Ido/Query/IdoQuery.php | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/modules/monitoring/application/controllers/ListController.php b/modules/monitoring/application/controllers/ListController.php index c0ae1133f..2e3ab27aa 100644 --- a/modules/monitoring/application/controllers/ListController.php +++ b/modules/monitoring/application/controllers/ListController.php @@ -545,6 +545,7 @@ class ListController extends Controller $this->applyRestriction('monitoring/filter/objects', $query); $this->filterQuery($query); $filter = (bool) $this->params->shift('problems', false) ? Filter::where('service_problem', 1) : null; + $query->getQuery()->setGroupBase(array()); $pivot = $query ->pivot( 'service_description', diff --git a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php index 265543020..48a1b499b 100644 --- a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php +++ b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php @@ -990,6 +990,12 @@ abstract class IdoQuery extends DbQuery return $this; } + public function setGroupBase(array $groupBase) + { + $this->groupBase = $groupBase; + return $this; + } + /** * Register the GROUP BY columns required for the given alias * From d0ab00143728bfed1dcb15b122884c8b89cd6860 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 13 Nov 2015 11:44:00 +0100 Subject: [PATCH 6/9] PivotTable: Fix axis queries if only one is being filtered refs #10316 --- library/Icinga/Data/PivotTable.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/Icinga/Data/PivotTable.php b/library/Icinga/Data/PivotTable.php index 8463b483e..906703174 100644 --- a/library/Icinga/Data/PivotTable.php +++ b/library/Icinga/Data/PivotTable.php @@ -345,13 +345,19 @@ class PivotTable implements Sortable ) { $xAxis = $this->queryXAxis()->fetchPairs(); $yAxis = $this->queryYAxis()->fetchPairs(); + $xAxisKeys = array_keys($xAxis); + $yAxisKeys = array_keys($yAxis); } else { if ($this->xAxisFilter !== null) { $xAxis = $this->queryXAxis()->fetchPairs(); - $yAxis = $this->queryYAxis()->where($this->xAxisColumn, $xAxis)->fetchPairs(); + $xAxisKeys = array_keys($xAxis); + $yAxis = $this->queryYAxis()->where($this->xAxisColumn, $xAxisKeys)->fetchPairs(); + $yAxisKeys = array_keys($yAxis); } else { // $this->yAxisFilter !== null $yAxis = $this->queryYAxis()->fetchPairs(); - $xAxis = $this->queryXAxis()->where($this->yAxisColumn, $yAxis)->fetchPairs(); + $yAxisKeys = array_keys($yAxis); + $xAxis = $this->queryXAxis()->where($this->yAxisColumn, $yAxisKeys)->fetchPairs(); + $xAxisKeys = array_keys($xAxis); } } $pivotData = array(); @@ -360,8 +366,6 @@ class PivotTable implements Sortable 'rows' => $yAxis ); if (! empty($xAxis) && ! empty($yAxis)) { - $xAxisKeys = array_keys($xAxis); - $yAxisKeys = array_keys($yAxis); $this->baseQuery ->where($this->xAxisColumn, $xAxisKeys) ->where($this->yAxisColumn, $yAxisKeys); From 005ec27ceadbf3fac20d0eb4efda46c8e1d96cdd Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 13 Nov 2015 12:13:02 +0100 Subject: [PATCH 7/9] IdoQuery: Fix method isTimestamp() not handling customvars properly refs #10316 --- .../Monitoring/Backend/Ido/Query/IdoQuery.php | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php index 48a1b499b..d7d279c3a 100644 --- a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php +++ b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php @@ -546,16 +546,17 @@ abstract class IdoQuery extends DbQuery /** * Return true if an field contains an explicit timestamp * - * @param String $field The field to test for containing an timestamp - * @return bool True when the field represents an timestamp + * @param string $field The field to test for containing an timestamp + * + * @return bool True when the field represents an timestamp */ public function isTimestamp($field) { - $mapped = $this->getMappedField($field); - if ($mapped === null) { - return stripos($field, 'UNIX_TIMESTAMP') !== false; + if ($this->isCustomVar($field)) { + return false; } - return stripos($mapped, 'UNIX_TIMESTAMP') !== false; + + return stripos($this->getMappedField($field) ?: $field, 'UNIX_TIMESTAMP') !== false; } /** From 5e37f7758ba3d183a2d05fbbd997a2fd2edc883d Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 13 Nov 2015 12:55:06 +0100 Subject: [PATCH 8/9] ServicecommenthistoryQuery: Add missing group origin "servicegroups" refs #10316 --- .../Monitoring/Backend/Ido/Query/ServicecommenthistoryQuery.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/monitoring/library/Monitoring/Backend/Ido/Query/ServicecommenthistoryQuery.php b/modules/monitoring/library/Monitoring/Backend/Ido/Query/ServicecommenthistoryQuery.php index 7c78c70ed..65a0ea1c2 100644 --- a/modules/monitoring/library/Monitoring/Backend/Ido/Query/ServicecommenthistoryQuery.php +++ b/modules/monitoring/library/Monitoring/Backend/Ido/Query/ServicecommenthistoryQuery.php @@ -21,7 +21,7 @@ class ServicecommenthistoryQuery extends IdoQuery /** * {@inheritdoc} */ - protected $groupOrigin = array('hostgroups', 'services'); + protected $groupOrigin = array('hostgroups', 'servicegroups', 'services'); /** * {@inheritdoc} From 26e6acf9af023ef14596bc9a5dbd61cea44d958b Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 13 Nov 2015 14:42:21 +0100 Subject: [PATCH 9/9] ListController: Fix servicegrid grouping when applying group restrictions #2 PostgreSQL had still issues with it. Quickfix only, again. refs #10316 --- library/Icinga/Data/PivotTable.php | 2 ++ .../monitoring/application/controllers/ListController.php | 1 - .../library/Monitoring/Backend/Ido/Query/IdoQuery.php | 5 +++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/library/Icinga/Data/PivotTable.php b/library/Icinga/Data/PivotTable.php index 906703174..8423c2dfa 100644 --- a/library/Icinga/Data/PivotTable.php +++ b/library/Icinga/Data/PivotTable.php @@ -226,6 +226,7 @@ class PivotTable implements Sortable { if ($this->xAxisQuery === null) { $this->xAxisQuery = clone $this->baseQuery; + $this->xAxisQuery->clearGroupingRules(); $xAxisHeader = $this->getXAxisHeader(); $columns = array($this->xAxisColumn, $xAxisHeader); $this->xAxisQuery->group(array_unique($columns)); // xAxisColumn and header may be the same column @@ -253,6 +254,7 @@ class PivotTable implements Sortable { if ($this->yAxisQuery === null) { $this->yAxisQuery = clone $this->baseQuery; + $this->yAxisQuery->clearGroupingRules(); $yAxisHeader = $this->getYAxisHeader(); $columns = array($this->yAxisColumn, $yAxisHeader); $this->yAxisQuery->group(array_unique($columns)); // yAxisColumn and header may be the same column diff --git a/modules/monitoring/application/controllers/ListController.php b/modules/monitoring/application/controllers/ListController.php index 2e3ab27aa..c0ae1133f 100644 --- a/modules/monitoring/application/controllers/ListController.php +++ b/modules/monitoring/application/controllers/ListController.php @@ -545,7 +545,6 @@ class ListController extends Controller $this->applyRestriction('monitoring/filter/objects', $query); $this->filterQuery($query); $filter = (bool) $this->params->shift('problems', false) ? Filter::where('service_problem', 1) : null; - $query->getQuery()->setGroupBase(array()); $pivot = $query ->pivot( 'service_description', diff --git a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php index d7d279c3a..5edc8f6f1 100644 --- a/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php +++ b/modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php @@ -991,9 +991,10 @@ abstract class IdoQuery extends DbQuery return $this; } - public function setGroupBase(array $groupBase) + public function clearGroupingRules() { - $this->groupBase = $groupBase; + $this->groupBase = array(); + $this->groupOrigin = array(); return $this; }