From e30de3346a74ea908ee1e9944dbc5686016e11a4 Mon Sep 17 00:00:00 2001
From: Thomas Steur <thomas.steur@gmail.com>
Date: Tue, 28 Jul 2015 15:30:38 +0000
Subject: [PATCH] A Row implementation that is based on ArrayObject which is
faster
---
core/DataTable/Filter/ColumnDelete.php | 13 +++++-
core/DataTable/Row.php | 60 ++++++++------------------
2 files changed, 30 insertions(+), 43 deletions(-)
diff --git a/core/DataTable/Filter/ColumnDelete.php b/core/DataTable/Filter/ColumnDelete.php
index a0fba5a963..60e53fd7dd 100644
--- a/core/DataTable/Filter/ColumnDelete.php
+++ b/core/DataTable/Filter/ColumnDelete.php
@@ -103,6 +103,10 @@ class ColumnDelete extends BaseFilter
if (!empty($this->columnsToRemove)) {
foreach ($table as $index => $row) {
foreach ($this->columnsToRemove as $column) {
+ if (!array_key_exists($column, $row)) {
+ continue;
+ }
+
if ($this->deleteIfZeroOnly) {
$value = $row[$column];
if ($value === false || !empty($value)) {
@@ -115,11 +119,13 @@ class ColumnDelete extends BaseFilter
}
$recurse = true;
+
}
// remove columns not specified in $columnsToKeep
if (!empty($this->columnsToKeep)) {
foreach ($table as $index => $row) {
+ $columnsToDelete = array();
foreach ($row as $name => $value) {
$keep = false;
// @see self::APPEND_TO_COLUMN_NAME_TO_KEEP
@@ -133,9 +139,14 @@ class ColumnDelete extends BaseFilter
&& $name != 'label' // label cannot be removed via whitelisting
&& !isset($this->columnsToKeep[$name])
) {
- unset($table[$index][$name]);
+ // we cannot remove row directly to prevent notice "ArrayIterator::next(): Array was modified
+ // outside object and internal position is no longer valid in /var/www..."
+ $columnsToDelete[] = $name;
}
}
+ foreach ($columnsToDelete as $columnToDelete) {
+ unset($table[$index][$columnToDelete]);
+ }
}
$recurse = true;
diff --git a/core/DataTable/Row.php b/core/DataTable/Row.php
index 973974033c..70ef662710 100644
--- a/core/DataTable/Row.php
+++ b/core/DataTable/Row.php
@@ -21,7 +21,7 @@ use Piwik\Metrics;
*
* @api
*/
-class Row implements \ArrayAccess, \IteratorAggregate
+class Row extends \ArrayObject
{
/**
* List of columns that cannot be summed. An associative array for speed.
@@ -36,7 +36,6 @@ class Row implements \ArrayAccess, \IteratorAggregate
// @see sumRow - implementation detail
public $maxVisitsSummed = 0;
- private $columns = array();
private $metadata = array();
private $isSubtableLoaded = false;
@@ -67,7 +66,7 @@ class Row implements \ArrayAccess, \IteratorAggregate
public function __construct($row = array())
{
if (isset($row[self::COLUMNS])) {
- $this->columns = $row[self::COLUMNS];
+ $this->exchangeArray($row[self::COLUMNS]);
}
if (isset($row[self::METADATA])) {
$this->metadata = $row[self::METADATA];
@@ -89,7 +88,7 @@ class Row implements \ArrayAccess, \IteratorAggregate
public function export()
{
return array(
- self::COLUMNS => $this->columns,
+ self::COLUMNS => $this->getArrayCopy(),
self::METADATA => $this->metadata,
self::DATATABLE_ASSOCIATED => $this->subtableId,
);
@@ -148,11 +147,11 @@ class Row implements \ArrayAccess, \IteratorAggregate
*/
public function deleteColumn($name)
{
- if (!array_key_exists($name, $this->columns)) {
+ if (!$this->offsetExists($name)) {
return false;
}
- unset($this->columns[$name]);
+ unset($this[$name]);
return true;
}
@@ -164,12 +163,14 @@ class Row implements \ArrayAccess, \IteratorAggregate
*/
public function renameColumn($oldName, $newName)
{
- if (isset($this->columns[$oldName])) {
- $this->columns[$newName] = $this->columns[$oldName];
+ if (isset($this[$oldName])) {
+ $this[$newName] = $this[$oldName];
}
// outside the if () since we want to delete nulled columns
- unset($this->columns[$oldName]);
+ if ($this->offsetExists($oldName)) {
+ unset($this[$oldName]);
+ }
}
/**
@@ -180,11 +181,11 @@ class Row implements \ArrayAccess, \IteratorAggregate
*/
public function getColumn($name)
{
- if (!isset($this->columns[$name])) {
+ if (!isset($this[$name])) {
return false;
}
- return $this->columns[$name];
+ return $this[$name];
}
/**
@@ -213,7 +214,7 @@ class Row implements \ArrayAccess, \IteratorAggregate
*/
public function hasColumn($name)
{
- return array_key_exists($name, $this->columns);
+ return $this->offsetExists($name);
}
/**
@@ -229,7 +230,7 @@ class Row implements \ArrayAccess, \IteratorAggregate
*/
public function getColumns()
{
- return $this->columns;
+ return $this->getArrayCopy();
}
/**
@@ -336,7 +337,7 @@ class Row implements \ArrayAccess, \IteratorAggregate
*/
public function setColumns($columns)
{
- $this->columns = $columns;
+ $this->exchangeArray($columns);
}
/**
@@ -347,7 +348,7 @@ class Row implements \ArrayAccess, \IteratorAggregate
*/
public function setColumn($name, $value)
{
- $this->columns[$name] = $value;
+ $this[$name] = $value;
}
/**
@@ -389,7 +390,7 @@ class Row implements \ArrayAccess, \IteratorAggregate
*/
public function addColumn($name, $value)
{
- if (isset($this->columns[$name])) {
+ if (isset($this[$name])) {
throw new Exception("Column $name already in the array!");
}
$this->setColumn($name, $value);
@@ -452,7 +453,7 @@ class Row implements \ArrayAccess, \IteratorAggregate
*/
public function sumRow(Row $rowToSum, $enableCopyMetadata = true, $aggregationOperations = false)
{
- foreach ($rowToSum->getColumns() as $columnToSumName => $columnToSumValue) {
+ foreach ($rowToSum as $columnToSumName => $columnToSumValue) {
if (!$this->isSummableColumn($columnToSumName)) {
continue;
}
@@ -667,31 +668,6 @@ class Row implements \ArrayAccess, \IteratorAggregate
return true;
}
- public function offsetExists($offset)
- {
- return $this->hasColumn($offset);
- }
-
- public function offsetGet($offset)
- {
- return $this->getColumn($offset);
- }
-
- public function offsetSet($offset, $value)
- {
- $this->setColumn($offset, $value);
- }
-
- public function offsetUnset($offset)
- {
- $this->deleteColumn($offset);
- }
-
- public function getIterator()
- {
- return new \ArrayIterator($this->columns);
- }
-
private function warnIfSubtableAlreadyExists()
{
if (!is_null($this->subtableId)) {
--
GitLab