-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Calculation Error - Default to 0 when supplying empty IF result? #2146
Comments
There's already a PR in the pipeline to change the behaviour of the calculation engine for passing empty values. However, there is also a known issue with empty arguments in the lazy evaluation of $calculationEngine = Calculation::getInstance($spreadSheet);
$calculationEngine->disableBranchPruning(); until we have a resolution for the lazy evaluation issue. |
Thanks @MarkBaker for the feedback & this awesome package. I can confirm that the In the real excel sheet (were the formula is even more compelx) I am getting I guess it's best to wait and test after the PR is merged. |
We ran into issues where disableBranchPruning() lead to wrong results, we think those are related to |
If |
@MarkBaker I was a bit unclear. disableBranchPruning() stops Exceptions from happening when we have those partial-IF clauses, but then some values in a very complex sheet are not updated properly. If I don't use disableBranchPruning, it throws an error. But: So for now we have a viable workaround. |
It's generally better to leave branch pruning enabled for performance reasons; but it can't determine whether an if clause has a null value or is missing a value, so it doesn't know whether it should populate that value or not. When branch pruning is disabled, it can correctly identify whether the argument is missing or not, and populate it with the default.
|
Okay, so I understand that miscalculation we were facing is probably something else. |
Fix PHPOffice#3875. Even better, fix PHPOffice#2146, which has been open for 2.5 years. Empty arguments are improperly placed on the stack; in particular, they are added without `onlyIf` and `onlyIfNot` attributes.This results in problems described in 3875. IF has a somewhat unexpected design. In Excel, `IF(false, valueIfTrue)` evaluates as `false`, but `IF(false, valueIfTrue,)` evaluates as 0. This means that IF empty arguments should be handled in the same manner as MIN/MAX/MINA/MAXA, but you need to be careful to distinguish empty from omitted. Also note that IF requires 2 operands - `IF(true)` is an error, but `IF(true,)` evaluates to 0.
This is:
We have quite complex excel sheet which causes an error in calculation. We were able to track down the problem to the example below.
It seems to have something to do with leaving some IF branches empty - but
=IF(0<9,1,)
works.Especially the calculation works in Libre Office and Excel, so it would be nice if there is a fix.
What is the expected behavior?
Return of 1
What is the current behavior?
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:
It does seem to work with the expression
Even more minmal failing example (okay, that means the one before was not minimal :-))
Which versions of PhpSpreadsheet and PHP are affected?
1.18.0
The text was updated successfully, but these errors were encountered: