diff --git a/core/Plugin/Segment.php b/core/Plugin/Segment.php
index 8553703b08f8bcd86b222a9374345aa6def1613e..b9164898a8b44e22429956412dc415a58033c70b 100644
--- a/core/Plugin/Segment.php
+++ b/core/Plugin/Segment.php
@@ -124,6 +124,8 @@ class Segment
* or callable: `string $valueToMatch`, `string $segment` (see {@link setSegment()}), `string $matchType`
* (eg SegmentExpression::MATCH_EQUAL or any other match constant of this class) and `$segmentName`.
*
+ * If the closure returns NULL, then Piwik assumes the segment sub-string will not match any visitor.
+ *
* @param string|\Closure $sqlFilter
* @api
*/
diff --git a/core/Segment.php b/core/Segment.php
index 511d4fca10832263e2d679033b616578bbd4f140..54e93f82afd53d1a9fe072a321d14417ef669081 100644
--- a/core/Segment.php
+++ b/core/Segment.php
@@ -179,12 +179,19 @@ class Segment
if (isset($segment['sqlFilter'])) {
$value = call_user_func($segment['sqlFilter'], $value, $segment['sqlSegment'], $matchType, $name);
+ if(is_null($value)) {
+ // eg. see TableLogAction::getIdActionFromSegment() defined
+ return array();
+ }
+
// sqlFilter-callbacks might return arrays for more complex cases
// e.g. see TableLogAction::getIdActionFromSegment()
if (is_array($value) && isset($value['SQL'])) {
// Special case: returned value is a sub sql expression!
$matchType = SegmentExpression::MATCH_ACTIONS_CONTAINS;
}
+
+
}
}
break;
diff --git a/core/Segment/SegmentExpression.php b/core/Segment/SegmentExpression.php
index 3fcf232a142f0dc53b68181a398b988d28690641..07b535f8c7860fb0e48cc96e09a40483a74506c4 100644
--- a/core/Segment/SegmentExpression.php
+++ b/core/Segment/SegmentExpression.php
@@ -40,6 +40,8 @@ class SegmentExpression
const INDEX_BOOL_OPERATOR = 0;
const INDEX_OPERAND = 1;
+ const SQL_WHERE_DO_NOT_MATCH_ANY_ROW = "(1 = 0)";
+
public function __construct($string)
{
$this->string = $string;
@@ -138,13 +140,21 @@ class SegmentExpression
$operator = $leaf[self::INDEX_BOOL_OPERATOR];
$operandDefinition = $leaf[self::INDEX_OPERAND];
- $operand = $this->getSqlMatchFromDefinition($operandDefinition, $availableTables);
- if ($operand[1] !== null) {
- $this->valuesBind[] = $operand[1];
+ // in case we know already the segment won't match any row...
+ if($operandDefinition === array() ) { // see getCleanedExpression()
+ $operand = self::SQL_WHERE_DO_NOT_MATCH_ANY_ROW;
+
+ } else {
+ $operand = $this->getSqlMatchFromDefinition($operandDefinition, $availableTables);
+
+ if ($operand[1] !== null) {
+ $this->valuesBind[] = $operand[1];
+ }
+
+ $operand = $operand[0];
}
- $operand = $operand[0];
$sqlSubExpressions[] = array(
self::INDEX_BOOL_OPERATOR => $operator,
self::INDEX_OPERAND => $operand,
@@ -381,3 +391,4 @@ class SegmentExpression
);
}
}
+
diff --git a/core/Tracker/TableLogAction.php b/core/Tracker/TableLogAction.php
index 21350aedb56a4ff0b9d6a2a99d08f97b293bae40..d78240cb2587b3ee789d162a67ac919e89282cb9 100644
--- a/core/Tracker/TableLogAction.php
+++ b/core/Tracker/TableLogAction.php
@@ -177,10 +177,9 @@ class TableLogAction
|| $matchType == SegmentExpression::MATCH_NOT_EQUAL
) {
$idAction = self::getModel()->getIdActionMatchingNameAndType($valueToMatch, $actionType);
- // if the action is not found, we hack -100 to ensure it tries to match against an integer
- // otherwise binding idaction_name to "false" returns some rows for some reasons (in case &segment=pageTitle==Větrnásssssss)
+ // Action is not found (eg. &segment=pageTitle==Větrnásssssss)
if (empty($idAction)) {
- $idAction = -100;
+ $idAction = null;
}
return $idAction;
}
diff --git a/tests/PHPUnit/Integration/SegmentTest.php b/tests/PHPUnit/Integration/SegmentTest.php
index e5b6bae199f00cbcb7a81ef4ab50a71d1c8452b8..0940766011d4cb9149df0ea1fd134494d77a5f4e 100644
--- a/tests/PHPUnit/Integration/SegmentTest.php
+++ b/tests/PHPUnit/Integration/SegmentTest.php
@@ -505,20 +505,15 @@ class SegmentTest extends IntegrationTestCase
public function test_getSelectQuery_whenPageUrlExists_asStatementAND()
{
+ $pageUrlFoundInDb = 'example.com/page.html?hello=world';
+
+ $actionIdFoundInDb = $this->insertPageUrlAsAction($pageUrlFoundInDb);
+
$select = 'log_visit.*';
$from = 'log_visit';
$where = false;
$bind = array();
- $pageUrlFoundInDb = 'example.com/page.html?hello=world';
-
- TableLogAction::loadIdsAction( array(
- 'idaction_url' => array($pageUrlFoundInDb, Action::TYPE_PAGE_URL)
- ));
-
- $actionIdFoundInDb = Db::fetchOne("SELECT idaction from " . Common::prefixTable('log_action') . " WHERE name = ?", $pageUrlFoundInDb);
- $this->assertNotEmpty( $actionIdFoundInDb, "Action $pageUrlFoundInDb was not found in the ". Common::prefixTable('log_action') ." table.");
-
$segment = 'visitServerHour==3;pageUrl==' . urlencode($pageUrlFoundInDb);
$segment = new Segment($segment, $idSites = array());
@@ -559,21 +554,13 @@ class SegmentTest extends IntegrationTestCase
$expected = array(
"sql" => "
- SELECT
- log_inner.*
- FROM
- (
SELECT
log_visit.*
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
WHERE HOUR(log_visit.visit_last_action_time) = ?
- AND log_link_visit_action.idaction_url = ?
- GROUP BY log_visit.idvisit
- ORDER BY NULL
- ) AS log_inner",
- "bind" => array(12, -100));
+ AND (1 = 0) ",
+ "bind" => array(12));
$this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
}
@@ -590,6 +577,34 @@ class SegmentTest extends IntegrationTestCase
$query = $segment->getSelectQuery($select, $from, $where, $bind);
+ $expected = array(
+ "sql" => "
+ SELECT
+ log_visit.*
+ FROM
+ " . Common::prefixTable('log_visit') . " AS log_visit
+ WHERE (HOUR(log_visit.visit_last_action_time) = ?
+ OR (1 = 0) )",
+ "bind" => array(12));
+
+ $this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
+ }
+
+ public function test_getSelectQuery_whenPageUrlDoesNotExist_asBothStatements_OR_AND()
+ {
+ $pageUrlFoundInDb = 'example.com/found-in-db';
+ $actionIdFoundInDb = $this->insertPageUrlAsAction($pageUrlFoundInDb);
+
+ $select = 'log_visit.*';
+ $from = 'log_visit';
+ $where = false;
+ $bind = array();
+
+ $segment = 'visitServerHour==12,pageUrl==xyz;pageUrl==abcdefg,pageUrl=='.urlencode($pageUrlFoundInDb);
+ $segment = new Segment($segment, $idSites = array());
+
+ $query = $segment->getSelectQuery($select, $from, $where, $bind);
+
$expected = array(
"sql" => "
SELECT
@@ -602,11 +617,13 @@ class SegmentTest extends IntegrationTestCase
" . 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
WHERE (HOUR(log_visit.visit_last_action_time) = ?
- OR log_link_visit_action.idaction_url = ? )
+ OR (1 = 0))
+ AND ((1 = 0)
+ OR log_link_visit_action.idaction_url = ? )
GROUP BY log_visit.idvisit
ORDER BY NULL
) AS log_inner",
- "bind" => array(12, -100));
+ "bind" => array(12, $actionIdFoundInDb));
$this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
}
@@ -617,4 +634,20 @@ class SegmentTest extends IntegrationTestCase
'Piwik\Access' => new FakeAccess()
);
}
+
+ /**
+ * @param $pageUrlFoundInDb
+ * @return string
+ * @throws Exception
+ */
+ private function insertPageUrlAsAction($pageUrlFoundInDb)
+ {
+ TableLogAction::loadIdsAction(array(
+ 'idaction_url' => array($pageUrlFoundInDb, Action::TYPE_PAGE_URL)
+ ));
+
+ $actionIdFoundInDb = Db::fetchOne("SELECT idaction from " . Common::prefixTable('log_action') . " WHERE name = ?", $pageUrlFoundInDb);
+ $this->assertNotEmpty($actionIdFoundInDb, "Action $pageUrlFoundInDb was not found in the " . Common::prefixTable('log_action') . " table.");
+ return $actionIdFoundInDb;
+ }
}