Remove no longer needed double rollup result iteration

Author: mneudert

plugins/CustomDimensions/RecordBuilders/CustomDimension.php

@@ -186,100 +186,65 @@ protected function aggregateFromActions(DataTable $report, LogAggregator $logAgg
         $metricIds[] = Metrics::INDEX_BOUNCE_COUNT;
         $metricIds[] = Metrics::INDEX_PAGE_EXIT_NB_VISITS;
 
-        $actionRows = [];
-
         while ($row = $resultSet->fetch()) {
             if (!isset($row[Metrics::INDEX_NB_VISITS])) {
                 return;
             }
 
             $label = $row[$valueField];
 
-            if ($withRollup) {
-                $url = $row['url'];
-
-                if (is_null($label)) {
-                    continue;
-                }
-
-                if (!is_null($url)) {
-                    $actionRows[] = $row;
-                    continue;
-                }
+            if ($withRollup && $label === null) {
+                // top-level rollup result
+                continue;
             }
 
             $columns = [];
+
             foreach ($metricIds as $id) {
                 $columns[$id] = (float) ($row[$id] ?? 0);
             }
+
             $label = $this->cleanCustomDimensionValue($label);
-            $tableRow = $report->sumRowWithLabel($label, $columns);
+            $url = $row['url'];
 
             if (!$withRollup) {
-                $url = $row['url'];
-                if (empty($url)) {
-                    continue;
-                }
-
-                // make sure we always work with normalized URL no matter how the individual action stores it
-                $normalized = Tracker\PageUrl::normalizeUrl($url);
-                $url = $normalized['url'];
-
-                if (empty($url)) {
-                    continue;
-                }
-
-                $tableRow->sumRowWithLabelToSubtable($url, $columns);
-            }
-        }
-
-        if ($withRollup) {
-            $previousLabel = null;
-
-            foreach ($actionRows as $row) {
-                if (!isset($row[Metrics::INDEX_NB_VISITS])) {
-                    return;
-                }
-
-                $label = $row[$valueField];
-                $url = $row['url'];
-
-                if (is_null($label) || is_null($url)) {
+                $tableRow = $report->sumRowWithLabel($label, $columns);
+            } else {
+                if ($url === null) {
+                    // second-level rollup result
+                    $report->sumRowWithLabel($label, $columns);
                     continue;
                 }
 
-                $label = $this->cleanCustomDimensionValue($label);
                 $tableRow = $report->getRowFromLabel($label);
 
-                if (empty($tableRow)) {
-                    continue;
-                }
-
-                // make sure we always work with normalized URL no matter how the individual action stores it
-                $normalized = Tracker\PageUrl::normalizeUrl($url);
-                $url = $normalized['url'];
-
-                if (empty($url)) {
-                    continue;
-                }
-
-                // skip subtable creation if only one "Others" row would appear
-                if (
-                    $label !== $previousLabel
-                    && $url === RankingQuery::LABEL_SUMMARY_ROW
-                ) {
+                if (false === $tableRow) {
+                    // non-rollup row but rollup row is missing
+                    // should not happen, but don't break
                     continue;
                 }
+            }
 
-                $previousLabel = $label;
-                $columns = [];
+            // make sure we always work with normalized URL no matter how the individual action stores it
+            $normalized = Tracker\PageUrl::normalizeUrl($url);
+            $url = $normalized['url'];
 
-                foreach ($metricIds as $id) {
-                    $columns[$id] = (float) ($row[$id] ?? 0);
-                }
+            if (empty($url)) {
+                continue;
+            }
 
-                $tableRow->sumRowWithLabelToSubtable($url, $columns);
+            if (
+                $withRollup
+                && $url === RankingQuery::LABEL_SUMMARY_ROW
+                && !$tableRow->isSubtableLoaded()
+            ) {
+                // skip creating the subtable if:
+                // - we are using rollups
+                // - the only row would be "Others"
+                continue;
             }
+
+            $tableRow->sumRowWithLabelToSubtable($url, $columns);
         }
     }
 

plugins/Referrers/RecordBuilders/AIReferrers.php

@@ -88,40 +88,30 @@ protected function aggregateFromVisits(LogAggregator $logAggregator, DataTable $
     {
         $resultSet = $this->queryAIReferrerEntryPages($logAggregator, $field);
 
-        $actionRows = [];
-
         while ($row = $resultSet->fetch()) {
             if (!isset($row[Metrics::INDEX_NB_VISITS])) {
                 return;
             }
 
             $label = $row['referer_name'];
-
             $pageUrlOrTitle = $row['action_name'];
 
-            if (is_null($label)) {
+            if ($label === null) {
+                // top-level rollup result
                 continue;
             }
 
-            if (!is_null($pageUrlOrTitle)) {
-                $actionRows[] = $row;
+            if ($pageUrlOrTitle === null) {
+                // second-level rollup result
+                $report->sumRowWithLabel($label, $this->makeVisitRow($row));
                 continue;
             }
 
-            $report->sumRowWithLabel($label, $this->makeVisitRow($row));
-        }
-
-        foreach ($actionRows as $row) {
-            if (!isset($row[Metrics::INDEX_NB_VISITS])) {
-                return;
-            }
-
-            $label          = $row['referer_name'];
-            $pageUrlOrTitle = $row['action_name'];
-
             $tableRow = $report->getRowFromLabel($label);
 
-            if (empty($tableRow)) {
+            if (false === $tableRow) {
+                // non-rollup row but rollup row is missing
+                // should not happen, but don't break
                 continue;
             }
 
@@ -131,6 +121,16 @@ protected function aggregateFromVisits(LogAggregator $logAggregator, DataTable $
                 $pageUrlOrTitle = $normalized['url'];
             }
 
+            if (
+                $pageUrlOrTitle === RankingQuery::LABEL_SUMMARY_ROW
+                && !$tableRow->isSubtableLoaded()
+            ) {
+                // skip creating the subtable if:
+                // - we are using rollups
+                // - the only row would be "Others"
+                continue;
+            }
+
             $tableRow->sumRowWithLabelToSubtable($pageUrlOrTitle, $this->makeVisitRow($row));
         }
     }