Improved join generation for segments (#10264)

* better join generation for segments
* do not update visit if there are no values to be updated

17 files changed, 1046 insertions, 198 deletions

diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index c65f36bf10..186ed0d96e 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -10,11 +10,23 @@
 namespace Piwik\DataAccess;
 
 use Exception;
-use Piwik\Common;
+use Piwik\DataAccess\LogQueryBuilder\JoinGenerator;
+use Piwik\DataAccess\LogQueryBuilder\JoinTables;
+use Piwik\Plugin\LogTablesProvider;
 use Piwik\Segment\SegmentExpression;
 
 class LogQueryBuilder
 {
+    /**
+     * @var LogTablesProvider
+     */
+    private $logTableProvider;
+
+    public function __construct(LogTablesProvider $logTablesProvider)
+    {
+        $this->logTableProvider = $logTablesProvider;
+    }
+
     public function getSelectQueryString(SegmentExpression $segmentExpression, $select, $from, $where, $bind, $groupBy,
                                          $orderBy, $limitAndOffset)
     {
@@ -31,9 +43,11 @@ class LogQueryBuilder
             $bind = array_merge($bind, $segmentSql['bind']);
         }
 
-        $joins = $this->generateJoinsString($from);
-        $joinWithSubSelect = $joins['joinWithSubSelect'];
-        $from = $joins['sql'];
+        $tables = new JoinTables($this->logTableProvider, $from);
+        $join = new JoinGenerator($tables);
+        $join->generate();
+        $from = $join->getJoinString();
+        $joinWithSubSelect = $join->shouldJoinWithSelect();
 
         // hack for https://github.com/piwik/piwik/issues/9194#issuecomment-164321612
         $useSpecialConversionGroupBy = (!empty($segmentSql)
@@ -55,192 +69,15 @@ class LogQueryBuilder
         );
     }
 
-    private function hasJoinedTableAlreadyManually($tableToFind, $joinToFind, $tables)
+    private function getKnownTables()
     {
-        foreach ($tables as $index => $table) {
-            if (is_array($table)
-                && !empty($table['table'])
-                && $table['table'] === $tableToFind
-                && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableToFind)
-                && isset($table['joinOn']) && $table['joinOn'] === $joinToFind) {
-                return true;
-            }
+        $names = array();
+        foreach ($this->logTableProvider->getAllLogTables() as $logTable) {
+            $names[] = $logTable->getName();
         }
-
-        return false;
+        return $names;
     }
 
-    private function findIndexOfManuallyAddedTable($tableToFind, $tables)
-    {
-        foreach ($tables as $index => $table) {
-            if (is_array($table)
-                && !empty($table['table'])
-                && $table['table'] === $tableToFind
-                && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableToFind)) {
-                return $index;
-            }
-        }
-    }
-
-    private function hasTableAddedManually($tableToFind, $tables)
-    {
-        $table = $this->findIndexOfManuallyAddedTable($tableToFind, $tables);
-
-        return isset($table);
-    }
-
-    /**
-     * Generate the join sql based on the needed tables
-     * @param array $tables tables to join
-     * @throws Exception if tables can't be joined
-     * @return array
-     */
-    private function generateJoinsString(&$tables)
-    {
-        $knownTables = array("log_action", "log_visit", "log_link_visit_action", "log_conversion", "log_conversion_item");
-        $visitsAvailable = $linkVisitActionsTableAvailable = $conversionsAvailable = $conversionItemAvailable = $actionsTableAvailable = false;
-        $defaultLogActionJoin = "log_link_visit_action.idaction_url = log_action.idaction";
-
-        $joinWithSubSelect = false;
-        $sql = '';
-
-        // make sure the tables are joined in the right order
-        // base table first, then action before conversion
-        // this way, conversions can be left joined on idvisit
-        $actionIndex = array_search("log_link_visit_action", $tables);
-        $conversionIndex = array_search("log_conversion", $tables);
-        if ($actionIndex > 0 && $conversionIndex > 0 && $actionIndex > $conversionIndex) {
-            $tables[$actionIndex] = "log_conversion";
-            $tables[$conversionIndex] = "log_link_visit_action";
-        }
-        // same as above: action before visit
-        $actionIndex = array_search("log_link_visit_action", $tables);
-        $visitIndex = array_search("log_visit", $tables);
-        if ($actionIndex > 0 && $visitIndex > 0 && $actionIndex > $visitIndex) {
-            $tables[$actionIndex] = "log_visit";
-            $tables[$visitIndex] = "log_link_visit_action";
-        }
-
-        // we need to add log_link_visit_action dynamically to join eg visit with action
-        $linkVisitAction = array_search("log_link_visit_action", $tables);
-        $actionIndex     = array_search("log_action", $tables);
-        if ($linkVisitAction === false && $actionIndex > 0) {
-            $tables[] = "log_link_visit_action";
-        }
-
-        if ($actionIndex > 0
-            && $this->hasTableAddedManually('log_action', $tables)
-            && !$this->hasJoinedTableAlreadyManually('log_action', $defaultLogActionJoin, $tables)) {
-            // we cannot join the same table with same alias twice, therefore we need to combine the join via AND
-            $tableIndex = $this->findIndexOfManuallyAddedTable('log_action', $tables);
-            $defaultLogActionJoin = '(' . $tables[$tableIndex]['joinOn'] . ' AND ' . $defaultLogActionJoin . ')';
-            unset($tables[$tableIndex]);
-        }
-
-        $linkVisitAction = array_search("log_link_visit_action", $tables);
-        $actionIndex     = array_search("log_action", $tables);
-        if ($linkVisitAction > 0 && $actionIndex > 0 && $linkVisitAction > $actionIndex) {
-            $tables[$actionIndex] = "log_link_visit_action";
-            $tables[$linkVisitAction] = "log_action";
-        }
-
-        foreach ($tables as $i => $table) {
-            if (is_array($table)) {
-                // join condition provided
-                $alias = isset($table['tableAlias']) ? $table['tableAlias'] : $table['table'];
-                $sql .= "
-				LEFT JOIN " . Common::prefixTable($table['table']) . " AS " . $alias
-                    . " ON " . $table['joinOn'];
-                continue;
-            }
-
-            if (!in_array($table, $knownTables)) {
-                throw new Exception("Table '$table' can't be used for segmentation");
-            }
-
-            $tableSql = Common::prefixTable($table) . " AS $table";
-
-            if ($i == 0) {
-                // first table
-                $sql .= $tableSql;
-            } else {
-
-                if ($linkVisitActionsTableAvailable && $table === 'log_action') {
-                    $join = $defaultLogActionJoin;
-
-                    if ($this->hasJoinedTableAlreadyManually($table, $join, $tables)) {
-                        $actionsTableAvailable = true;
-                        continue;
-                    }
-
-                } elseif ($linkVisitActionsTableAvailable && $table == "log_conversion") {
-                    // have actions, need conversions => join on idvisit
-                    $join = "log_conversion.idvisit = log_link_visit_action.idvisit";
-                } elseif ($linkVisitActionsTableAvailable && $table == "log_visit") {
-                    // have actions, need visits => join on idvisit
-                    $join = "log_visit.idvisit = log_link_visit_action.idvisit";
-
-                    if ($this->hasJoinedTableAlreadyManually($table, $join, $tables)) {
-                        $visitsAvailable = true;
-                        continue;
-                    }
-
-                } elseif ($visitsAvailable && $table == "log_link_visit_action") {
-                    // have visits, need actions => we have to use a more complex join
-                    // we don't hande this here, we just return joinWithSubSelect=true in this case
-                    $joinWithSubSelect = true;
-                    $join = "log_link_visit_action.idvisit = log_visit.idvisit";
-
-                    if ($this->hasJoinedTableAlreadyManually($table, $join, $tables)) {
-                        $linkVisitActionsTableAvailable = true;
-                        continue;
-                    }
-
-                } elseif ($conversionsAvailable && $table == "log_link_visit_action") {
-                    // have conversions, need actions => join on idvisit
-                    $join = "log_conversion.idvisit = log_link_visit_action.idvisit";
-                } elseif (($visitsAvailable && $table == "log_conversion")
-                    || ($conversionsAvailable && $table == "log_visit")
-                ) {
-                    // have visits, need conversion (or vice versa) => join on idvisit
-                    // notice that joining conversions on visits has lower priority than joining it on actions
-                    $join = "log_conversion.idvisit = log_visit.idvisit";
-
-                    // if conversions are joined on visits, we need a complex join
-                    if ($table == "log_conversion") {
-                        $joinWithSubSelect = true;
-                    }
-                } elseif ($conversionItemAvailable && $table === 'log_visit') {
-                    $join = "log_conversion_item.idvisit = log_visit.idvisit";
-                } elseif ($conversionItemAvailable && $table === 'log_link_visit_action') {
-                    $join = "log_conversion_item.idvisit = log_link_visit_action.idvisit";
-                } elseif ($conversionItemAvailable && $table === 'log_conversion') {
-                    $join = "log_conversion_item.idvisit = log_conversion.idvisit";
-                } else {
-                    throw new Exception("Table '$table' can't be joined for segmentation");
-                }
-
-                // the join sql the default way
-                $sql .= "
-				LEFT JOIN $tableSql ON $join";
-            }
-
-            // remember which tables are available
-            $visitsAvailable = ($visitsAvailable || $table == "log_visit");
-            $linkVisitActionsTableAvailable = ($linkVisitActionsTableAvailable || $table == "log_link_visit_action");
-            $actionsTableAvailable = ($actionsTableAvailable || $table == "log_action");
-            $conversionsAvailable = ($conversionsAvailable || $table == "log_conversion");
-            $conversionItemAvailable = ($conversionItemAvailable || $table == "log_conversion_item");
-        }
-
-        $return = array(
-            'sql'               => $sql,
-            'joinWithSubSelect' => $joinWithSubSelect
-        );
-        return $return;
-    }
-
-
     /**
      * Build a select query where actions have to be joined on visits (or conversions)
      * In this case, the query gets wrapped in another query so that grouping by visit is possible
@@ -256,7 +93,7 @@ class LogQueryBuilder
      */
     private function buildWrappedSelectQuery($select, $from, $where, $groupBy, $orderBy, $limitAndOffset, $innerGroupBy = null)
     {
-        $matchTables = "(log_visit|log_conversion_item|log_conversion|log_action)";
+        $matchTables = '(' . implode('|', $this->getKnownTables()) . ')';
         preg_match_all("/". $matchTables ."\.[a-z0-9_\*]+/", $select, $matches);
         $neededFields = array_unique($matches[0]);
 
diff --git a/core/DataAccess/LogQueryBuilder/JoinGenerator.php b/core/DataAccess/LogQueryBuilder/JoinGenerator.php
new file mode 100644
index 0000000000..1bd3ab3912
--- /dev/null
+++ b/core/DataAccess/LogQueryBuilder/JoinGenerator.php
@@ -0,0 +1,264 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+
+namespace Piwik\DataAccess\LogQueryBuilder;
+
+use Exception;
+use Piwik\Common;
+use Piwik\Tracker\LogTable;
+
+class JoinGenerator
+{
+    /**
+     * @var JoinTables
+     */
+    protected $tables;
+
+    /**
+     * @var bool
+     */
+    private $joinWithSubSelect = false;
+
+    /**
+     * @var string
+     */
+    private $joinString = '';
+
+    /**
+     * @var array
+     */
+    private $nonVisitJoins = array();
+
+    public function __construct(JoinTables $tables)
+    {
+        $this->tables = $tables;
+        $this->addMissingTablesNeededForJoins();
+    }
+    
+    private function addMissingTablesNeededForJoins()
+    {
+        foreach ($this->tables as $index => $table) {
+            if (is_array($table)) {
+                continue;
+            }
+
+            $logTable = $this->tables->getLogTable($table);
+
+            if (!$logTable->getColumnToJoinOnIdVisit()) {
+                $tableNameToJoin = $logTable->getLinkTableToBeAbleToJoinOnVisit();
+
+                if ($index > 0 && !$this->tables->hasJoinedTable($tableNameToJoin)) {
+                    $this->tables->addTableToJoin($tableNameToJoin);
+                }
+
+                if ($this->tables->hasJoinedTable($tableNameToJoin)) {
+                    $this->generateNonVisitJoins($table, $tableNameToJoin, $index);
+                }
+            }
+        }
+    }
+
+    /**
+     * Generate the join sql based on the needed tables
+     * @throws Exception if tables can't be joined
+     * @return array
+     */
+    public function generate()
+    {
+        /** @var LogTable[] $availableLogTables */
+        $availableLogTables = array();
+
+        $this->tables->sort(array($this, 'sortTablesForJoin'));
+
+        foreach ($this->tables as $i => $table) {
+            if (is_array($table)) {
+
+                // join condition provided
+                $alias = isset($table['tableAlias']) ? $table['tableAlias'] : $table['table'];
+                $this->joinString .= " LEFT JOIN " . Common::prefixTable($table['table']) . " AS " . $alias
+                                   . " ON " . $table['joinOn'];
+                continue;
+            }
+
+            $tableSql = Common::prefixTable($table) . " AS $table";
+
+            $logTable = $this->tables->getLogTable($table);
+
+            if ($i == 0) {
+                // first table
+                $this->joinString .= $tableSql;
+            } else {
+
+                $join = $this->findJoinCriteriasForTables($logTable, $availableLogTables);
+
+                if ($join === null) {
+                    $availableLogTables[$table] = $logTable;
+                    continue;
+                }
+
+                // the join sql the default way
+                $this->joinString .= " LEFT JOIN $tableSql ON " . $join;
+            }
+
+            $availableLogTables[$table] = $logTable;
+        }
+    }
+
+    public function getJoinString()
+    {
+        return $this->joinString;
+    }
+
+    public function shouldJoinWithSelect()
+    {
+        return $this->joinWithSubSelect;
+    }
+
+    /**
+     * @param LogTable $logTable
+     * @param LogTable[] $availableLogTables
+     * @return string|null   returns null in case the table is already joined, or the join string if the table needs
+     *                       to be joined
+     * @throws Exception if table cannot be joined for segmentation
+     */
+    protected function findJoinCriteriasForTables(LogTable $logTable, $availableLogTables)
+    {
+        $join = null;
+        $alternativeJoin = null;
+        $table = $logTable->getName();
+
+        foreach ($availableLogTables as $availableLogTable) {
+            if ($logTable->getColumnToJoinOnIdVisit() && $availableLogTable->getColumnToJoinOnIdVisit()) {
+
+                $join = sprintf("%s.%s = %s.%s", $table, $logTable->getColumnToJoinOnIdVisit(),
+                                                 $availableLogTable->getName(), $availableLogTable->getColumnToJoinOnIdVisit());
+                $alternativeJoin = sprintf("%s.%s = %s.%s", $availableLogTable->getName(), $availableLogTable->getColumnToJoinOnIdVisit(),
+                                                            $table, $logTable->getColumnToJoinOnIdVisit());
+
+                if ($availableLogTable->shouldJoinWithSubSelect()) {
+                    $this->joinWithSubSelect = true;
+                }
+
+                break;
+            }
+
+            if ($logTable->getColumnToJoinOnIdAction() && $availableLogTable->getColumnToJoinOnIdAction()) {
+                if (isset($this->nonVisitJoins[$logTable->getName()][$availableLogTable->getName()])) {
+                    $join = $this->nonVisitJoins[$logTable->getName()][$availableLogTable->getName()];
+                }
+
+                break;
+            }
+        }
+
+        if (!isset($join)) {
+            throw new Exception("Table '$table' can't be joined for segmentation");
+        }
+
+        if ($this->tables->hasJoinedTableManually($table, $join)
+            || $this->tables->hasJoinedTableManually($table, $alternativeJoin)) {
+            // already joined, no need to join it again
+            return null;
+        }
+
+        return $join;
+    }
+
+    /**
+     * This code is a bit tricky. We have to execute this right at the beginning before actually iterating over all the
+     * tables and generating the join string as we may have to delete a table from the tables. If we did not delete
+     * this table upfront, we would have maybe already added a joinString for that table, even though it will be later
+     * removed by another table. This means if we wouldn't delete/unset that table upfront, we would need to alter
+     * an already generated join string which would not be really nice code as well.
+     *
+     * Next problem is, because we are deleting a table, we have to remember the "joinOn" string for that table in a
+     * property "nonVisitJoins". Otherwise we would not be able to generate the correct "joinOn" string when actually
+     * iterating over all the tables to generate that string.
+     *
+     * @param $tableName
+     * @param $tableNameToJoin
+     * @param $index
+     */
+    protected function generateNonVisitJoins($tableName, $tableNameToJoin, $index)
+    {
+        $logTable = $this->tables->getLogTable($tableName);
+        $logTableToJoin = $this->tables->getLogTable($tableNameToJoin);
+
+        $nonVisitJoin = sprintf("%s.%s = %s.%s", $logTableToJoin->getName(), $logTableToJoin->getColumnToJoinOnIdAction(),
+                                                 $tableName, $logTable->getColumnToJoinOnIdAction());
+
+        $altNonVisitJoin = sprintf("%s.%s = %s.%s", $tableName, $logTable->getColumnToJoinOnIdAction(),
+                                                    $logTableToJoin->getName(), $logTableToJoin->getColumnToJoinOnIdAction());
+
+        if ($index > 0
+            && $this->tables->hasAddedTableManually($tableName)
+            && !$this->tables->hasJoinedTableManually($tableName, $nonVisitJoin)
+            && !$this->tables->hasJoinedTableManually($tableName, $altNonVisitJoin)) {
+            $tableIndex = $this->tables->findIndexOfManuallyAddedTable($tableName);
+            $nonVisitJoin = '(' . $this->tables[$tableIndex]['joinOn'] . ' AND ' . $nonVisitJoin . ')';
+            unset($this->tables[$tableIndex]);
+        }
+
+        if (!isset($this->nonVisitJoins[$tableName])) {
+            $this->nonVisitJoins[$tableName] = array();
+        }
+
+        if (!isset($this->nonVisitJoins[$tableNameToJoin])) {
+            $this->nonVisitJoins[$tableNameToJoin] = array();
+        }
+
+        $this->nonVisitJoins[$tableName][$tableNameToJoin] = $nonVisitJoin;
+        $this->nonVisitJoins[$tableNameToJoin][$tableName] = $nonVisitJoin;
+    }
+
+    public function sortTablesForJoin($tA, $tB)
+    {
+        $coreSort = array(
+            'log_link_visit_action' => 0,
+            'log_action' => 1,
+            'log_visit' => 2,
+            'log_conversion' => 3,
+            'log_conversion_item' => 4
+        );
+
+        if (is_array($tA) && is_array($tB)) {
+            return 0;
+        }
+
+        if (is_array($tA)) {
+            return -1;
+        }
+
+        if (is_array($tB)) {
+            return 1;
+        }
+
+        if (isset($coreSort[$tA])) {
+            $weightA = $coreSort[$tA];
+        } else {
+            $weightA = 999;
+        }
+        if (isset($coreSort[$tB])) {
+            $weightB = $coreSort[$tB];
+        } else {
+            $weightB = 999;
+        }
+
+        if ($weightA === $weightB) {
+            return 0;
+        }
+
+        if ($weightA > $weightB) {
+            return 1;
+        }
+
+        return -1;
+    }
+    
+}
diff --git a/core/DataAccess/LogQueryBuilder/JoinTables.php b/core/DataAccess/LogQueryBuilder/JoinTables.php
new file mode 100644
index 0000000000..d840cfba3f
--- /dev/null
+++ b/core/DataAccess/LogQueryBuilder/JoinTables.php
@@ -0,0 +1,114 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+
+namespace Piwik\DataAccess\LogQueryBuilder;
+
+use Exception;
+use Piwik\Plugin\LogTablesProvider;
+
+class JoinTables extends \ArrayObject
+{
+    /**
+     * @var LogTablesProvider
+     */
+    private $logTableProvider;
+
+    /**
+     * Tables constructor.
+     * @param LogTablesProvider $logTablesProvider
+     * @param array $tables
+     */
+    public function __construct(LogTablesProvider $logTablesProvider, $tables)
+    {
+        $this->logTableProvider = $logTablesProvider;
+
+        foreach ($tables as $table) {
+            $this->checkTableCanBeUsedForSegmentation($table);
+        }
+
+        $this->exchangeArray(array_values($tables));
+    }
+
+    public function getTables()
+    {
+        return $this->getArrayCopy();
+    }
+
+    public function addTableToJoin($tableName)
+    {
+        $this->checkTableCanBeUsedForSegmentation($tableName);
+        $this->append($tableName);
+    }
+
+    public function hasJoinedTable($tableName)
+    {
+        return in_array($tableName, $this->getTables());
+    }
+
+    public function hasJoinedTableManually($tableToFind, $joinToFind)
+    {
+        foreach ($this as $table) {
+            if (is_array($table)
+                && !empty($table['table'])
+                && $table['table'] === $tableToFind
+                && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableToFind)
+                && isset($table['joinOn']) && $table['joinOn'] === $joinToFind) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    public function getLogTable($tableName)
+    {
+        return $this->logTableProvider->getLogTable($tableName);
+    }
+
+    public function findIndexOfManuallyAddedTable($tableNameToFind)
+    {
+        foreach ($this as $index => $table) {
+            if (is_array($table)
+                && !empty($table['table'])
+                && $table['table'] === $tableNameToFind
+                && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableNameToFind)) {
+                return $index;
+            }
+        }
+    }
+
+    public function hasAddedTableManually($tableToFind)
+    {
+        $table = $this->findIndexOfManuallyAddedTable($tableToFind);
+
+        return isset($table);
+    }
+
+    public function sort($cmpFunction)
+    {
+        // we do not use $this->uasort as we do not want to maintain keys
+        $tables = $this->getTables();
+
+        // we need to make sure first table always comes first, only sort tables after the first table
+        $firstTable = array_shift($tables);
+        usort($tables, $cmpFunction);
+        array_unshift($tables, $firstTable);
+
+        $this->exchangeArray($tables);
+    }
+
+    private function checkTableCanBeUsedForSegmentation($tableName)
+    {
+        if (!is_array($tableName) && !$this->getLogTable($tableName)) {
+            throw new Exception("Table '$tableName' can't be used for segmentation");
+        }
+    }
+
+    
+}
diff --git a/core/Plugin/LogTablesProvider.php b/core/Plugin/LogTablesProvider.php
new file mode 100644
index 0000000000..92afca14c7
--- /dev/null
+++ b/core/Plugin/LogTablesProvider.php
@@ -0,0 +1,65 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\Plugin;
+
+use Piwik\Container\StaticContainer;
+use Piwik\Tracker\LogTable;
+
+class LogTablesProvider {
+
+    /**
+     * @var Manager
+     */
+    private $pluginManager;
+
+    /**
+     * @var LogTable[]
+     */
+    private $tablesCache;
+
+    public function __construct(Manager $pluginManager)
+    {
+        $this->pluginManager = $pluginManager;
+    }
+
+    /**
+     * Get an instance of a specific log table if such a log table exists.
+     *
+     * @param string $tableNameWithoutPrefix  eg "log_visit"
+     * @return LogTable|null
+     */
+    public function getLogTable($tableNameWithoutPrefix)
+    {
+        foreach ($this->getAllLogTables() as $table) {
+            if ($table->getName() === $tableNameWithoutPrefix) {
+                return $table;
+            }
+        }
+    }
+
+    /**
+     * Get all log table instances defined by any activated and loaded plugin. The returned tables are not sorted in
+     * any order.
+     * @return LogTable[]
+     */
+    public function getAllLogTables()
+    {
+        if (!isset($this->tablesCache)) {
+            $tables = $this->pluginManager->findMultipleComponents('Tracker', 'Piwik\\Tracker\\LogTable');
+
+            $this->tablesCache = array();
+            foreach ($tables as $table) {
+                $this->tablesCache[] = StaticContainer::get($table);
+            }
+        }
+
+        return $this->tablesCache;
+    }
+
+}
diff --git a/core/Tracker/LogTable.php b/core/Tracker/LogTable.php
new file mode 100644
index 0000000000..8e66f43463
--- /dev/null
+++ b/core/Tracker/LogTable.php
@@ -0,0 +1,75 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\Tracker;
+
+/**
+ * Base class for LogTables. You need to create a log table eg if you want to be able to create a segment for a custom
+ * log table.
+ */
+abstract class LogTable {
+
+    /**
+     * Get the unprefixed database table name. For example 'log_visit' or 'log_action'.
+     * @return string
+     */
+    abstract public function getName();
+
+    /**
+     * Get the name of the column that can be used to join a visit with another table. This is the name of the column
+     * that represents the "idvisit".
+     * @return string
+     */
+    public function getColumnToJoinOnIdVisit()
+    {
+        return '';
+    }
+
+    /**
+     * Get the name of the column that can be used to join an action with another table. This is the name of the column
+     * that represents the "idaction".
+     *
+     * This could be more generic eg by specifiying "$this->joinableOn = array('action' => 'idaction') and this
+     * would allow to also add more complex structures in the future but not needed for now I'd say. Let's go with
+     * simpler, more clean and expressive solution for now until needed.
+     *
+     * @return string
+     */
+    public function getColumnToJoinOnIdAction()
+    {
+        return '';
+    }
+
+    /**
+     * Defines whether this table should be joined via a subselect. Return true if a complex join is needed. (eg when
+     * having visits and needing actions, or when having visits and needing conversions, or vice versa).
+     * @return bool
+     */
+    public function shouldJoinWithSubSelect()
+    {
+        return false;
+    }
+    
+    /**
+     * Returns the name of a log table that allows to join on a visit. Eg if there is a table "action", and it is not
+     * joinable with "visit" table, it can return "log_link_visit_action" to be able to join the action table on visit
+     * via this link table.
+     *
+     * In theory there could be case where it may be needed to join via two tables, so it could be needed at some
+     * point to return an array of tables here. not sure if we should handle this case just yet. Alternatively,
+     * once needed eg in LogQueryBuilder, we should maybe better call instead ->getLinkTableToBeAbleToJoinOnVisit()
+     * again on the returned table until we have found a table that can be joined with visit.
+     *
+     * @return string
+     */
+    public function getLinkTableToBeAbleToJoinOnVisit()
+    {
+        return;
+    }
+
+}
diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php
index f4485f52fb..9de1490424 100644
--- a/core/Tracker/Visit.php
+++ b/core/Tracker/Visit.php
@@ -397,6 +397,11 @@ class Visit implements VisitInterface
      */
     protected function updateExistingVisit($valuesToUpdate)
     {
+        if (empty($valuesToUpdate)) {
+            Common::printDebug('There are no values to be updated for this visit');
+            return;
+        }
+
         $idSite = $this->request->getIdSite();
         $idVisit = (int)$this->visitProperties->getProperty('idvisit');
 
diff --git a/plugins/CoreHome/Tracker/LogTable/Action.php b/plugins/CoreHome/Tracker/LogTable/Action.php
new file mode 100644
index 0000000000..fdb69e5ec6
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/Action.php
@@ -0,0 +1,30 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class Action extends LogTable
+{
+    public function getName()
+    {
+        return 'log_action';
+    }
+
+    public function getColumnToJoinOnIdAction()
+    {
+        return 'idaction';
+    }
+
+    public function getLinkTableToBeAbleToJoinOnVisit()
+    {
+        return 'log_link_visit_action';
+    }
+
+}
diff --git a/plugins/CoreHome/Tracker/LogTable/Conversion.php b/plugins/CoreHome/Tracker/LogTable/Conversion.php
new file mode 100644
index 0000000000..e920864088
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/Conversion.php
@@ -0,0 +1,24 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class Conversion extends LogTable
+{
+    public function getName()
+    {
+        return 'log_conversion';
+    }
+
+    public function getColumnToJoinOnIdVisit()
+    {
+        return 'idvisit';
+    }
+}
\ No newline at end of file
diff --git a/plugins/CoreHome/Tracker/LogTable/ConversionItem.php b/plugins/CoreHome/Tracker/LogTable/ConversionItem.php
new file mode 100644
index 0000000000..566841b4fa
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/ConversionItem.php
@@ -0,0 +1,25 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class ConversionItem extends LogTable
+{
+    public function getName()
+    {
+        return 'log_conversion_item';
+    }
+
+    public function getColumnToJoinOnIdVisit()
+    {
+        return 'idvisit';
+    }
+    
+}
diff --git a/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php b/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php
new file mode 100644
index 0000000000..cb102d407c
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/LinkVisitAction.php
@@ -0,0 +1,29 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class LinkVisitAction extends LogTable
+{
+    public function getName()
+    {
+        return 'log_link_visit_action';
+    }
+
+    public function getColumnToJoinOnIdAction()
+    {
+        return 'idaction_url';
+    }
+
+    public function getColumnToJoinOnIdVisit()
+    {
+        return 'idvisit';
+    }
+}
diff --git a/plugins/CoreHome/Tracker/LogTable/Visit.php b/plugins/CoreHome/Tracker/LogTable/Visit.php
new file mode 100644
index 0000000000..f403c37697
--- /dev/null
+++ b/plugins/CoreHome/Tracker/LogTable/Visit.php
@@ -0,0 +1,30 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\CoreHome\Tracker\LogTable;
+
+use Piwik\Tracker\LogTable;
+
+class Visit extends LogTable
+{
+    public function getName()
+    {
+        return 'log_visit';
+    }
+
+    public function getColumnToJoinOnIdVisit()
+    {
+        return 'idvisit';
+    }
+
+    public function shouldJoinWithSubSelect()
+    {
+        return true;
+    }
+
+}
\ No newline at end of file
diff --git a/plugins/SegmentEditor/SegmentQueryDecorator.php b/plugins/SegmentEditor/SegmentQueryDecorator.php
index d501f6f9b1..130cc59bd5 100644
--- a/plugins/SegmentEditor/SegmentQueryDecorator.php
+++ b/plugins/SegmentEditor/SegmentQueryDecorator.php
@@ -9,6 +9,7 @@
 namespace Piwik\Plugins\SegmentEditor;
 
 use Piwik\DataAccess\LogQueryBuilder;
+use Piwik\Plugin\LogTablesProvider;
 use Piwik\Plugins\SegmentEditor\Services\StoredSegmentService;
 use Piwik\Segment\SegmentExpression;
 use Piwik\SettingsServer;
@@ -26,9 +27,10 @@ class SegmentQueryDecorator extends LogQueryBuilder
      */
     private $storedSegmentService;
 
-    public function __construct(StoredSegmentService $storedSegmentService)
+    public function __construct(StoredSegmentService $storedSegmentService, LogTablesProvider $logTablesProvider)
     {
         $this->storedSegmentService = $storedSegmentService;
+        parent::__construct($logTablesProvider);
     }
 
     public function getSelectQueryString(SegmentExpression $segmentExpression, $select, $from, $where, $bind, $groupBy,
diff --git a/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php b/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php
index 6ecad82f2f..7e38188966 100644
--- a/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php
+++ b/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php
@@ -11,6 +11,7 @@ namespace Piwik\Plugins\SegmentEditor\tests\Unit;
 use Piwik\Plugins\SegmentEditor\SegmentQueryDecorator;
 use Piwik\Plugins\SegmentEditor\Services\StoredSegmentService;
 use Piwik\Segment\SegmentExpression;
+use Piwik\Tests\Framework\Mock\Plugin\LogTablesProvider;
 
 /**
  * @group SegmentEditor
@@ -36,7 +37,8 @@ class SegmentQueryDecoratorTest extends \PHPUnit_Framework_TestCase
 
         /** @var StoredSegmentService $service */
         $service = $this->getMockSegmentEditorService();
-        $this->decorator = new SegmentQueryDecorator($service);
+        $logTables = new LogTablesProvider();
+        $this->decorator = new SegmentQueryDecorator($service, $logTables);
     }
 
     public function test_getSelectQueryString_DoesNotDecorateSql_WhenNoSegmentUsed()
@@ -86,4 +88,5 @@ class SegmentQueryDecoratorTest extends \PHPUnit_Framework_TestCase
         $mock->expects($this->any())->method('getAllSegmentsAndIgnoreVisibility')->willReturn(self::$storedSegments);
         return $mock;
     }
+
 }
diff --git a/tests/PHPUnit/Framework/Mock/Plugin/LogTablesProvider.php b/tests/PHPUnit/Framework/Mock/Plugin/LogTablesProvider.php
new file mode 100644
index 0000000000..c563e250cd
--- /dev/null
+++ b/tests/PHPUnit/Framework/Mock/Plugin/LogTablesProvider.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Piwik\Tests\Framework\Mock\Plugin;
+
+use Piwik\Plugins\CoreHome\Tracker\LogTable\Action;
+use Piwik\Plugins\CoreHome\Tracker\LogTable\Conversion;
+use Piwik\Plugins\CoreHome\Tracker\LogTable\ConversionItem;
+use Piwik\Plugins\CoreHome\Tracker\LogTable\LinkVisitAction;
+use Piwik\Plugins\CoreHome\Tracker\LogTable\Visit;
+
+class LogTablesProvider extends \Piwik\Plugin\LogTablesProvider
+{
+    public function __construct()
+    {
+    }
+
+    public function getAllLogTables()
+    {
+        return array(
+            new Visit(),
+            new Action(),
+            new LinkVisitAction(),
+            new ConversionItem(),
+            new Conversion()
+        );
+    }
+
+}
diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index 9f626e7969..37d13cc2e7 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -271,7 +271,7 @@ class SegmentTest extends IntegrationTestCase
                     *
                 FROM
                     " . Common::prefixTable('log_conversion') . " AS log_conversion
-                    LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
+                    LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
                 WHERE
                     ( log_conversion.idvisit = ? )
                     AND
@@ -361,7 +361,7 @@ class SegmentTest extends IntegrationTestCase
                     *
                 FROM
                     " . Common::prefixTable('log_conversion') . " AS log_conversion
-                    LEFT JOIN " . Common::prefixTable('log_visit') . " AS log_visit ON log_conversion.idvisit = log_visit.idvisit
+                    LEFT JOIN " . Common::prefixTable('log_visit') . " AS log_visit ON log_visit.idvisit = log_conversion.idvisit
                 WHERE
                     ( log_conversion.idvisit = ? )
                     AND
@@ -524,7 +524,7 @@ class SegmentTest extends IntegrationTestCase
                 FROM
                     " . Common::prefixTable('log_visit') . " AS log_visit
                     LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit
-                    LEFT JOIN " . Common::prefixTable('log_conversion') . " AS log_conversion ON log_conversion.idvisit = log_link_visit_action.idvisit
+                    LEFT JOIN " . Common::prefixTable('log_conversion') . " AS log_conversion ON log_conversion.idvisit = log_visit.idvisit
                 WHERE
                      log_conversion.idgoal = ? AND HOUR(log_visit.visit_last_action_time) = ? AND log_link_visit_action.custom_var_k1 = ?
                       AND (
@@ -693,7 +693,7 @@ class SegmentTest extends IntegrationTestCase
              SELECT log_conversion.idgoal AS `idgoal`, log_conversion.custom_dimension_1 AS `custom_dimension_1`, count(*) AS `1`, count(distinct log_conversion.idvisit) AS `3`,
              FROM $logConversionsTable AS log_conversion
                   LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action
-                       ON log_conversion.idvisit = log_link_visit_action.idvisit
+                       ON log_link_visit_action.idvisit = log_conversion.idvisit
                   LEFT JOIN $logActionTable AS log_action
                        ON log_link_visit_action.idaction_url = log_action.idaction
              WHERE ( log_conversion.server_time >= ?
@@ -761,7 +761,7 @@ class SegmentTest extends IntegrationTestCase
                     SUM(log_conversion.items) AS `8`
                 FROM
                     " . Common::prefixTable('log_conversion') . " AS log_conversion
-                    LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
+                    LEFT JOIN " . Common::prefixTable('log_link_visit_action') . " AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
                 WHERE
                     ( log_conversion.idsite IN (?) )
                     AND
@@ -1289,7 +1289,7 @@ class SegmentTest extends IntegrationTestCase
                 FROM (
                       SELECT log_conversion.idgoal, log_conversion.idvisit, log_conversion.revenue, log_conversion.items
                       FROM $logConversionTable AS log_conversion
-                        LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
+                        LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
                       WHERE ( log_conversion.server_time >= ?
                           AND log_conversion.server_time <= ?
                           AND log_conversion.idsite IN (?) )
@@ -1340,7 +1340,7 @@ class SegmentTest extends IntegrationTestCase
                 FROM (
                       SELECT log_conversion.idgoal, log_conversion.referer_type, log_conversion.referer_name, log_conversion.referer_keyword, log_conversion.idvisit, log_conversion.revenue
                       FROM $logConversionTable AS log_conversion
-                        LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
+                        LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
                       WHERE ( log_conversion.server_time >= ?
                           AND log_conversion.server_time <= ?
                           AND log_conversion.idsite IN (?) )
@@ -1385,8 +1385,8 @@ class SegmentTest extends IntegrationTestCase
                 FROM (
                     SELECT log_conversion.idgoal, log_conversion.idvisit, log_conversion.revenue
                     FROM $logConversionTable AS log_conversion
-                       LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
-                       LEFT JOIN $logVisitTable AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit
+                       LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_link_visit_action.idvisit = log_conversion.idvisit
+                       LEFT JOIN $logVisitTable AS log_visit ON log_visit.idvisit = log_conversion.idvisit
                     WHERE ( log_conversion.server_time >= ?
                         AND log_conversion.server_time <= ?
                         AND log_conversion.idsite IN (?) )
diff --git a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
new file mode 100644
index 0000000000..cd572e4849
--- /dev/null
+++ b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
@@ -0,0 +1,171 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\Tests\Unit\DataAccess;
+
+use Piwik\DataAccess\LogQueryBuilder\JoinGenerator;
+use Piwik\DataAccess\LogQueryBuilder\JoinTables;
+use Piwik\Tests\Framework\Mock\Plugin\LogTablesProvider;
+use Piwik\Tracker\Visit;
+
+/**
+ * @group Core
+ */
+class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @var JoinGenerator
+     */
+    private $generator;
+
+    public function setUp()
+    {
+        $this->generator = $this->makeTables(array(
+            'log_visit',
+            array('table' => 'log_conversion', 'joinOn' => 'log_conversion.idvisit = log_visit.idvisit'),
+            'log_action'));
+    }
+
+    public function test_constructor_shouldAddTablesIfNeeded()
+    {
+        $tables = $this->makeTables(array('log_visit', 'log_action'));
+        $this->makeGenerator($tables);
+
+        $this->assertEquals(array('log_visit', 'log_action', 'log_link_visit_action'), $tables->getTables());
+    }
+
+    public function test_generate_shouldJoinWithSubselect_IfBaseTableIsLogVisit()
+    {
+        $generator = $this->generate(array('log_visit', 'log_action'));
+        $this->assertTrue($generator->shouldJoinWithSelect());
+    }
+
+    public function test_generate_shouldNotJoinWithSubselect_IfBaseTableIsLogVisitButNoTableToJoin()
+    {
+        $generator = $this->generate(array('log_visit'));
+        $this->assertFalse($generator->shouldJoinWithSelect());
+    }
+
+    public function test_generate_shouldNotJoinWithSubselect_IfLogVisitIsGivenButItIsNotBaseTable()
+    {
+        $generator = $this->generate(array('log_conversion', 'log_visit'));
+        $this->assertFalse($generator->shouldJoinWithSelect());
+    }
+
+    public function test_generate_getJoinString()
+    {
+        $generator = $this->generate(array('log_action', 'log_link_visit_action', 'log_visit'));
+
+        $expected  = 'log_action AS log_action ';
+        $expected .= 'LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idaction_url = log_action.idaction ';
+        $expected .= 'LEFT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit';
+        $this->assertEquals($expected, $generator->getJoinString());
+    }
+
+    public function test_generate_getJoinString_OnlyOneTable()
+    {
+        $generator = $this->generate(array('log_visit'));
+        $this->assertEquals('log_visit AS log_visit', $generator->getJoinString());
+    }
+
+    public function test_generate_getJoinString_OnlyOneActionTable()
+    {
+        $generator = $this->generate(array('log_action'));
+        $this->assertEquals('log_action AS log_action', $generator->getJoinString());
+    }
+
+    public function test_generate_getJoinString_OnlyActionTables()
+    {
+        $generator = $this->generate(array('log_link_visit_action', 'log_action'));
+        $expected  = 'log_link_visit_action AS log_link_visit_action';
+        $expected .= ' LEFT JOIN log_action AS log_action ON log_link_visit_action.idaction_url = log_action.idaction';
+        $this->assertEquals($expected, $generator->getJoinString());
+    }
+
+    public function test_generate_getJoinString_manuallyJoinedAlready()
+    {
+        $generator = $this->generate(array(
+            'log_link_visit_action',
+            array('table' => 'log_visit', 'joinOn' => 'log_visit.idvisit = log_link_visit_action.idvisit'),
+            array('table' => 'log_action', 'joinOn' => 'log_link_visit_action.idaction_name = log_action.idaction'),
+            'log_action'
+        ));
+
+        $expected  = 'log_link_visit_action AS log_link_visit_action ';
+        $expected .= 'LEFT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit ';
+        $expected .= 'LEFT JOIN log_action AS log_action ON (log_link_visit_action.idaction_name = log_action.idaction AND log_link_visit_action.idaction_url = log_action.idaction)';
+        $this->assertEquals($expected, $generator->getJoinString());
+    }
+
+    public function test_generate_getJoinString_allTables()
+    {
+        $generator = $this->generate(array(
+            'log_action',
+            'log_conversion_item',
+            'log_link_visit_action',
+            'log_conversion',
+            'log_visit',
+        ));
+
+        $expected  = 'log_action AS log_action ';
+        $expected .= 'LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idaction_url = log_action.idaction ';
+        $expected .= 'LEFT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit ';
+        $expected .= 'LEFT JOIN log_conversion AS log_conversion ON log_conversion.idvisit = log_link_visit_action.idvisit ';
+        $expected .= 'LEFT JOIN log_conversion_item AS log_conversion_item ON log_conversion_item.idvisit = log_link_visit_action.idvisit';
+        $this->assertEquals($expected, $generator->getJoinString());
+    }
+
+    public function test_sortTablesForJoin_shouldSortTablesAsSpecified()
+    {
+        $tables = array(
+            'log_action',
+            array('table' => 'log_conversion', 'joinOn' => 'log_conversion.idvisit = log_visit.idvisit'),
+            'log_conversion_item',
+            'log_link_visit_action',
+            'log_conversion',
+            'log_visit',
+        );
+
+        $generator = $this->makeGenerator($tables);
+        $tables[] = 'log_foo_bar';
+        usort($tables, array($generator, 'sortTablesForJoin'));
+
+        $expected = array(
+            array('table' => 'log_conversion', 'joinOn' => 'log_conversion.idvisit = log_visit.idvisit'),
+            'log_link_visit_action',
+            'log_action',
+            'log_visit',
+            'log_conversion',
+            'log_conversion_item',
+            'log_foo_bar'
+        );
+
+        $this->assertEquals($expected, $tables);
+    }
+
+    private function generate($tables)
+    {
+        $generator = $this->makeGenerator($tables);
+        $generator->generate();
+        return $generator;
+    }
+
+    private function makeGenerator($tables)
+    {
+        if (is_array($tables)) {
+            $tables = $this->makeTables($tables);
+        }
+
+        return new JoinGenerator($tables);
+    }
+
+    private function makeTables($tables)
+    {
+        return new JoinTables(new LogTablesProvider(), $tables);
+    }
+}
\ No newline at end of file
diff --git a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php
new file mode 100644
index 0000000000..8f5a52fd0b
--- /dev/null
+++ b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php
@@ -0,0 +1,146 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\Tests\Unit\DataAccess;
+
+use Piwik\DataAccess\LogQueryBuilder\JoinTables;
+use Piwik\Tests\Framework\Mock\Plugin\LogTablesProvider;
+use Piwik\Tracker\Visit;
+
+/**
+ * @group Core
+ */
+class JoinTablesTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @var JoinTables
+     */
+    private $tables;
+
+    public function setUp()
+    {
+        $this->tables = $this->makeTables(array(
+            'log_visit',
+            array('table' => 'log_conversion', 'joinOn' => 'log_conversion.idvisit = log_visit.idvisit'),
+            'log_action'));
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage Table 'log_foo_bar_baz' can't be used for segmentation
+     */
+    public function test_construct_shouldThrowException_IfTableIsNotPossibleToJoin()
+    {
+        $this->makeTables(array('log_visit', 'log_foo_bar_baz'));
+    }
+
+    public function test_hasJoinedTable_shouldDetectIfTableIsAlreadyAdded()
+    {
+        $this->assertTrue($this->tables->hasJoinedTable('log_visit'));
+        $this->assertTrue($this->tables->hasJoinedTable('log_action'));
+
+        $this->assertFalse($this->tables->hasJoinedTable('log_foo_bar_baz'));
+        $this->assertFalse($this->tables->hasJoinedTable('log_conversion')); // we do not check for manually joined tables
+    }
+
+    public function test_addTableToJoin_shouldAddGivenTable()
+    {
+        $table = 'log_conversion_item';
+        $this->assertFalse($this->tables->hasJoinedTable($table));
+
+        $this->tables->addTableToJoin($table);
+
+        $this->assertTrue($this->tables->hasJoinedTable($table));
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage Table 'log_foo_bar_baz' can't be used for segmentation
+     */
+    public function test_addTableToJoin_shouldCheckIfTableCanBeUsedForSegmentation()
+    {
+        $table = 'log_foo_bar_baz';
+        $this->assertFalse($this->tables->hasJoinedTable($table));
+
+        $this->tables->addTableToJoin($table);
+
+        $this->assertTrue($this->tables->hasJoinedTable($table));
+    }
+
+    public function test_hasJoinedTableManually_shouldReturnTrue_IfTableJoinExistsExactlyAsGiven()
+    {
+        $result = $this->tables->hasJoinedTableManually('log_conversion', 'log_conversion.idvisit = log_visit.idvisit');
+
+        $this->assertTrue($result);
+    }
+
+    public function test_hasJoinedTableManually_shouldReturnFalse_IfTableOrJoinDoesNotMatch()
+    {
+        $result = $this->tables->hasJoinedTableManually('log_foo_bar_baz', 'log_conversion.idvisit = log_visit.idvisit');
+        $this->assertFalse($result);
+
+        $result = $this->tables->hasJoinedTableManually('log_conversion', 'log_foo_bar_baz.idvisit = log_visit.idvisit');
+        $this->assertFalse($result);
+    }
+
+    public function test_hasAddedTableManually_shouldReturnTrue_IfTableWasAddedManually()
+    {
+        $result = $this->tables->hasAddedTableManually('log_conversion');
+
+        $this->assertTrue($result);
+    }
+
+    public function test_hasAddedTableManually_shouldReturnFalse_IfTableWasNotAddedManually()
+    {
+        $result = $this->tables->hasAddedTableManually('log_foo_bar_baz');
+        $this->assertFalse($result);
+
+        $result = $this->tables->hasAddedTableManually('log_conversion_item');
+        $this->assertFalse($result);
+    }
+
+    public function test_getLogTable_shouldReturnInstanceOfLogTable_IfTableExists()
+    {
+        $visit = $this->tables->getLogTable('log_visit');
+        $this->assertFalse($visit instanceof Visit);
+    }
+
+    public function test_getLogTable_shouldReturnNull_IfLogTableDoesNotExist()
+    {
+        $visit = $this->tables->getLogTable('log_foo_bar_baz');
+        $this->assertNull($visit);
+    }
+
+    public function test_findIndexOfManuallyAddedTable_shouldReturnTheIndex_IfTableWasAddedManually()
+    {
+        $this->assertSame(1, $this->tables->findIndexOfManuallyAddedTable('log_conversion'));
+    }
+
+    public function test_findIndexOfManuallyAddedTable_shouldReturnNull_IfTableWasNotAddedManually()
+    {
+        $this->assertNull($this->tables->findIndexOfManuallyAddedTable('log_visit'));
+        $this->assertNull($this->tables->findIndexOfManuallyAddedTable('log_action'));
+        $this->assertNull($this->tables->findIndexOfManuallyAddedTable('log_foo_bar_baz'));
+    }
+
+    public function test_sort_shouldNeverSortFirstEntry_AndNotMaintainKeys()
+    {
+        $tables = $this->makeTables(array('log_conversion', 'log_visit', 'log_action', 'log_conversion_item'));
+        $tables->sort(function($a, $b) {
+            return strcmp($a, $b);
+        });
+
+        $expected = array('log_conversion', 'log_action', 'log_conversion_item', 'log_visit');
+        $this->assertEquals($expected, $tables->getTables());
+    }
+
+    private function makeTables($tables)
+    {
+        return new JoinTables(new LogTablesProvider(), $tables);
+    }
+}
\ No newline at end of file