Fix problem with unprefixed order by columns in select queries

matomo-org · Oct 5, 2023 · cd4fb85 · cd4fb85
1 parent d4d8f81
commit cd4fb85
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 33 deletions.
diff --git a/core/DataAccess/LogAggregator.php b/core/DataAccess/LogAggregator.php
@@ -659,45 +659,74 @@ protected function getDimensionsToSelect($dimensions, $additionalSelects)
     }
 
     /**
-     * Returns the dimensions array, where
-     * (1) the table name is prepended to the field
-     * (2) the "AS `label` " is appended to the field
+     * Returns an array of select expressions based on the provided dimensions array
+     * Each dimension will be prefixed with the table name, if it's not an expression and will be alias
+     * with the dimension name or an custom alias if one was provided as array key.
      *
-     * @param $dimensions
-     * @param $tableName
-     * @param bool $appendSelectAs
-     * @return mixed
+     * @param array $dimensions An array of dimensions, where an alias can be provided as key
+     * @param string $tableName
+     * @return array
      */
-    protected function getSelectDimensions($dimensions, $tableName, $appendSelectAs = true)
+    protected function getSelectDimensions(array $dimensions, string $tableName): array
     {
-        foreach ($dimensions as $selectAs => &$field) {
-            $selectAsString = $field;
+        $selectDimensions = [];
 
-            if (!is_numeric($selectAs)) {
-                $selectAsString = $selectAs;
-            } else if ($this->isFieldFunctionOrComplexExpression($field)) {
-                // if complex expression has a select as, use it
-                if (!$appendSelectAs && preg_match('/\s+AS\s+(.*?)\s*$/', $field, $matches)) {
-                    $field = $matches[1];
-                    continue;
-                }
+        foreach ($dimensions as $selectAs => $field) {
 
-                // if function w/o select as, do not alias or prefix
-                $selectAsString = $appendSelectAs = false;
+            if ($this->isFieldFunctionOrComplexExpression($field) && is_numeric($selectAs)) {
+                // an expression or field function without an alias should be used as is
+                $selectDimensions[] = $field;
+                continue;
             }
 
-            $isKnownField = !in_array($field, array('referrer_data'));
+            $selectAlias = !is_numeric($selectAs) ? $selectAs : $field;
 
-            if ($selectAsString == $field && $isKnownField) {
+            if (!$this->isFieldFunctionOrComplexExpression($field)) {
+                // prefix field name with table if it's not an expression
                 $field = $this->prefixColumn($field, $tableName);
             }
 
-            if ($appendSelectAs && $selectAsString) {
-                $field = $this->prefixColumn($field, $tableName) . $this->getSelectAliasAs($selectAsString);
+            // append " AS alias"
+            $field .= $this->getSelectAliasAs($selectAlias);
+            $selectDimensions[] = $field;
+        }
+
+        return $selectDimensions;
+    }
+
+    /**
+     * Returns an array of fields to be used in an grouped by statement.
+     * For that either the alias, the field expression or prefixed column name of the provided dimensions will be used.
+     *
+     * @param array $dimensions An array of dimensions, where an alias can be provided as key
+     * @param string $tableName
+     * @return array
+     */
+    protected function getGroupByDimensions(array $dimensions, string $tableName): array
+    {
+        $orderByDimensions = [];
+
+        foreach ($dimensions as $selectAs => $field) {
+            if (!is_numeric($selectAs)) {
+                $orderByDimensions[] = $selectAs;
+                continue;
+            }
+
+            if ($this->isFieldFunctionOrComplexExpression($field)) {
+                // if complex expression has a select as, use it
+                if (preg_match('/\s+AS\s+(.*?)\s*$/', $field, $matches)) {
+                    $orderByDimensions[] = $matches[1];
+                    continue;
+                }
+
+                $orderByDimensions[] = $field;
+                continue;
             }
+
+            $orderByDimensions[] = $this->prefixColumn($field, $tableName);
         }
 
-        return $dimensions;
+        return $orderByDimensions;
     }
 
     /**
@@ -755,7 +784,7 @@ public function getWhereStatement($tableName, $datetimeField, $extraWhere = fals
 
     protected function getGroupByStatement($dimensions, $tableName)
     {
-        $dimensions = $this->getSelectDimensions($dimensions, $tableName, $appendSelectAs = false);
+        $dimensions = $this->getGroupByDimensions($dimensions, $tableName);
         $groupBy    = implode(", ", $dimensions);
 
         return $groupBy;

diff --git a/plugins/Transitions/API.php b/plugins/Transitions/API.php
@@ -351,7 +351,7 @@ protected function queryFollowingActions($idaction, $actionType, LogAggregator $
     protected function queryExternalReferrers($idaction, $actionType, $logAggregator, $limitBeforeGrouping = 0)
     {
 
-        $rankingQuery = new RankingQuery($limitBeforeGrouping ? $limitBeforeGrouping: $this->limitBeforeGrouping);
+        $rankingQuery = new RankingQuery($limitBeforeGrouping ?: $this->limitBeforeGrouping);
         $rankingQuery->setOthersLabel('Others');
 
         // we generate a single column that contains the interesting data for each referrer.
@@ -360,16 +360,14 @@ protected function queryExternalReferrers($idaction, $actionType, $logAggregator
         // group by. when we group by both, we don't get a single column for the keyword but instead
         // one column per keyword + search engine url. this way, we could not get the top keywords using
         // the ranking query.
-        $dimensions = array('referrer_data', 'referer_type');
-        $rankingQuery->addLabelColumn('referrer_data');
-        $selects = array(
-            'CASE log_visit.referer_type
+        $dimensions = array('referrer_data' => 'CASE log_visit.referer_type
 				WHEN ' . Common::REFERRER_TYPE_DIRECT_ENTRY . ' THEN \'\'
 				WHEN ' . Common::REFERRER_TYPE_SEARCH_ENGINE . ' THEN log_visit.referer_keyword
 				WHEN ' . Common::REFERRER_TYPE_SOCIAL_NETWORK . ' THEN log_visit.referer_name
 				WHEN ' . Common::REFERRER_TYPE_WEBSITE . ' THEN log_visit.referer_url
 				WHEN ' . Common::REFERRER_TYPE_CAMPAIGN . ' THEN CONCAT(log_visit.referer_name, \' \', log_visit.referer_keyword)
-			END AS `referrer_data`');
+			END', 'referer_type');
+        $rankingQuery->addLabelColumn('referrer_data');
 
         // get one limited group per referrer type
         $rankingQuery->partitionResultIntoMultipleGroups('referer_type', array(
@@ -384,7 +382,7 @@ protected function queryExternalReferrers($idaction, $actionType, $logAggregator
         $where = 'visit_entry_idaction_' . $type . ' = ' . intval($idaction);
 
         $metrics = array(Metrics::INDEX_NB_VISITS);
-        $data = $logAggregator->queryVisitsByDimension($dimensions, $where, $selects, $metrics, $rankingQuery, false, Config::getInstance()->General['live_query_max_execution_time']);
+        $data = $logAggregator->queryVisitsByDimension($dimensions, $where, [], $metrics, $rankingQuery, false, Config::getInstance()->General['live_query_max_execution_time']);
 
         $referrerData = array();
         $referrerSubData = array();