From 21c643d68b883bb946351235b8152f43843e4ad6 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Mon, 17 Jan 2022 16:37:59 +0100 Subject: [statistics] Fix join ordering; renames and comments --- modules-available/statistics/api.inc.php | 6 +++--- .../statistics/hooks/config-tgz.inc.php | 2 +- .../statistics/inc/devicetype.inc.php | 7 ------- .../statistics/inc/hardwareinfo.inc.php | 1 + .../statistics/inc/hardwarequery.inc.php | 3 ++- .../statistics/inc/hardwarequerycolumn.inc.php | 23 +++++++++++++++++++--- modules-available/statistics/pages/machine.inc.php | 2 +- .../statistics/pages/projectors.inc.php | 2 +- 8 files changed, 29 insertions(+), 17 deletions(-) delete mode 100644 modules-available/statistics/inc/devicetype.inc.php diff --git a/modules-available/statistics/api.inc.php b/modules-available/statistics/api.inc.php index fc0b2afe..7aa6de34 100644 --- a/modules-available/statistics/api.inc.php +++ b/modules-available/statistics/api.inc.php @@ -338,7 +338,7 @@ if ($type[0] === '~') { $hwid = $hwids[$screen['name']]; } else { $hwid = (int)Database::insertIgnore('statistic_hw', 'hwid', - array('hwtype' => DeviceType::SCREEN, 'hwname' => $screen['name'])); + array('hwtype' => HardwareInfo::SCREEN, 'hwname' => $screen['name'])); $hwids[$screen['name']] = $hwid; } // Now add new entries @@ -384,7 +384,7 @@ if ($type[0] === '~') { Database::exec("UPDATE machine_x_hw x, statistic_hw h SET x.disconnecttime = UNIX_TIMESTAMP() WHERE x.machineuuid = :uuid AND x.hwid = h.hwid AND h.hwtype = :type AND x.disconnecttime = 0", - array('uuid' => $uuid, 'type' => DeviceType::SCREEN)); + array('uuid' => $uuid, 'type' => HardwareInfo::SCREEN)); } else { // Some screens connected, make sure old entries get removed Database::exec("UPDATE machine_x_hw x, statistic_hw h @@ -393,7 +393,7 @@ if ($type[0] === '~') { AND x.machineuuid = :uuid", array( 'pairs' => $keepPair, 'uuid' => $uuid, - 'type' => DeviceType::SCREEN, + 'type' => HardwareInfo::SCREEN, )); } } diff --git a/modules-available/statistics/hooks/config-tgz.inc.php b/modules-available/statistics/hooks/config-tgz.inc.php index 0a43e115..b732fd7a 100644 --- a/modules-available/statistics/hooks/config-tgz.inc.php +++ b/modules-available/statistics/hooks/config-tgz.inc.php @@ -4,7 +4,7 @@ $res = Database::simpleQuery('SELECT h.hwname FROM statistic_hw h' . " INNER JOIN statistic_hw_prop p ON (h.hwid = p.hwid AND p.prop = :projector)" . " WHERE h.hwtype = :screen ORDER BY h.hwname ASC", array( 'projector' => 'projector', - 'screen' => DeviceType::SCREEN, + 'screen' => HardwareInfo::SCREEN, )); if ($res !== false) { // CHeck this in case we're running on old DB during update diff --git a/modules-available/statistics/inc/devicetype.inc.php b/modules-available/statistics/inc/devicetype.inc.php deleted file mode 100644 index a01ec310..00000000 --- a/modules-available/statistics/inc/devicetype.inc.php +++ /dev/null @@ -1,7 +0,0 @@ -columns as $column) { if ($column instanceof HardwareQueryColumn) { - $column->generate($this->joins, $columns, $this->args); + $column->generate($this->joins, $columns, $this->args, $groupConcat); } else { $columns[] = $column; } diff --git a/modules-available/statistics/inc/hardwarequerycolumn.inc.php b/modules-available/statistics/inc/hardwarequerycolumn.inc.php index 71793008..d073530b 100644 --- a/modules-available/statistics/inc/hardwarequerycolumn.inc.php +++ b/modules-available/statistics/inc/hardwarequerycolumn.inc.php @@ -11,6 +11,7 @@ class HardwareQueryColumn private $alias; private $conditions = []; private $params = []; + private $classId; private static function getId(): string { @@ -19,13 +20,19 @@ class HardwareQueryColumn public function __construct(bool $global, string $column, string $alias = null) { + $this->classId = ++self::$id; $this->global = $global; $this->tableAlias = self::getId(); $this->virtualColumnName = $column; $this->alias = $alias ?? $column; } - public function generate(array &$joins, array &$columns, array &$params) + /** + * Add necessary conditions, joins, columns to final SQL arrays. To be called + * from HardwareQuery::buildQuery(). + * @param bool $groupConcat whether to add distinct GROUP_CONCAT to column. + */ + public function generate(array &$joins, array &$columns, array &$params, bool $groupConcat) { if ($this->global) { $srcTable = 'shw'; @@ -46,7 +53,11 @@ class HardwareQueryColumn // INNER JOIN, so the result will be empty if the condition doesn't match. $type = count($this->conditions) === 1 ? 'LEFT' : 'INNER'; $joins[] = "$type JOIN $table $tid ON (" . implode(' AND ', $this->conditions) . ")"; - $columns[] = "$tid.`value` AS `{$this->alias}`"; + if ($groupConcat) { + $columns[] = "Group_Concat(DISTINCT $tid.`value` SEPARATOR ', ') AS `{$this->alias}`"; + } else { + $columns[] = "$tid.`value` AS `{$this->alias}`"; + } $params += $this->params; } @@ -60,7 +71,13 @@ class HardwareQueryColumn { $valueCol = ($op === '<' || $op === '>' || $op === '<=' || $op === '>=') ? 'numeric' : 'value'; if ($other instanceof HardwareQueryColumn) { - $this->conditions[] = "{$this->tableAlias}.`$valueCol` $op {$other->tableAlias}.`$valueCol`"; + $cond = "{$this->tableAlias}.`$valueCol` $op {$other->tableAlias}.`$valueCol`"; + // Don't reference a column of a table that hasn't been joined yet + if ($this->classId > $other->classId) { + $this->conditions[] = $cond; + } else { + $other->conditions[] = $cond; + } } else { $pid = self::getId(); $this->conditions[] = "{$this->tableAlias}.`$valueCol` $op (:$pid)"; diff --git a/modules-available/statistics/pages/machine.inc.php b/modules-available/statistics/pages/machine.inc.php index 019b0d8d..6ea0ce28 100644 --- a/modules-available/statistics/pages/machine.inc.php +++ b/modules-available/statistics/pages/machine.inc.php @@ -229,7 +229,7 @@ class SubPage . " LEFT JOIN machine_x_hw_prop p ON (m.machinehwid = p.machinehwid AND p.prop = 'resolution')" . " LEFT JOIN statistic_hw_prop q ON (m.hwid = q.hwid AND q.prop = 'projector')" . " WHERE m.machineuuid = :uuid", - array('screen' => DeviceType::SCREEN, 'uuid' => $uuid)); + array('screen' => HardwareInfo::SCREEN, 'uuid' => $uuid)); $client['screens'] = array(); $ports = array(); foreach ($res as $row) { diff --git a/modules-available/statistics/pages/projectors.inc.php b/modules-available/statistics/pages/projectors.inc.php index 97a21ebd..91abfe9c 100644 --- a/modules-available/statistics/pages/projectors.inc.php +++ b/modules-available/statistics/pages/projectors.inc.php @@ -49,7 +49,7 @@ class SubPage . " INNER JOIN statistic_hw_prop p ON (h.hwid = p.hwid AND p.prop = :projector)" . " WHERE h.hwtype = :screen ORDER BY h.hwname ASC", array( 'projector' => 'projector', - 'screen' => DeviceType::SCREEN, + 'screen' => HardwareInfo::SCREEN, )); $data = array( 'projectors' => $res->fetchAll() -- cgit v1.2.3-55-g7522