From e41ee6f2cf8fe4efad622ce979cdb08c43e97c63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marcin=20Czo=C5=82nowski?= <marcin@czolnowski.net>
Date: Fri, 29 Nov 2013 12:04:41 +0100
Subject: [PATCH] Change handling of tables using for join in segment.

---
 core/DataAccess/LogAggregator.php | 85 ++++++++++++++++++++++---------
 core/Segment.php                  | 16 +++---
 2 files changed, 66 insertions(+), 35 deletions(-)

diff --git a/core/DataAccess/LogAggregator.php b/core/DataAccess/LogAggregator.php
index 82c886236e..426569a327 100644
--- a/core/DataAccess/LogAggregator.php
+++ b/core/DataAccess/LogAggregator.php
@@ -499,35 +499,70 @@ class LogAggregator
      */
     public function queryEcommerceItems($dimension)
     {
-        $select = array(
-            "name as label",
-            self::getSqlRevenue('SUM(quantity * price)') . " as `" . Metrics::INDEX_ECOMMERCE_ITEM_REVENUE . "`",
-            self::getSqlRevenue('SUM(quantity)') . " as `" . Metrics::INDEX_ECOMMERCE_ITEM_QUANTITY . "`",
-            self::getSqlRevenue('SUM(price)') . " as `" . Metrics::INDEX_ECOMMERCE_ITEM_PRICE . "`",
-            "count(distinct idorder) as `" . Metrics::INDEX_ECOMMERCE_ORDERS . "`",
-            "count(log_conversion_item.idvisit) as `" . Metrics::INDEX_NB_VISITS . "`",
-            "case idorder when '0' then " . GoalManager::IDGOAL_CART . " else " . GoalManager::IDGOAL_ORDER . " end as ecommerceType"
-        );
+        $query = $this->generateQuery(
+            // SELECT ...
+            implode(
+                ', ',
+                array(
+                    "log_action.name AS label",
+                    sprintf(
+                        '%s AS `%d`',
+                        self::getSqlRevenue('SUM(log_conversion_item.quantity * log_conversion_item.price)'),
+                        Metrics::INDEX_ECOMMERCE_ITEM_REVENUE
+                    ),
+                    sprintf(
+                        '%s AS `%d`',
+                        self::getSqlRevenue('SUM(log_conversion_item.quantity)'),
+                        Metrics::INDEX_ECOMMERCE_ITEM_QUANTITY
+                    ),
+                    sprintf(
+                        '%s AS `%d`',
+                        self::getSqlRevenue('SUM(log_conversion_item.price)'),
+                        Metrics::INDEX_ECOMMERCE_ITEM_PRICE
+                    ),
+                    sprintf(
+                        'COUNT(DISTINCT log_conversion_item.idorder) AS `%d`',
+                        Metrics::INDEX_ECOMMERCE_ORDERS
+                    ),
+                    sprintf(
+                        'COUNT(log_conversion_item.idvisit) AS `%d`',
+                        Metrics::INDEX_NB_VISITS
+                    ),
+                    sprintf(
+                        'CASE log_conversion_item.idorder WHEN \'0\' THEN %d ELSE %d END AS ecommerceType',
+                        GoalManager::IDGOAL_CART,
+                        GoalManager::IDGOAL_ORDER
+                    )
+                )
+            ),
 
-        $from = array(
-            "log_conversion_item",
+            // FROM ...
             array(
-                "table" => "log_action",
-                "joinOn" => sprintf("log_conversion_item.%s = log_action.idaction", $dimension)
+                "log_conversion_item",
+                array(
+                    "table" => "log_action",
+                    "joinOn" => sprintf("log_conversion_item.%s = log_action.idaction", $dimension)
+                )
             ),
-            array(
-                "table" => "log_visit",
-                "joinOn" => "log_conversion_item.idvisit = log_visit.idvisit"
-            )
-        );
-        $where = "server_time >= ? AND server_time <= ? AND log_conversion_item.idsite = ? AND deleted = 0";
-        $groupBy = sprintf("ecommerceType, %s", $dimension);
 
-        $query = $this->generateQuery(
-            implode(", ", $select),
-            $from,
-            $where,
-            $groupBy,
+            // WHERE ... AND ...
+            implode(
+                ' AND ',
+                array(
+                    'log_conversion_item.server_time >= ?',
+                    'log_conversion_item.server_time <= ?',
+                    'log_conversion_item.idsite = ?',
+                    'log_conversion_item.deleted = 0'
+                )
+            ),
+
+            // GROUP BY ...
+            sprintf(
+                "ecommerceType, log_conversion_item.%s",
+                $dimension
+            ),
+
+            // ORDER ...
             false
         );
 
diff --git a/core/Segment.php b/core/Segment.php
index 5adfc7238b..07ae0a21e7 100644
--- a/core/Segment.php
+++ b/core/Segment.php
@@ -278,7 +278,7 @@ class Segment
     private function generateJoins($tables)
     {
         $knownTables = array("log_visit", "log_link_visit_action", "log_conversion", "log_conversion_item");
-        $visitsAvailable = $actionsAvailable = $conversionsAvailable = $conversionsAvailableItem = false;
+        $visitsAvailable = $actionsAvailable = $conversionsAvailable = $conversionItemAvailable = false;
         $joinWithSubSelect = false;
         $sql = '';
 
@@ -300,14 +300,6 @@ class Segment
             $tables[$visitIndex] = "log_link_visit_action";
         }
 
-        $conversionsAvailableItem = array_search("log_conversion_item", $tables);
-        if ($conversionsAvailableItem > -1) {
-            $visitIndex = array_search("log_visit", $tables);
-            if ($visitIndex > -1) {
-                unset ($tables[$visitIndex]);
-            }
-        }
-
         foreach ($tables as $i => $table) {
             if (is_array($table)) {
                 // join condition provided
@@ -356,8 +348,11 @@ class Segment
                     if ($table == "log_conversion") {
                         $joinWithSubSelect = true;
                     }
-                } elseif ($table === 'log_conversion_item') {
+                } elseif ($conversionItemAvailable && $table === 'log_visit') {
                     $join = "log_conversion_item.idvisit = log_visit.idvisit";
+                } elseif ($conversionItemAvailable && $table === 'log_link_visit_action') {
+                    $joinWithSubSelect = true;
+                    $join = "log_conversion_item.idvisit = log_link_visit_action.idvisit";
                 } else {
                     throw new Exception("Table '$table', can't be joined for segmentation");
                 }
@@ -371,6 +366,7 @@ class Segment
             $visitsAvailable = ($visitsAvailable || $table == "log_visit");
             $actionsAvailable = ($actionsAvailable || $table == "log_link_visit_action");
             $conversionsAvailable = ($conversionsAvailable || $table == "log_conversion");
+            $conversionItemAvailable = ($conversionItemAvailable || $table == "log_conversion_item");
         }
 
         return array(
-- 
GitLab