From 7fbea546f7f23ff438c0f9c4c2db0c77a367aed8 Mon Sep 17 00:00:00 2001
From: Thomas Steur <thomas.steur@gmail.com>
Date: Wed, 2 Dec 2015 01:37:07 +0000
Subject: [PATCH] Revert "reuse existing joins when possible"
This reverts commit 745c6c6d38993f35a059f02b2877dff42141c1b2.
---
core/DataAccess/LogQueryBuilder.php | 74 ++++++-----------------
tests/PHPUnit/Integration/SegmentTest.php | 49 +--------------
2 files changed, 18 insertions(+), 105 deletions(-)
diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php
index 31691c0184..2ba44f27df 100644
--- a/core/DataAccess/LogQueryBuilder.php
+++ b/core/DataAccess/LogQueryBuilder.php
@@ -44,50 +44,6 @@ class LogQueryBuilder
);
}
- private function getIndexIfTableInTables($tableToFind, $tables)
- {
- $index = array_search($tableToFind, $tables);
-
- if (false !== $index) {
- return $index;
- }
-
- foreach ($tables as $index => $table) {
- if (is_array($table)
- && !empty($table['table'])
- && $table['table'] === $tableToFind
- && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableToFind)) {
- return $index;
- }
- }
-
- return false;
- }
-
- private function swapPositionOfTableEntries($tables, $index1, $index2)
- {
- $table1 = $tables[$index1];
- $table2 = $tables[$index2];
-
- $tables[$index1] = $table2;
- $tables[$index2] = $table1;
-
- return $tables;
- }
-
- private function hasJoinedTableAlreadyManually($tableToFind, $tables)
- {
- foreach ($tables as $index => $table) {
- if (is_array($table)
- && !empty($table['table'])
- && $table['table'] === $tableToFind
- && (!isset($table['tableAlias']) || $table['tableAlias'] === $tableToFind)) {
- return true;
- }
- }
-
- return false;
- }
/**
* Generate the join sql based on the needed tables
@@ -97,7 +53,7 @@ class LogQueryBuilder
*/
private function generateJoinsString(&$tables)
{
- $knownTables = array('log_action', 'log_visit', 'log_link_visit_action', 'log_conversion', 'log_conversion_item');
+ $knownTables = array("log_action", "log_visit", "log_link_visit_action", "log_conversion", "log_conversion_item");
$visitsAvailable = $linkVisitActionsTableAvailable = $conversionsAvailable = $conversionItemAvailable = $actionsTableAvailable = false;
$joinWithSubSelect = false;
$sql = '';
@@ -105,30 +61,33 @@ class LogQueryBuilder
// 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 = $this->getIndexIfTableInTables("log_link_visit_action", $tables);
- $conversionIndex = $this->getIndexIfTableInTables("log_conversion", $tables);
+ $actionIndex = array_search("log_link_visit_action", $tables);
+ $conversionIndex = array_search("log_conversion", $tables);
if ($actionIndex > 0 && $conversionIndex > 0 && $actionIndex > $conversionIndex) {
- $tables = $this->swapPositionOfTableEntries($tables, $actionIndex, $conversionIndex);
+ $tables[$actionIndex] = "log_conversion";
+ $tables[$conversionIndex] = "log_link_visit_action";
}
// same as above: action before visit
- $actionIndex = $this->getIndexIfTableInTables("log_link_visit_action", $tables);
- $visitIndex = $this->getIndexIfTableInTables("log_visit", $tables);
+ $actionIndex = array_search("log_link_visit_action", $tables);
+ $visitIndex = array_search("log_visit", $tables);
if ($actionIndex > 0 && $visitIndex > 0 && $actionIndex > $visitIndex) {
- $tables = $this->swapPositionOfTableEntries($tables, $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 = $this->getIndexIfTableInTables("log_link_visit_action", $tables);
- $actionIndex = $this->getIndexIfTableInTables("log_action", $tables);
+ $linkVisitAction = array_search("log_link_visit_action", $tables);
+ $actionIndex = array_search("log_action", $tables);
if ($linkVisitAction === false && $actionIndex > 0) {
$tables[] = "log_link_visit_action";
}
- $linkVisitAction = $this->getIndexIfTableInTables("log_link_visit_action", $tables);
- $actionIndex = $this->getIndexIfTableInTables("log_action", $tables);
+ $linkVisitAction = array_search("log_link_visit_action", $tables);
+ $actionIndex = array_search("log_action", $tables);
if ($linkVisitAction > 0 && $actionIndex > 0 && $linkVisitAction > $actionIndex) {
- $tables = $this->swapPositionOfTableEntries($tables, $linkVisitAction, $actionIndex);
+ $tables[$actionIndex] = "log_link_visit_action";
+ $tables[$linkVisitAction] = "log_action";
}
foreach ($tables as $i => $table) {
@@ -150,7 +109,8 @@ class LogQueryBuilder
if ($i == 0) {
// first table
$sql .= $tableSql;
- } elseif (!$this->hasJoinedTableAlreadyManually($table, $tables)) {
+ } else {
+
if ($linkVisitActionsTableAvailable && $table === 'log_action') {
$join = "log_link_visit_action.idaction_url = log_action.idaction";
} elseif ($linkVisitActionsTableAvailable && $table == "log_conversion") {
diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index 73324b088b..80c17ebe63 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -486,7 +486,7 @@ class SegmentTest extends IntegrationTestCase
$this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
}
- public function test_getSelectQuery_whenJoinLogLinkVisitActionOnActionOnVisit_WithDifferentTableAlias()
+ public function test_getSelectQuery_whenJoinLogLinkVisitActionOnActionOnVisit()
{
$actionType = 3;
$idSite = 1;
@@ -535,53 +535,6 @@ class SegmentTest extends IntegrationTestCase
$this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
}
- public function test_getSelectQuery_whenJoinLogLinkVisitActionOnActionOnVisit_WithSameTableAlias()
- {
- $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_visit', 'joinOn' => 'log_visit.idvisit = log_link_visit_action.idvisit'),
- array('table' => 'log_action', 'joinOn' => '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 = ?';
- $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);
-
- $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,
- 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_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_whenJoinLogLinkVisitActionOnAction()
{
$actionType = 3;
--
GitLab