diff options
4 files changed, 84 insertions, 65 deletions
diff --git a/core/DataAccess/LogQueryBuilder/JoinGenerator.php b/core/DataAccess/LogQueryBuilder/JoinGenerator.php index b14bf7b2d7..7b69184f0b 100644 --- a/core/DataAccess/LogQueryBuilder/JoinGenerator.php +++ b/core/DataAccess/LogQueryBuilder/JoinGenerator.php @@ -235,12 +235,7 @@ class JoinGenerator return -1; } - $tA = $tA['table']; - $tB = $tB['table']; - - if ($tA === $tB) { - return 1; // if both join same table keep order - } + return 0; } if (is_array($tA)) { diff --git a/core/DataAccess/LogQueryBuilder/JoinTables.php b/core/DataAccess/LogQueryBuilder/JoinTables.php index d840cfba3f..1faad323a7 100644 --- a/core/DataAccess/LogQueryBuilder/JoinTables.php +++ b/core/DataAccess/LogQueryBuilder/JoinTables.php @@ -97,7 +97,17 @@ class JoinTables extends \ArrayObject // we need to make sure first table always comes first, only sort tables after the first table $firstTable = array_shift($tables); - usort($tables, $cmpFunction); + usort($tables, function ($ta, $tb) use ($tables, $cmpFunction) { + $return = call_user_func($cmpFunction, $ta, $tb); + if ($return === 0) { + $indexA = array_search($ta, $tables); + $indexB = array_search($tb, $tables); + + return $indexA - $indexB; + } + + return $return; + }); array_unshift($tables, $firstTable); $this->exchangeArray($tables); diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php index 9558cc0929..2023f652c6 100644 --- a/tests/PHPUnit/Integration/SegmentTest.php +++ b/tests/PHPUnit/Integration/SegmentTest.php @@ -426,6 +426,78 @@ class SegmentTest extends IntegrationTestCase $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query)); } + public function test_getSelectQuery_whenJoiningManyCustomTablesItShouldKeepTheOrderAsDefined() + { + $actionType = 3; + $idSite = 1; + $select = 'log_link_visit_action.custom_dimension_1, + 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 = array( + 'log_link_visit_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', + ), + array( + 'table' => 'log_action', + 'tableAlias' => 'log_action_foo', + 'joinOn' => 'log_link_visit_action_foo.idaction_url = log_action_foo.idaction', + ), + array( + 'table' => 'log_link_visit_action', + 'tableAlias' => 'log_link_visit_action_bar', + 'joinOn' => "log_link_visit_action.idvisit = log_link_visit_action_bar.idvisit" + ), + array( + 'table' => 'log_action', + 'tableAlias' => 'log_action_bar', + 'joinOn' => "log_link_visit_action_bar.idaction_url = log_action_bar.idaction" + ), + 'log_action', + ); + + $where = 'log_link_visit_action.server_time >= ? + AND log_link_visit_action.server_time <= ? + AND log_link_visit_action.idsite = ?'; + $bind = array('2015-11-30 11:00:00', '2015-12-01 10:59:59', $idSite); + + $segment = 'actionType==' . $actionType; + $segment = new Segment($segment, $idSites = array()); + + $query = $segment->getSelectQuery($select, $from, $where, $bind); + + $logActionTable = Common::prefixTable('log_action'); + $logLinkVisitActionTable = Common::prefixTable('log_link_visit_action'); + + $expected = array( + "sql" => " + SELECT log_link_visit_action.custom_dimension_1, + 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 $logActionTable AS log_action + ON log_link_visit_action.idaction_url = log_action.idaction + WHERE ( log_link_visit_action.server_time >= ? + AND log_link_visit_action.server_time <= ? + AND log_link_visit_action.idsite = ? ) + AND ( log_action.type = ? )", + "bind" => array('2015-11-30 11:00:00', '2015-12-01 10:59:59', $idSite, $actionType)); + + $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query)); + } + public function test_getSelectQuery_whenJoinLogLinkVisitActionOnActionOnVisit_WithSameTableAliasButDifferentJoin() { $actionType = 3; diff --git a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php index 45e356d13a..cb20726968 100644 --- a/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php +++ b/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php @@ -207,64 +207,6 @@ class JoinGeneratorTest extends \PHPUnit_Framework_TestCase $this->assertEquals($expected, $tables); } - public function test_sortTablesForJoin_shouldSortCorrecTWhenManyTableJoins() - { - $tables = array( - 'log_link_visit_action', - '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" - ), - array( - 'table' => 'log_action', - 'tableAlias' => 'log_action_foo', - 'joinOn' => "log_link_visit_action_foo.idaction_url = log_action_foo.idaction" - ), - array( - 'table' => 'log_link_visit_action', - 'tableAlias' => 'log_link_visit_action_bar', - 'joinOn' => "log_link_visit_action.idvisit = log_link_visit_action_bar.idvisit" - ), - array( - 'table' => 'log_action', - 'tableAlias' => 'log_action_bar', - 'joinOn' => "log_link_visit_action_bar.idaction_url = log_action_bar.idaction" - ) - ); - - $generator = $this->makeGenerator($tables); - usort($tables, array($generator, 'sortTablesForJoin')); - - $expected = array( - array( - 'table' => 'log_link_visit_action', - 'tableAlias' => 'log_link_visit_action_foo', - 'joinOn' => 'log_link_visit_action.idvisit = log_link_visit_action_foo.idvisit', - ), - array( - 'table' => 'log_link_visit_action', - 'tableAlias' => 'log_link_visit_action_bar', - 'joinOn' => "log_link_visit_action.idvisit = log_link_visit_action_bar.idvisit" - ), - array( - 'table' => 'log_action', - 'tableAlias' => 'log_action_foo', - 'joinOn' => 'log_link_visit_action_foo.idaction_url = log_action_foo.idaction', - ), - array( - 'table' => 'log_action', - 'tableAlias' => 'log_action_bar', - 'joinOn' => "log_link_visit_action_bar.idaction_url = log_action_bar.idaction" - ), - 'log_link_visit_action', - 'log_action', - ); - - $this->assertEquals($expected, $tables); - } - private function generate($tables) { $generator = $this->makeGenerator($tables); |