Refs #3906

* Speeding up the suggestion list by only selecting visitors that have a value set for the requested segment
matomo-org · Apr 21, 2013 · 3069828 · 3069828
1 parent ea240af
commit 3069828
Show file tree

Hide file tree

Showing 10 changed files with 38 additions and 38 deletions.
diff --git a/core/Segment.php b/core/Segment.php
@@ -91,13 +91,10 @@ protected function getCleanedExpression($expression)
                 throw new Exception("You do not have enough permission to access the segment " . $name);
             }
 
-//            $this->segmentsHumanReadable[] = $segment['name'] . " " .
-//                                            $this->getNameForMatchType($matchType) .
-//                                            $value;
-
             // apply presentation filter
             if (isset($segment['sqlFilter'])
                 && !empty($segment['sqlFilter'])
+                && $matchType != Piwik_SegmentExpression::MATCH_IS_NOT_NULL
             ) {
                 $value = call_user_func($segment['sqlFilter'], $value, $segment['sqlSegment'], $matchType, $name);
 
@@ -184,7 +181,6 @@ public function getSelectQuery($select, $from, $where = false, $bind = array(),
         } else {
             $sql = $this->buildSelectQuery($select, $from, $where, $orderBy, $groupBy);
         }
-
         return array(
             'sql'  => $sql,
             'bind' => $bind

diff --git a/core/SegmentExpression.php b/core/SegmentExpression.php
@@ -27,6 +27,9 @@ class Piwik_SegmentExpression
     const MATCH_CONTAINS = '=@';
     const MATCH_DOES_NOT_CONTAIN = '!@';
 
+    // Note: undocumented for now, only used in API.getSuggestedValuesForSegment
+    const MATCH_IS_NOT_NULL = '::';
+
     // Special case, since we look up Page URLs/Page titles in a sub SQL query
     const MATCH_ACTIONS_CONTAINS = 'IN';
 
@@ -66,6 +69,7 @@ public function parseSubExpressions()
                 . self::MATCH_LESS_OR_EQUAL . '|'
                 . self::MATCH_LESS . '|'
                 . self::MATCH_CONTAINS . '|'
+                . self::MATCH_IS_NOT_NULL . '|'
                 . self::MATCH_DOES_NOT_CONTAIN
                 . '){1}(.+)/';
             $match = preg_match($pattern, $operand, $matches);
@@ -180,6 +184,11 @@ protected function getSqlMatchFromDefinition($def, &$availableTables)
                 $value = '%' . $this->escapeLikeString($value) . '%';
                 break;
 
+            case self::MATCH_IS_NOT_NULL:
+                $sqlMatch = 'IS NOT NULL AND ('.$field.' <> \'\' OR '.$field.' = 0)';
+                $value = null;
+                break;
+
             case self::MATCH_ACTIONS_CONTAINS:
                 // this match type is not accessible from the outside
                 // (it won't be matched in self::parseSubExpressions())
@@ -193,7 +202,8 @@ protected function getSqlMatchFromDefinition($def, &$availableTables)
                 break;
         }
 
-        if ($matchType === self::MATCH_ACTIONS_CONTAINS) {
+        if ($matchType === self::MATCH_ACTIONS_CONTAINS
+            || is_null($value)) {
             $sqlExpression = "$field $sqlMatch";
         } else {
             $sqlExpression = "$field $sqlMatch ?";

diff --git a/plugins/API/API.php b/plugins/API/API.php
@@ -1663,15 +1663,17 @@ public function getSuggestedValuesForSegment($segmentName, $idSite)
             &date=$startDate,today
             &format=original
             &serialize=0
-            &flat=1
-            &segment=";
+            &flat=1";
+
+        // Select non empty fields only
+        $requestLastVisits.= "&segment=$segmentName" . Piwik_SegmentExpression::MATCH_IS_NOT_NULL . "null";
 
         // By default Live fetches all actions for all visitors, but we'd rather do this only when required
         if(!$doesSegmentNeedActionsInfo) {
             $requestLastVisits .= "&doNotFetchActions=1";
-            $requestLastVisits .= "&filter_limit=10000";
-        } else {
             $requestLastVisits .= "&filter_limit=1000";
+        } else {
+            $requestLastVisits .= "&filter_limit=500";
         }
 
         $request = new Piwik_API_Request($requestLastVisits);

diff --git a/plugins/Live/API.php b/plugins/Live/API.php
@@ -398,7 +398,6 @@ private function loadLastVisitorDetailsFromDatabase($idSite, $period = false, $d
 			GROUP BY sub.idvisit
 			ORDER BY $orderByParent
 		";
-
         try {
             $data = Piwik_FetchAll($sql, $subQuery['bind']);
         } catch (Exception $e) {
@@ -432,11 +431,6 @@ private function cleanVisitorDetails(&$visitorDetails, $idSite)
     /**
      * @param $visitorDetailsArray
      * @param $actionsLimit
-     * @param $actionDetail
-     * @param $ecommerceDetail
-     * @param $ecommerceConversion
-     * @param $detail
-     * @param $details
      * @param $timezone
      * @return array
      */

diff --git a/...tegration/expected/test_AutoSuggestAPITest_provider__API.getSuggestedValuesForSegment.xml b/...tegration/expected/test_AutoSuggestAPITest_provider__API.getSuggestedValuesForSegment.xml
@@ -1,4 +1,2 @@
 <?xml version="1.0" encoding="utf-8" ?>
-<result>
-	<row>Unknown</row>
-</result>
+<result />
diff --git a/...Integration/expected/test_ManyVisitorsOneWebsiteTest__Live.getLastVisitsDetails_month.xml b/...Integration/expected/test_ManyVisitorsOneWebsiteTest__Live.getLastVisitsDetails_month.xml
@@ -42,7 +42,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>11</visitServerHour>
+
 
 
 
@@ -180,7 +180,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>12</visitServerHour>
+
 
 
 
@@ -301,7 +301,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>11</visitServerHour>
+
 
 
 
@@ -422,7 +422,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>12</visitServerHour>
+
 
 
 
@@ -543,7 +543,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>11</visitServerHour>
+
 
 
 
@@ -681,7 +681,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>12</visitServerHour>
+
 
 
 
@@ -802,7 +802,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>11</visitServerHour>
+
 
 
 
@@ -923,7 +923,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>12</visitServerHour>
+
 
 
 
@@ -1044,7 +1044,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>11</visitServerHour>
+
 
 
 
@@ -1182,7 +1182,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>12</visitServerHour>
+
 
 
 

diff --git a/...on/expected/test_OneVisitorTwoVisits_withCookieSupport__Live.getLastVisitsDetails_day.xml b/...on/expected/test_OneVisitorTwoVisits_withCookieSupport__Live.getLastVisitsDetails_day.xml
@@ -43,7 +43,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>12</visitServerHour>
+
 
 
 
@@ -227,7 +227,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>11</visitServerHour>
+
 
 
 

diff --git a/...ntegration/expected/test_OneVisitor_NoKeywordSpecified__Live.getLastVisitsDetails_day.xml b/...ntegration/expected/test_OneVisitor_NoKeywordSpecified__Live.getLastVisitsDetails_day.xml
@@ -32,7 +32,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>13</visitServerHour>
+
 
 
 
@@ -135,7 +135,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>11</visitServerHour>
+
 
 
 

diff --git a/...est_ecommerceOrderWithItems_LiveEcommerceStatusOrdered__Live.getLastVisitsDetails_day.xml b/...est_ecommerceOrderWithItems_LiveEcommerceStatusOrdered__Live.getLastVisitsDetails_day.xml
@@ -70,7 +70,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>6</visitServerHour>
+
 
 
 
@@ -277,7 +277,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>2</visitServerHour>
+
 
 
 

diff --git a/...Unit/Integration/expected/test_ecommerceOrderWithItems__Live.getLastVisitsDetails_day.xml b/...Unit/Integration/expected/test_ecommerceOrderWithItems__Live.getLastVisitsDetails_day.xml
@@ -146,7 +146,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>7</visitServerHour>
+
 
 
 
@@ -472,7 +472,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>2</visitServerHour>
+
 
 
 
@@ -680,7 +680,7 @@
 
 		<visitLocalTime>12:34:06</visitLocalTime>
 		<visitLocalHour>12</visitLocalHour>
-		<visitServerHour>0</visitServerHour>
+