From b65aecdcec73092b9ac09740b175188b475c711a Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 18 Jun 2026 10:04:39 -0700 Subject: [PATCH 1/2] Readers Should Directly Access readFilter A number of interesting performance ideas have been submitted by @kemo. PR #4833 optimizes Xls Reader in a number of different ways. One of those, slightly modified, is to use the protected `readFilter` property which is available to all classes which inherit from `BaseReader` (which all our supported readers do), rather than call the `getReadFilter` method to obtain it (often in a loop). That applies to all readers, not just Xls, and is such a good idea that I am fast-tracking it ahead of 4833. Gnumeric, Ods, Xls, Xlsx, and Xml will all receive small performance boosts from it. Csv already does this, so no change is needed there. Html and Slk do not appear to do any filtering; possibly work for another day. --- src/PhpSpreadsheet/Reader/Gnumeric.php | 2 +- src/PhpSpreadsheet/Reader/Ods.php | 6 +++--- src/PhpSpreadsheet/Reader/Xls.php | 20 +++++++++---------- .../Reader/Xls/LoadSpreadsheet.php | 2 +- src/PhpSpreadsheet/Reader/Xlsx.php | 4 ++-- src/PhpSpreadsheet/Reader/Xml.php | 2 +- .../Reader/Xlsx/Issue4416Test.php | 3 +++ 7 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/PhpSpreadsheet/Reader/Gnumeric.php b/src/PhpSpreadsheet/Reader/Gnumeric.php index 85f8404bd..0e881b28d 100644 --- a/src/PhpSpreadsheet/Reader/Gnumeric.php +++ b/src/PhpSpreadsheet/Reader/Gnumeric.php @@ -311,7 +311,7 @@ class Gnumeric extends BaseReader $column = Coordinate::stringFromColumnIndex($column + 1); // Read cell? - if (!$this->getReadFilter()->readCell($column, $row, $worksheetName)) { + if (!$this->readFilter->readCell($column, $row, $worksheetName)) { continue; } diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index 818753a79..4ddbae090 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -851,7 +851,7 @@ class Ods extends BaseReader // repeated range passes the read filter. If not, skip the entire group. // If some columns pass, we need to fall through to the processing block // which will handle per-column filtering. - if (!$this->getReadFilter()->readCell($columnID, $rowID, $worksheetName)) { + if (!$this->readFilter->readCell($columnID, $rowID, $worksheetName)) { if ($colRepeats <= 1) { StringHelper::stringIncrement($columnID); @@ -865,7 +865,7 @@ class Ods extends BaseReader if ($i > 0) { StringHelper::stringIncrement($tempCol); } - if ($this->getReadFilter()->readCell($tempCol, $rowID, $worksheetName)) { + if ($this->readFilter->readCell($tempCol, $rowID, $worksheetName)) { $anyColumnPasses = true; break; @@ -1143,7 +1143,7 @@ class Ods extends BaseReader StringHelper::stringIncrement($columnID); } - if (!$this->getReadFilter()->readCell($columnID, $rowID, $worksheetName)) { + if (!$this->readFilter->readCell($columnID, $rowID, $worksheetName)) { continue; } diff --git a/src/PhpSpreadsheet/Reader/Xls.php b/src/PhpSpreadsheet/Reader/Xls.php index b7d34bc28..a79c66269 100644 --- a/src/PhpSpreadsheet/Reader/Xls.php +++ b/src/PhpSpreadsheet/Reader/Xls.php @@ -2728,7 +2728,7 @@ class Xls extends XlsBase $columnString = Coordinate::stringFromColumnIndex($column + 1); // Read cell? - if ($this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { // offset: 4; size: 2; index to XF record $xfIndex = self::getUInt2d($recordData, 4); @@ -2773,7 +2773,7 @@ class Xls extends XlsBase $cell = null; // Read cell? - if ($this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { // offset: 4; size: 2; index to XF record $xfIndex = self::getUInt2d($recordData, 4); @@ -2874,7 +2874,7 @@ class Xls extends XlsBase $columnString = Coordinate::stringFromColumnIndex($colFirst + $i); // Read cell? - if ($this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { // offset: var; size: 2; index to XF record $xfIndex = self::getUInt2d($recordData, $offset); @@ -2918,7 +2918,7 @@ class Xls extends XlsBase $columnString = Coordinate::stringFromColumnIndex($column + 1); // Read cell? - if ($this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { // offset 4; size: 2; index to XF record $xfIndex = self::getUInt2d($recordData, 4); @@ -2984,7 +2984,7 @@ class Xls extends XlsBase } // Read cell? - if ($this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { if ($isPartOfSharedFormula) { // formula is added to this cell after the sheet has been read $this->sharedFormulaParts[$columnString . ($row + 1)] = $this->baseCell; @@ -3150,7 +3150,7 @@ class Xls extends XlsBase $columnString = Coordinate::stringFromColumnIndex($column + 1); // Read cell? - if ($this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { // offset: 4; size: 2; index to XF record $xfIndex = self::getUInt2d($recordData, 4); @@ -3214,7 +3214,7 @@ class Xls extends XlsBase $columnString = Coordinate::stringFromColumnIndex($fc + $i + 1); // Read cell? - if ($this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { $xfIndex = self::getUInt2d($recordData, 4 + 2 * $i); if (isset($this->mapCellXfIndex[$xfIndex])) { $this->phpSheet->getCell($columnString . ($row + 1))->setXfIndex($this->mapCellXfIndex[$xfIndex]); @@ -3252,7 +3252,7 @@ class Xls extends XlsBase $columnString = Coordinate::stringFromColumnIndex($column + 1); // Read cell? - if ($this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { // offset: 4; size: 2; XF index $xfIndex = self::getUInt2d($recordData, 4); @@ -3297,7 +3297,7 @@ class Xls extends XlsBase $columnString = Coordinate::stringFromColumnIndex($col + 1); // Read cell? - if ($this->getReadFilter()->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($columnString, $row + 1, $this->phpSheet->getTitle())) { // offset: 4; size: 2; XF index $xfIndex = self::getUInt2d($recordData, 4); @@ -3605,7 +3605,7 @@ class Xls extends XlsBase StringHelper::stringIncrement($rangeBoundaries[1][0]); for ($row = $rangeBoundaries[0][1]; $row <= $rangeBoundaries[1][1]; ++$row) { for ($column = $rangeBoundaries[0][0]; $column != $rangeBoundaries[1][0]; StringHelper::stringIncrement($column)) { - if ($this->getReadFilter()->readCell($column, $row, $this->phpSheet->getTitle())) { + if ($this->readFilter->readCell($column, $row, $this->phpSheet->getTitle())) { $includeCellRange = true; break 2; diff --git a/src/PhpSpreadsheet/Reader/Xls/LoadSpreadsheet.php b/src/PhpSpreadsheet/Reader/Xls/LoadSpreadsheet.php index 402b9f59d..ebe9d515f 100644 --- a/src/PhpSpreadsheet/Reader/Xls/LoadSpreadsheet.php +++ b/src/PhpSpreadsheet/Reader/Xls/LoadSpreadsheet.php @@ -551,7 +551,7 @@ class LoadSpreadsheet extends Xls /** @var int $row */ [$column, $row] = Coordinate::coordinateFromString($cell); /** @var string $baseCell */ - if ($xls->getReadFilter()->readCell($column, $row, $xls->phpSheet->getTitle())) { + if ($xls->readFilter->readCell($column, $row, $xls->phpSheet->getTitle())) { /** @var string */ $temp = $xls->sharedFormulas[$baseCell]; $formula = $xls->getFormulaFromStructure($temp, $cell); diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php index 4aa55b999..a65628fcb 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx.php +++ b/src/PhpSpreadsheet/Reader/Xlsx.php @@ -890,7 +890,7 @@ class Xlsx extends BaseReader $sheetViewOptions->load($this->readDataOnly, $this->styleReader); (new ColumnAndRowAttributes($docSheet, $xmlSheetNS)) - ->load($this->getReadFilter(), $this->readDataOnly, $this->ignoreRowsWithNoCells); + ->load($this->readFilter, $this->readDataOnly, $this->ignoreRowsWithNoCells); } $holdSelectedCells = $docSheet->getSelectedCells(); @@ -1931,7 +1931,7 @@ class Xlsx extends BaseReader // Read cell? $coordinates = Coordinate::coordinateFromString($r); - if (!$this->getReadFilter()->readCell($coordinates[0], (int) $coordinates[1], $docSheet->getTitle())) { + if (!$this->readFilter->readCell($coordinates[0], (int) $coordinates[1], $docSheet->getTitle())) { // Normally, just testing for the f attribute should identify this cell as containing a formula // that we need to read, even though it is outside of the filter range, in case it is a shared formula. // But in some cases, this attribute isn't set; so we need to delve a level deeper and look at diff --git a/src/PhpSpreadsheet/Reader/Xml.php b/src/PhpSpreadsheet/Reader/Xml.php index 4fba7dcd7..f5cdc8108 100644 --- a/src/PhpSpreadsheet/Reader/Xml.php +++ b/src/PhpSpreadsheet/Reader/Xml.php @@ -432,7 +432,7 @@ class Xml extends BaseReader $arrayRef = AddressHelper::convertFormulaToA1($arrayRange, $rowID, $columnIndex); } - if (!$this->getReadFilter()->readCell($columnID, $rowID, $worksheetName)) { + if (!$this->readFilter->readCell($columnID, $rowID, $worksheetName)) { continue; } diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4416Test.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4416Test.php index 8a139b03c..2e5ae320a 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4416Test.php +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4416Test.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx; +use PhpOffice\PhpSpreadsheet\Reader\DefaultReadFilter; use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader; use PHPUnit\Framework\TestCase; @@ -49,7 +50,9 @@ class Issue4416Test extends TestCase { $file = self::$file; $reader = new XlsxReader(); + self::assertInstanceOf(DefaultReadFilter::class, $reader->getReadFilter()); $reader->setReadFilter(new Issue4416Filter()); + self::assertInstanceOf(Issue4416Filter::class, $reader->getReadFilter()); $spreadsheet = $reader->load($file); $sheet = $spreadsheet->getActiveSheet(); self::assertEqualsWithDelta( From 81379f430c457a00459214aa7e8b429ee94bb893 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 20 Jun 2026 20:57:17 -0700 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 793174876..e003679ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). Thia is a - Navigate Cell as Cursor. [Issue #863](https://github.com/PHPOffice/PhpSpreadsheet/issues/863) [PR #4909](https://github.com/PHPOffice/PhpSpreadsheet/pull/4909) - Chart DataTables. [Issue #413](https://github.com/PHPOffice/PhpSpreadsheet/issues/413) [PR #4911](https://github.com/PHPOffice/PhpSpreadsheet/pull/4911) +- Permit Ignoring "Misleading Format" Tag. [PR #4914](https://github.com/PHPOffice/PhpSpreadsheet/pull/4914) ### Removed @@ -33,6 +34,8 @@ and this project adheres to [Semantic Versioning](https://semver.org). Thia is a - Small improvement for Radar Charts. [Issue #661](https://github.com/PHPOffice/PhpSpreadsheet/issues/661) [PR #4908](https://github.com/PHPOffice/PhpSpreadsheet/pull/4908) - Allow Use of Multiple Ranges When Setting Styles. [Issue #411](https://github.com/PHPOffice/PhpSpreadsheet/issues/411) [PR #4910](https://github.com/PHPOffice/PhpSpreadsheet/pull/4910) - Small change to Writer Html. [Issue #434](https://github.com/PHPOffice/PhpSpreadsheet/issues/434) [PR #4912](https://github.com/PHPOffice/PhpSpreadsheet/pull/4912) +- Avoid hard-coding some chart Xml attributes. [PR #4915](https://github.com/PHPOffice/PhpSpreadsheet/pull/4915) +- Readers should directly access readFilter. [PR #4919](https://github.com/PHPOffice/PhpSpreadsheet/pull/4919) ## 2026-06-06 - 5.8.0