Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect evaluating Named Ranges #2826

Closed
2 of 8 tasks
kusnir opened this issue May 12, 2022 · 2 comments
Closed
2 of 8 tasks

Incorrect evaluating Named Ranges #2826

kusnir opened this issue May 12, 2022 · 2 comments

Comments

@kusnir
Copy link

kusnir commented May 12, 2022

This is:

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

What is the expected behavior?

Return correct value from range defined by Named Range.

From test:

  • B7 should by 6
  • B8 should by 2
  • B9 should by 2

What is the current behavior?

Return 0 or #N/A value

From test:

  • B7 is 0
  • B8 is empty
  • B8 is #N/A

What are the steps to reproduce?

Create a Xls file:

  1. Create new file
  2. Fill data:
  • A1: TEST1
  • A2: TEST2
  • A3: TEST3
  • A5: TEST2
  • B1: 3
  • B2: 2
  • B3: 1
  1. Create Named Range
  • NAMED_RANGE: =Sheet1!$A$1:$B$3
  • NAMED_VALUES_RANGE: =Sheet1!$B$1:$B$3
  1. Fill data:
  • A7: =SUM(B1:B3)
  • B7: =SUM(NAMED_VALUES_RANGE)
  • A8: =VLOOKUP(A2;A1:B3;2;FALSE)
  • B8: =VLOOKUP(A2;NAMED_RANGE;2;FALSE)
  • A9: =VLOOKUP("TEST2";A1:B3;2;FALSE)
  • B9: =VLOOKUP("TEST2";NAMED_RANGE;2;FALSE)
  1. Save as test.xls
<?php
chdir(__DIR__);
set_include_path(__DIR__);
require_once 'vendor/autoload.php';

$composerData = json_decode(file_get_contents(__DIR__ . '/composer.lock'), true);
$phpspreadsheet = array_values(array_filter($composerData['packages'], function ($value, $key) {
    return $value['name'] == 'phpoffice/phpspreadsheet';
}, ARRAY_FILTER_USE_BOTH))[0];
echo '<h2>PHP ' . phpversion() . ', PHPOffice / PhpSpreadsheet ' . $phpspreadsheet['version'] . '</h2>';

$filePath = __DIR__ . '/test.xls';
echo '<strong>' . basename($filePath) . '</strong>';

$inputFileType = 'Xls';
$objReader = \PhpOffice\PhpSpreadsheet\IOFactory::createReader($inputFileType);
$objReader->setReadDataOnly(true);
$objPHPExcel = $objReader->load($filePath);
$sheets = $objReader->listWorksheetNames($filePath);
foreach ($sheets as $sheet_index => $sheet_name) {
    // Get worksheet dimensions
    $sheet = $objPHPExcel->getSheet($sheet_index);
    $highestRow = $sheet->getHighestRow();
    $highestColumn = $sheet->getHighestColumn();

    if ($highestRow > 1) {
        // Loop through each row of the worksheet in turn
        echo '<h2>' . $sheet_name . '</h2>';
        echo '<table border="1">';
        for ($row = 1; $row <= $highestRow; $row ++) {
            if ($row == 1) {
                echo '<tr>';
                echo '<th></th>';
                $highestColumnIndex = array_values(\PhpOffice\PhpSpreadsheet\Cell\Coordinate::indexesFromString($highestColumn . '1'))[0];
                for ($index = 1; $index <= $highestColumnIndex; $index++) {
                    echo '<th>' . \PhpOffice\PhpSpreadsheet\Cell\Coordinate::stringFromColumnIndex($index) . '</th>';
                }
                echo '</tr>';
            }

            // Read a row of data into an array
            $formulas = $sheet->rangeToArray('A' . $row . ':' . $highestColumn . $row, NULL, false, true);
            $values = $sheet->rangeToArray('A' . $row . ':' . $highestColumn . $row, NULL, true, true);

            echo '<tr>';
            echo '<th>' . $row . '</th>';
            if ($values) {
                foreach (array_keys($values[0]) as $k) {
                    //$values[0][$k] = str_replace("\n", "; ", $values[0][$k]);
                    echo '<td>';
                    echo '<small style="color: #999;">' . $formulas[0][$k] . '</small><br/>';
                    echo $values[0][$k];
                    echo '</td>';
                }
            }
            echo '</tr>';
        }
        echo '</table>';
    }
}

echo '<h2>Named Ranges</h2>';
foreach ($objPHPExcel->getNamedRanges() as $range) {
    /* @var \PhpOffice\PhpSpreadsheet\NamedRange $range */
    echo $range->getName() . ': ' . $range->getRange() . '<br>';

}

Output HTML:

PHP 7.4.27, PHPOffice / PhpSpreadsheet 1.23.0

test.xls

Sheet1

AB
1TEST1
TEST1
3
3
2TEST2
TEST2
2
2
3TEST3
TEST3
1
1
4

5TEST2
TEST2

6

7=SUM(B1:B3)
6
=SUM(NAMED_VALUES_RANGE)
0
8=VLOOKUP(A5,A1:B3,2,FALSE)
2
=VLOOKUP(A5,NAMED_RANGE,2,FALSE)
9=VLOOKUP("TEST2",A1:B3,2,FALSE)
2
=VLOOKUP("TEST2",NAMED_RANGE,2,FALSE)
#N/A

Named Ranges

NAMED_RANGE: A1:B3
NAMED_VALUES_RANGE: B1:B3

What features do you think are causing the issue

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

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

xls

Which versions of PhpSpreadsheet and PHP are affected?

PhpSpreadsheet Ver : V1.23.0
PHP Ver: 7.4.27

@MarkBaker
Copy link
Member

Thanks for raising this issue, and for your detailed example making it very easy to verify, and to recreate the problem.

In version 1.15.0 we fixed an issue with named ranges in formula that was always treating cell references as absolute references, even when they were relative references (Excel adjusts the range when relative references are used in named ranges in formulae). The Calculation Engine processes these adjustments correctly; but it is important that named ranges are correctly defined as relative or absolute.

The problem here is that the named ranges are defined as absolute, but that the Xls Reader is converting them to relative ranges. Before 1.15.0 this didn't matter, because the Calculation Engine treated all named ranges as absolute; but since that fix to the Calculation Engine it does make a difference.

The solution is to prevent the Xls Reader from converting absolute ranges to relative ranges, but to treat them exactly as they are defined.

@MarkBaker
Copy link
Member

MarkBaker commented May 12, 2022

P.S. Nice use of reading the composer.lock file to extract the PhpSpreadsheet versions; I'll start using that technique myself

MarkBaker added a commit that referenced this issue Jul 9, 2022
Note that this will be the last 1.x branch release before the 2.x release. We will maintain both branches in parallel for a time; but users are requested to update to version 2.0 once that is fully available.

### Added

- Added `removeComment()` method for Worksheet [PR #2875](https://github.com/PHPOffice/PhpSpreadsheet/pull/2875/files)
- Add point size option for scatter charts [Issue #2298](#2298) [PR #2801](#2801)
- Basic support for Xlsx reading/writing Chart Sheets [PR #2830](#2830)

  Note that a ChartSheet is still only written as a normal Worksheet containing a single chart, not as an actual ChartSheet.

- Added Worksheet visibility in Ods Reader [PR #2851](#2851) and Gnumeric Reader [PR #2853](#2853)
- Added Worksheet visibility in Ods Writer [PR #2850](#2850)
- Allow Csv Reader to treat string as contents of file [Issue #1285](#1285) [PR #2792](#2792)
- Allow Csv Reader to store null string rather than leave cell empty [Issue #2840](#2840) [PR #2842](#2842)
- Provide new Worksheet methods to identify if a row or column is "empty", making allowance for different definitions of "empty":
  - Treat rows/columns containing no cell records as empty (default)
  - Treat cells containing a null value as empty
  - Treat cells containing an empty string as empty

### Changed

- Modify `rangeBoundaries()`, `rangeDimension()` and `getRangeBoundaries()` Coordinate methods to work with row/column ranges as well as with cell ranges and cells [PR #2926](#2926)
- Better enforcement of value modification to match specified datatype when using `setValueExplicit()`
- Relax validation of merge cells to allow merge for a single cell reference [Issue #2776](#2776)
- Memory and speed improvements, particularly for the Cell Collection, and the Writers.

  See [the Discussion section on github](#2821) for details of performance across versions
- Improved performance for removing rows/columns from a worksheet

### Deprecated

- Nothing

### Removed

- Nothing

### Fixed

- Xls Reader resolving absolute named ranges to relative ranges [Issue #2826](#2826) [PR #2827](#2827)
- Null value handling in the Excel Math/Trig PRODUCT() function [Issue #2833](#2833) [PR #2834](#2834)
- Invalid Print Area defined in Xlsx corrupts internal storage of print area [Issue #2848](#2848) [PR #2849](#2849)
- Time interval formatting [Issue #2768](#2768) [PR #2772](#2772)
- Copy from Xls(x) to Html/Pdf loses drawings [PR #2788](#2788)
- Html Reader converting cell containing 0 to null string [Issue #2810](#2810) [PR #2813](#2813)
- Many fixes for Charts, especially, but not limited to, Scatter, Bubble, and Surface charts. [Issue #2762](#2762) [Issue #2299](#2299) [Issue #2700](#2700) [Issue #2817](#2817) [Issue #2763](#2763) [Issue #2219](#2219) [Issue #2863](#2863) [PR #2828](#2828) [PR #2841](#2841) [PR #2846](#2846) [PR #2852](#2852) [PR #2856](#2856) [PR #2865](#2865) [PR #2872](#2872) [PR #2879](#2879) [PR #2898](#2898) [PR #2906](#2906) [PR #2922](#2922) [PR #2923](#2923)
- Adjust both coordinates for two-cell anchors when rows/columns are added/deleted. [Issue #2908](#2908) [PR #2909](#2909)
- Keep calculated string results below 32K. [PR #2921](#2921)
- Filter out illegal Unicode char values FFFE/FFFF. [Issue #2897](#2897) [PR #2910](#2910)
- Better handling of REF errors and propagation of all errors in Calculation engine. [PR #2902](#2902)
- Calculating Engine regexp for Column/Row references when there are multiple quoted worksheet references in the formula [Issue #2874](#2874) [PR #2899](#2899)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants