Fix bugs in table sorting query builder (#12154)

* fix bugs in table sorting query builder

* fix tests

3 files changed, 179 insertions, 49 deletions

diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index 0558d6a381..bf24a9d4af 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -131,6 +131,7 @@ class SegmentTest extends IntegrationTestCase
 
         $segment = new Segment($segment, $idSites = array());
         $sql = $segment->getSelectQuery($select, $from, false);
+        $this->assertQueryDoesNotFail($sql);
 
         $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($sql));
 
@@ -152,6 +153,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -179,6 +181,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -207,6 +210,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -243,6 +247,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -271,6 +276,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -299,6 +305,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -334,6 +341,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -361,6 +369,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -401,6 +410,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $logVisitTable = Common::prefixTable('log_visit');
         $logActionTable = Common::prefixTable('log_action');
@@ -436,6 +446,7 @@ class SegmentTest extends IntegrationTestCase
                   sum(case log_visit.visit_total_actions when 1 then 1 when 0 then 1 else 0 end) as `6`';
         $from  = array(
             'log_link_visit_action',
+            'log_visit',
             array(
                 'table' => 'log_link_visit_action',
                 'tableAlias' => 'log_link_visit_action_foo',
@@ -478,9 +489,11 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $logActionTable = Common::prefixTable('log_action');
         $logLinkVisitActionTable = Common::prefixTable('log_link_visit_action');
+        $logVisitTable = Common::prefixTable('log_visit');
 
         $expected = array(
             "sql"  => "
@@ -488,21 +501,15 @@ class SegmentTest extends IntegrationTestCase
                     log_action.name as url,
                     sum(log_link_visit_action.time_spent) as `13`,
                     sum(case log_visit.visit_total_actions when 1 then 1 when 0 then 1 else 0 end) as `6`
-             FROM $logLinkVisitActionTable AS log_link_visit_action
-                  LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action_foo
-                       ON log_link_visit_action.idvisit = log_link_visit_action_foo.idvisit
-                  LEFT JOIN $logActionTable AS log_action_foo
-                       ON log_link_visit_action_foo.idaction_url = log_action_foo.idaction 
-                  LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action_bar
-                       ON log_link_visit_action.idvisit = log_link_visit_action_bar.idvisit
-                  LEFT JOIN $logActionTable AS log_action_bar
-                       ON log_link_visit_action_bar.idaction_url = log_action_bar.idaction 
-                  LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action_baz
-                       ON log_link_visit_action.idvisit = log_link_visit_action_baz.idvisit
-                  LEFT JOIN $logActionTable AS log_action_baz
-                       ON log_link_visit_action_baz.idaction_url = log_action_baz.idaction 
-                  LEFT JOIN $logActionTable AS log_action
-                       ON log_link_visit_action.idaction_url = log_action.idaction 
+             FROM $logLinkVisitActionTable AS log_link_visit_action 
+             LEFT JOIN $logActionTable AS log_action ON log_link_visit_action.idaction_url = log_action.idaction 
+             LEFT JOIN $logVisitTable AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit 
+             LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action_foo ON log_link_visit_action.idvisit = log_link_visit_action_foo.idvisit 
+             LEFT JOIN $logActionTable AS log_action_foo ON log_link_visit_action_foo.idaction_url = log_action_foo.idaction 
+             LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action_bar ON log_link_visit_action.idvisit = log_link_visit_action_bar.idvisit 
+             LEFT JOIN $logActionTable AS log_action_bar ON log_link_visit_action_bar.idaction_url = log_action_bar.idaction 
+             LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action_baz ON log_link_visit_action.idvisit = log_link_visit_action_baz.idvisit 
+             LEFT JOIN $logActionTable AS log_action_baz ON log_link_visit_action_baz.idaction_url = log_action_baz.idaction
              WHERE ( log_link_visit_action.server_time >= ?
                  AND log_link_visit_action.server_time <= ?
                  AND log_link_visit_action.idsite = ? )
@@ -534,6 +541,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $logVisitTable = Common::prefixTable('log_visit');
         $logActionTable = Common::prefixTable('log_action');
@@ -545,11 +553,9 @@ class SegmentTest extends IntegrationTestCase
                     log_action.name as url,
                     sum(log_link_visit_action.time_spent) as `13`,
                     sum(case log_visit.visit_total_actions when 1 then 1 when 0 then 1 else 0 end) as `6`
-             FROM $logLinkVisitActionTable AS log_link_visit_action
-                  LEFT JOIN $logVisitTable AS log_visit
-                       ON log_visit.idvisit = log_link_visit_action.idvisit
-                  LEFT JOIN $logActionTable AS log_action
-                       ON log_link_visit_action.idaction_name = log_action.idaction
+             FROM $logLinkVisitActionTable AS log_link_visit_action 
+             LEFT JOIN $logVisitTable AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit 
+             LEFT JOIN $logActionTable AS log_action ON log_link_visit_action.idaction_name = log_action.idaction
              WHERE ( log_link_visit_action.server_time >= ?
                  AND log_link_visit_action.server_time <= ?
                  AND log_link_visit_action.idsite = ? )
@@ -574,6 +580,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -605,6 +612,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -650,6 +658,7 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $logVisitTable = Common::prefixTable('log_visit');
         $logActionTable = Common::prefixTable('log_action');
@@ -696,24 +705,21 @@ class SegmentTest extends IntegrationTestCase
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $logVisitTable = Common::prefixTable('log_visit');
         $logActionTable = Common::prefixTable('log_action');
         $logLinkVisitActionTable = Common::prefixTable('log_link_visit_action');
-
         $expected = array(
             "sql"  => "
              SELECT log_link_visit_action.custom_dimension_1,
                     actionAlias.name as url,
                     sum(log_link_visit_action.time_spent) as `13`,
                     sum(case visitAlias.visit_total_actions when 1 then 1 when 0 then 1 else 0 end) as `6`
-             FROM $logLinkVisitActionTable AS log_link_visit_action
-                  LEFT JOIN $logVisitTable AS visitAlias
-                       ON visitAlias.idvisit = log_link_visit_action.idvisit
-                  LEFT JOIN $logActionTable AS actionAlias
-                       ON log_link_visit_action.idaction_url = actionAlias.idaction
-                  LEFT JOIN $logActionTable AS log_action
-                       ON log_link_visit_action.idaction_url = log_action.idaction
+             FROM $logLinkVisitActionTable AS log_link_visit_action 
+             LEFT JOIN $logActionTable AS log_action ON log_link_visit_action.idaction_url = log_action.idaction 
+             LEFT JOIN $logVisitTable AS visitAlias ON visitAlias.idvisit = log_link_visit_action.idvisit 
+             LEFT JOIN $logActionTable AS actionAlias ON log_link_visit_action.idaction_url = actionAlias.idaction
              WHERE ( log_link_visit_action.server_time >= ?
                  AND log_link_visit_action.server_time <= ?
                  AND log_link_visit_action.idsite = ? )
@@ -723,6 +729,12 @@ class SegmentTest extends IntegrationTestCase
         $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
     }
 
+    private function assertQueryDoesNotFail($query)
+    {
+        Db::fetchAll($query['sql'], $query['bind']);
+        $this->assertTrue(true);
+    }
+
     public function test_getSelectQuery_whenJoinLogLinkVisitActionOnAction()
     {
         $actionType = 3;
@@ -943,6 +955,10 @@ class SegmentTest extends IntegrationTestCase
         $groupBy = 'log_action_visit_entry_idaction_name.name, log_action_idaction_event_action.name';
         $limit = 33;
 
+        $logVisitTable = Common::prefixTable('log_visit');
+        $logLinkVisitActionTable = Common::prefixTable('log_link_visit_action');
+        $logActionTable = Common::prefixTable('log_action');
+
         $query = $segment->getSelectQuery($select, $from, $where, $bind, $orderBy, $groupBy, $limit);
 
         $expected = array(
@@ -950,10 +966,10 @@ class SegmentTest extends IntegrationTestCase
                 SELECT log_inner.name AS 'EntryPageTitle', log_inner.name02fd90a35677a359ea5611a4bc456a6f AS 'EventAction', count(distinct log_inner.idvisit) AS 'nb_uniq_visits', count(distinct log_inner.idvisitor) AS 'nb_uniq_visitors', sum(case log_inner.visit_total_actions when 1 then 1 when 0 then 1 else 0 end) AS 'bounce_count', sum(log_inner.visit_total_actions) AS 'sum_actions', sum(log_inner.visit_goal_converted) AS 'sum_visit_goal_converted' 
                 FROM ( 
                   SELECT log_action_visit_entry_idaction_name.name, log_action_idaction_event_action.name as name02fd90a35677a359ea5611a4bc456a6f, log_visit.idvisit, log_visit.idvisitor, log_visit.visit_total_actions, log_visit.visit_goal_converted 
-                  FROM log_visit AS log_visit 
-                  LEFT JOIN log_action AS log_action_visit_entry_idaction_name ON log_visit.visit_entry_idaction_name = log_action_visit_entry_idaction_name.idaction 
-                  LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit 
-                  LEFT JOIN log_action AS log_action_idaction_event_action ON log_link_visit_action.idaction_event_action = log_action_idaction_event_action.idaction 
+                  FROM $logVisitTable AS log_visit 
+                  LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit 
+                  LEFT JOIN $logActionTable AS log_action_visit_entry_idaction_name ON log_visit.visit_entry_idaction_name = log_action_visit_entry_idaction_name.idaction 
+                  LEFT JOIN $logActionTable AS log_action_idaction_event_action ON log_link_visit_action.idaction_event_action = log_action_idaction_event_action.idaction
                   ORDER BY nb_uniq_visits, log_action_idaction_event_action.name LIMIT 0, 33 ) 
                 AS log_inner 
                 GROUP BY log_inner.name, log_inner.name02fd90a35677a359ea5611a4bc456a6f 
@@ -1402,6 +1418,7 @@ log_visit.visit_total_actions
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $this->assertQueryDoesNotFail($query);
 
         $expected = array(
             "sql"  => "
@@ -1463,6 +1480,7 @@ log_visit.visit_total_actions
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind, $orderBy = false, $groupBy);
+        $this->assertQueryDoesNotFail($query);
 
         $logConversionTable = Common::prefixTable('log_conversion');
         $logLinkVisitActionTable = Common::prefixTable('log_link_visit_action');
@@ -1508,6 +1526,7 @@ log_visit.visit_total_actions
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind, $orderBy = false, $groupBy);
+        $this->assertQueryDoesNotFail($query);
 
         $logConversionTable = Common::prefixTable('log_conversion');
         $logLinkVisitActionTable = Common::prefixTable('log_link_visit_action');
@@ -1556,6 +1575,7 @@ log_visit.visit_total_actions
         $segment = new Segment($segment, $idSites = array());
 
         $query = $segment->getSelectQuery($select, $from, $where, $bind, $orderBy = false, $groupBy);
+        $this->assertQueryDoesNotFail($query);
 
         $logConversionTable = Common::prefixTable('log_conversion');
         $logLinkVisitActionTable = Common::prefixTable('log_link_visit_action');
diff --git a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
index 0f75716c9c..0903098c05 100644
--- a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
+++ b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php
@@ -97,8 +97,8 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
         ));
 
         $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)';
+        $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) ';
+        $expected .= 'LEFT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit';
         $this->assertEquals($expected, $generator->getJoinString());
     }
 
@@ -125,8 +125,8 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
         ));
 
         $expected  = 'log_visit AS log_visit ';
-        $expected .= 'LEFT JOIN log_conversion AS log_conversion ON log_visit.idvisit2 = log_conversion.idvisit2 ';
-        $expected .= 'LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit';
+        $expected .= 'LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idvisit = log_visit.idvisit ';
+        $expected .= 'LEFT JOIN log_conversion AS log_conversion ON log_visit.idvisit2 = log_conversion.idvisit2';
         $this->assertEquals($expected, $generator->getJoinString());
     }
 
@@ -163,8 +163,8 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
         ));
 
         $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)';
+        $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) ';
+        $expected .= 'LEFT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit';
         $this->assertEquals($expected, $generator->getJoinString());
     }
 
@@ -182,9 +182,9 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
         ));
 
         $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 ';
         $expected .= 'RIGHT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit ';
-        $expected .= 'RIGHT JOIN log_action AS log_action_r ON log_link_visit_action.idaction_test = log_action_r.idaction ';
-        $expected .= 'LEFT JOIN log_action AS log_action ON log_link_visit_action.idaction_url = log_action.idaction';
+        $expected .= 'RIGHT JOIN log_action AS log_action_r ON log_link_visit_action.idaction_test = log_action_r.idaction';
         $this->assertEquals($expected, $generator->getJoinString());
     }
 
@@ -222,19 +222,97 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
         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'
+            'log_foo_bar',
+            array('table' => 'log_conversion', 'joinOn' => 'log_conversion.idvisit = log_visit.idvisit'),
+        );
+
+        $this->assertEquals($expected, $tables);
+    }
+
+    public function test_sortTablesForJoin_anotherTestMakingSureWorksOhPhp5_5()
+    {
+        $tables = array (
+            1 => 'log_link_visit_action',
+            2 =>
+                array (
+                    'table' => 'log_action',
+                    'tableAlias' => 'log_action_idaction_name',
+                    'joinOn' => 'log_link_visit_action.idaction_name = log_action_idaction_name.idaction',
+                ),
+            3 =>
+                array (
+                    'table' => 'log_action',
+                    'tableAlias' => 'log_action_visit_exit_idaction_name',
+                    'joinOn' => 'log_visit.visit_exit_idaction_name = log_action_visit_exit_idaction_name.idaction',
+                ),
+        )
+        ;
+
+        $generator = $this->makeGenerator($tables);
+        usort($tables, array($generator, 'sortTablesForJoin'));
+
+        $expected = array(
+            'log_link_visit_action',
+            array (
+                'table' => 'log_action',
+                'tableAlias' => 'log_action_visit_exit_idaction_name',
+                'joinOn' => 'log_visit.visit_exit_idaction_name = log_action_visit_exit_idaction_name.idaction',
+            ),
+            array (
+                'table' => 'log_action',
+                'tableAlias' => 'log_action_idaction_name',
+                'joinOn' => 'log_link_visit_action.idaction_name = log_action_idaction_name.idaction',
+            ),
         );
 
         $this->assertEquals($expected, $tables);
     }
 
-    public function test_sortTablesForJoin_shouldSortTablesWithCustomJoinRequiringEachOther()
+    public function test_sortTablesForJoin_anotherTest2MakingSureWorksOhPhp5_5()
+    {
+        $tables = array (
+            1 => 'log_link_visit_action',
+            3 =>
+                array (
+                    'table' => 'log_action',
+                    'tableAlias' => 'log_action_visit_exit_idaction_name',
+                    'joinOn' => 'log_visit.visit_exit_idaction_name = log_action_visit_exit_idaction_name.idaction',
+                ),
+            2 =>
+                array (
+                    'table' => 'log_action',
+                    'tableAlias' => 'log_action_idaction_name',
+                    'joinOn' => 'log_link_visit_action.idaction_name = log_action_idaction_name.idaction',
+                ),
+        )
+        ;
+
+        $generator = $this->makeGenerator($tables);
+        usort($tables, array($generator, 'sortTablesForJoin'));
+
+        $expected = array(
+            'log_link_visit_action',
+            array (
+                'table' => 'log_action',
+                'tableAlias' => 'log_action_idaction_name',
+                'joinOn' => 'log_link_visit_action.idaction_name = log_action_idaction_name.idaction',
+            ),
+            array (
+                'table' => 'log_action',
+                'tableAlias' => 'log_action_visit_exit_idaction_name',
+                'joinOn' => 'log_visit.visit_exit_idaction_name = log_action_visit_exit_idaction_name.idaction',
+            ),
+        );
+
+        $this->assertEquals($expected, $tables);
+    }
+
+    public function test_sortTablesForJoin_shouldSortTablesWithCustomJoinRequiringEachOther1()
     {
         $tables = array(
             'log_link_visit_action',
@@ -256,6 +334,7 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
 
         $expected = array(
             'log_link_visit_action',
+            'log_action',
             array (
                 'table' => 'log_link_visit_action',
                 'tableAlias' => 'log_link_visit_action_foo',
@@ -266,7 +345,6 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
                 'tableAlias' => 'log_action_foo',
                 'joinOn' => 'log_link_visit_action_foo.idaction_url = log_action_foo.idaction',
             ),
-            'log_action',
         );
 
         $this->assertEquals($expected, $tables);
@@ -291,6 +369,27 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
         usort($tables, array($generator, 'sortTablesForJoin'));
 
         $this->assertEquals($expected, $tables);
+
+        // should still be the same if inverted
+        $tables = array(
+            'log_link_visit_action',
+            array(
+                'table' => 'log_action',
+                'tableAlias' => 'log_action_foo',
+                'joinOn' => "log_link_visit_action_foo.idaction_url = log_action_foo.idaction"
+            ),
+            'log_action',
+            array(
+                'table' => 'log_link_visit_action',
+                'tableAlias' => 'log_link_visit_action_foo',
+                'joinOn' => "log_link_visit_action.idvisit = log_link_visit_action_foo.idvisit"
+            ),
+        );
+
+        $generator = $this->makeGenerator($tables);
+        usort($tables, array($generator, 'sortTablesForJoin'));
+
+        $this->assertEquals($expected, $tables);
     }
 
     public function test_sortTablesForJoin_shouldSortTablesWithCustomJoinRequiringEachOther2()
@@ -313,16 +412,16 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase
         usort($tables, array($generator, 'sortTablesForJoin'));
 
         $expected = array(
-            array(
-                'table' => 'log_action',
-                'tableAlias' => 'log_action_visit_entry_idaction_name',
-                'joinOn' => "log_visit.visit_entry_idaction_name = log_action_visit_entry_idaction_name.idaction"
-            ),
             'log_link_visit_action',
             array(
                 'table' => 'log_action',
                 'tableAlias' => 'log_action_idaction_event_action',
                 'joinOn' => "log_link_visit_action.idaction_event_action = log_action_idaction_event_action.idaction"
+            ),
+            array(
+                'table' => 'log_action',
+                'tableAlias' => 'log_action_visit_entry_idaction_name',
+                'joinOn' => "log_visit.visit_entry_idaction_name = log_action_visit_entry_idaction_name.idaction"
             )
         );
 
diff --git a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php
index 98aeb339ee..ec1f7b49eb 100644
--- a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php
+++ b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinTablesTest.php
@@ -150,6 +150,17 @@ class JoinTablesTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($expected, $tables->getTables());
     }
 
+    public function test_sort_ifAllReturn0_ThenSortByGivenOrder()
+    {
+        $tables = $this->makeTables(array('log_conversion', 'log_visit', 'log_action', 'log_conversion_item'));
+        $tables->sort(function($a, $b) {
+            return 0;
+        });
+
+        $expected = array('log_conversion', 'log_visit', 'log_action', 'log_conversion_item');
+        $this->assertEquals($expected, $tables->getTables());
+    }
+
     private function makeTables($tables)
     {
         return new JoinTables(new LogTablesProvider(), $tables);