regression bug in 1.29 for getHighestColumn in ods

[oprudkyi]

This is:

- [x] a bug report
- [ ] a feature request
- [x] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

getHighestColumn() should return correct value

What is the current behavior?

1.28.0 - 'C'
1.29.0 - 'BL'

What are the steps to reproduce?

Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue without relying on an external Excel file or a web server:

<?php

require 'vendor/autoload.php';

use PhpOffice\PhpSpreadsheet\IOFactory;

$filePath = 'test.ods';
$reader = IOFactory::createReaderForFile($filePath);
$reader->setReadDataOnly(false);

// disable warnings, because PHPSpreadsheet lib has error in own sources
try {
    $oldReporting = error_reporting(error_reporting() & ~E_NOTICE);
    $spreadsheet = $reader->load($filePath);
} finally {
    error_reporting($oldReporting);
}

$sheet = $spreadsheet->getSheetByName('sheet with data');
$highestColumn = $sheet->getHighestColumn(); // e.g 'F'
echo $highestColumn . PHP_EOL;

File used:
test.ods

What features do you think are causing the issue

• Reader
• Writer
• Styles
• Data Validations
• Formula Calculations
• Charts
• AutoFilter
• Form Elements

Does an issue affect all spreadsheet file formats? If not, which formats are affected?

ods

Which versions of PhpSpreadsheet and PHP are affected?

1.29.0
php: 8.2.8

[oleibman]

I can confirm the regression, but I don't receive any warning messages (or any other kinds) when I run this code without your try/finally block. Please let me know what message you are seeing.

You probably want to use getHighestDataColumn rather than getHighestColumn. That will give you your desired result.

Nevertheless, looking at your problem, the regression was probably caused by PR #3610. The ODS reader had been ignoring number-columns-repeated in the xml, leading to errors. In your file (and in many others that I've looked at), there is a declaration after the last populated cell in your rows:

<table:table-cell table:number-columns-repeated="61"/>

That is what is pushing you from 'C' to 'BL'. It seems to be an unnecessary declaration, and I am not sure what purpose it is serving for Ods. It might or might not be possible for me to identify this situation and ignore that declaration. I will look into it, but go with getHighestDataColumn anyhow.

[oprudkyi]

Hi @oleibman

Thank you very much.
getHighestDataColumn works correctly (I didn't know about existence of it )
as for try/catch etc - this is code from relatively legacy ETL and probably some random files did produce warnings which were regarded as exceptions. it is unrelated to the issue

[oprudkyi]

Hi @oleibman just found inconsistency for getHighestDataColumn,
if I run

<?php

require 'vendor/autoload.php';

use PhpOffice\PhpSpreadsheet\IOFactory;

$filePath = 'test.ods';
$reader = IOFactory::createReaderForFile($filePath);
$reader->setReadDataOnly(false);

$spreadsheet = $reader->load($filePath);

$sheet = $spreadsheet->getSheetByName('sheet with data');
foreach ($sheet->getRowIterator() as $row) {
    foreach ($row->getCellIterator() as $cell) {
    }
}
$highestColumn = $sheet->getHighestDataColumn(); // e.g 'F'
echo $highestColumn . PHP_EOL;

result is BL

but if I comment out getRowIterator block, like this

<?php

require 'vendor/autoload.php';

use PhpOffice\PhpSpreadsheet\IOFactory;

$filePath = 'test.ods';
$reader = IOFactory::createReaderForFile($filePath);
$reader->setReadDataOnly(false);

$spreadsheet = $reader->load($filePath);

$sheet = $spreadsheet->getSheetByName('sheet with data');
/*
foreach ($sheet->getRowIterator() as $row) {
    foreach ($row->getCellIterator() as $cell) {
    }
}
 */
$highestColumn = $sheet->getHighestDataColumn(); // e.g 'F'
echo $highestColumn . PHP_EOL;

the result is 'C'

i.e. look likes iterating resets HighestDataColumn to higher value

[oleibman]

Okay, reopening.

[oleibman]

foreach ($sheet->getRowIterator() as $row) {
    $iterator = $row->getCellIterator();
    $iterator->setIterateOnlyExistingCells(true);
    foreach ($iterator as $cell) {
    }
}

This should solve your problem. It seems a little awkward, and I think there's probably a good case to be made for adding a parameter for IterateOnlyExisting to the constructor. I'll look into that.

[oprudkyi]

Hi
Thank you.
as workaround I just call getHighestDataColumn before iterators
So it's low priority issue.