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

Error with the Calculation of VLOOKUP cells #2934

Closed
1 of 8 tasks
c-louis opened this issue Jul 12, 2022 · 2 comments · Fixed by #2939
Closed
1 of 8 tasks

Error with the Calculation of VLOOKUP cells #2934

c-louis opened this issue Jul 12, 2022 · 2 comments · Fixed by #2939

Comments

@c-louis
Copy link

c-louis commented Jul 12, 2022

Hello 😄

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?

VLOOKUP should find the cell and retrieve the value

What is the current behavior?

TypeError because of strToLower which happens because of an error of the evaluation of the formula

PHP Fatal error: Uncaught TypeError: PhpOffice\PhpSpreadsheet\Shared\StringHelper::strToLower(): Argument #1 ($textValue) must be of type string

What are the steps to reproduce?

<?php

require __DIR__ . '/vendor/autoload.php';

use \PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx;

$doc = "doc2.xlsx";

$spreadsheet = \PhpOffice\PhpSpreadsheet\IOFactory::load($doc);
$sheet = $spreadsheet->getActiveSheet();

evaluate($spreadsheet, $sheet, "C2");

function evaluate( $spreadSheet,  $workSheet,  $cellAddress)
{
    // Initialise the calculation engine for debug logging
    $calculationEngine = Calculation::getInstance($spreadSheet);
//    $calculationEngine->disableBranchPruning();
    $debugLog = $calculationEngine->getDebugLog();

    $calculationEngine->flushInstance();
    $debugLog->setWriteDebugLog(true);
//    $debugLog->setEchoDebugLog(true);

    $formulaValue = $workSheet->getCell($cellAddress)->getValue();

    echo PHP_EOL, 'Formula value for evaluation is ', $formulaValue, PHP_EOL;

    $canExecuteCalculation = false;

    try {
        $tokens = $calculationEngine->parseFormula($formulaValue);
        $canExecuteCalculation = true;
    } catch (Exception $e) {
        echo 'PARSER ERROR: ', $e->getMessage(), PHP_EOL;
    } finally {
        echo 'Parser Stack :-';
        print_r($tokens ?? PHP_EOL . 'NULL');
    }

    $callStartTime = microtime(true);

    if ($canExecuteCalculation) {
        //  calculate
        try {
            $cellValue = $workSheet->getCell($cellAddress)
                ->getCalculatedValue();

            echo 'Result is ', $cellValue, PHP_EOL;

            echo 'Evaluation Log:', PHP_EOL;
            print_r($debugLog->getLog());
        } catch (Exception $e) {
            echo 'CALCULATION ENGINE ERROR: ', $e->getMessage(), PHP_EOL;

            echo 'Evaluation Log:', PHP_EOL;
            print_r($debugLog->getLog());
        }
    }

    $callEndTime = microtime(true);
    $callTime = $callEndTime - $callStartTime;

    echo PHP_EOL;
    echo 'Call time to evaluate formula was ', sprintf('%.4f', $callTime), ' seconds', PHP_EOL;

    return $cellValue ?? null;
}

Here is the file I use to test (It's a minimal version of it, showing the bug) :

doc2.xlsx

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?

xlsx

Which versions of PhpSpreadsheet and PHP are affected?

1.24.0

What I did try to investigate :

If you remove the "FALSE" of =VLOOKUP(B2;Sheet2!B:C;2;FALSE) then it doesnt bug anymore.

@Moongun
Copy link

Moongun commented Jul 13, 2022

Hey. Have the same issue. Part of exception message below :):
"PhpOffice\\PhpSpreadsheet\\Shared\\StringHelper::strToLower(): Argument #1 ($textValue) must be of type string, null given, called in \/.\/vendor\/phpoffice\/phpspreadsheet\/src\/PhpSpreadsheet\/Calculation\/LookupRef\/VLookup.php on line 72"

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Jul 13, 2022
Fix PHPOffice#2934. Null is passed to StringHelper::strtolower which expects string. Same problem appears to be applicable to HLOOKUP.

I noted the following problem in the code, but will document it here as well. Excel's results are not consistent when a non-numeric string is passed as the third parameter. For example, if cell Z1 contains `xyz`, Excel will return a REF error for function `VLOOKUP(whatever,whatever,Z1)`, but it returns a VALUE error for function `VLOOKUP(whatever,whatever,"xyz")`. I don't think PhpSpreadsheet can match both behaviors. For now, it will return VALUE for both, with similar results for other errors.

While studying the returned errors, I realized there is something that needs to be deprecated. `ExcelError::$errorCodes` is a public static array. This means that a user can change its value, which should not be allowed. It is replaced by a constant. Since the original is public, I think it needs to stay, but with a deprecation notice; users can reference and change it, but it will be unused in the rest of the code. I suppose this might be considered a break in functionality (that should not have been allowed in the first place).
oleibman added a commit that referenced this issue Jul 17, 2022
Fix #2934. Null is passed to StringHelper::strtolower which expects string. Same problem appears to be applicable to HLOOKUP.

I noted the following problem in the code, but will document it here as well. Excel's results are not consistent when a non-numeric string is passed as the third parameter. For example, if cell Z1 contains `xyz`, Excel will return a REF error for function `VLOOKUP(whatever,whatever,Z1)`, but it returns a VALUE error for function `VLOOKUP(whatever,whatever,"xyz")`. I don't think PhpSpreadsheet can match both behaviors. For now, it will return VALUE for both, with similar results for other errors.

While studying the returned errors, I realized there is something that needs to be deprecated. `ExcelError::$errorCodes` is a public static array. This means that a user can change its value, which should not be allowed. It is replaced by a constant. Since the original is public, I think it needs to stay, but with a deprecation notice; users can reference and change it, but it will be unused in the rest of the code. I suppose this might be considered a break in functionality (that should not have been allowed in the first place).
MarkBaker added a commit that referenced this issue Jul 18, 2022
### Added

- Add Chart Axis Option textRotation [Issue #2705](#2705) [PR #2940](#2940)

### Changed

- Nothing

### Deprecated

- Nothing

### Removed

- Nothing

### Fixed

- Fix Encoding issue with Html reader (PHP 8.2 deprecation for mb_convert_encoding) [Issue #2942](#2942) [PR #2943](#2943)
- Additional Chart fixes
  - Pie chart with part separated unwantedly [Issue #2506](#2506) [PR #2928](#2928)
  - Chart styling is lost on simple load / save process [Issue #1797](#1797) [Issue #2077](#2077) [PR #2930](#2930)
  - Can't create contour chart (surface 2d) [Issue #2931](#2931) [PR #2933](#2933)
- VLOOKUP Breaks When Array Contains Null Cells [Issue #2934](#2934) [PR #2939](#2939)
@pcphatweb
Copy link

I have same issue when I work with merge cell null value and luckily the bug was fixed ^^
THANKS ALL!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants