diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f08b073..60d0eede 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - When <br> appears in a table cell, set the cell to wrap [Issue #1071](https://github.com/PHPOffice/PhpSpreadsheet/issues/1071) and [PR #1070](https://github.com/PHPOffice/PhpSpreadsheet/pull/1070) - Add MAXIFS, MINIFS, COUNTIFS and Remove MINIF, MAXIF - [Issue #1056](https://github.com/PHPOffice/PhpSpreadsheet/issues/1056) - HLookup needs an ordered list even if range_lookup is set to false [Issue #1055](https://github.com/PHPOffice/PhpSpreadsheet/issues/1055) and [PR #1076](https://github.com/PHPOffice/PhpSpreadsheet/pull/1076) +- Improve performance of IF function calls via ranch pruning to avoid resolution of every branches [#844](https://github.com/PHPOffice/PhpSpreadsheet/pull/844) ### Fixed diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 63cd69cc..449553a4 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -66,6 +66,15 @@ class Calculation */ private $calculationCacheEnabled = true; + /** + * Used to generate unique store keys. + * + * @var int + */ + private $branchStoreKeyCounter = 0; + + private $branchPruningEnabled = true; + /** * List of operators that can be used within formulae * The true/false value indicates whether it is a binary operator or a unary operator. @@ -2256,6 +2265,7 @@ class Calculation public function flushInstance() { $this->clearCalculationCache(); + $this->clearBranchStore(); } /** @@ -2399,6 +2409,32 @@ class Calculation } } + /** + * Enable/disable calculation cache. + * + * @param bool $pValue + * @param mixed $enabled + */ + public function setBranchPruningEnabled($enabled) + { + $this->branchPruningEnabled = $enabled; + } + + public function enableBranchPruning() + { + $this->setBranchPruningEnabled(true); + } + + public function disableBranchPruning() + { + $this->setBranchPruningEnabled(false); + } + + public function clearBranchStore() + { + $this->branchStoreKeyCounter = 0; + } + /** * Get the currently defined locale code. * @@ -2867,6 +2903,7 @@ class Calculation if (($this->calculationCacheEnabled) && (isset($this->calculationCache[$cellReference]))) { $this->debugLog->writeDebugLog('Retrieving value for cell ', $cellReference, ' from cache'); // Return the cached result + $cellValue = $this->calculationCache[$cellReference]; return true; @@ -3326,9 +3363,53 @@ class Calculation // - is a negation or + is a positive operator rather than an operation $expectingOperand = false; // We use this test in syntax-checking the expression to determine whether an operand // should be null in a function call + + // IF branch pruning + // currently pending storeKey (last item of the storeKeysStack + $pendingStoreKey = null; + // stores a list of storeKeys (string[]) + $pendingStoreKeysStack = []; + $expectingConditionMap = []; // ['storeKey' => true, ...] + $expectingThenMap = []; // ['storeKey' => true, ...] + $expectingElseMap = []; // ['storeKey' => true, ...] + $parenthesisDepthMap = []; // ['storeKey' => 4, ...] + // The guts of the lexical parser // Loop through the formula extracting each operator and operand in turn while (true) { + // Branch pruning: we adapt the output item to the context (it will + // be used to limit its computation) + $currentCondition = null; + $currentOnlyIf = null; + $currentOnlyIfNot = null; + $previousStoreKey = null; + $pendingStoreKey = end($pendingStoreKeysStack); + + if ($this->branchPruningEnabled) { + // this is a condition ? + if (isset($expectingConditionMap[$pendingStoreKey]) && $expectingConditionMap[$pendingStoreKey]) { + $currentCondition = $pendingStoreKey; + $stackDepth = count($pendingStoreKeysStack); + if ($stackDepth > 1) { // nested if + $previousStoreKey = $pendingStoreKeysStack[$stackDepth - 2]; + } + } + if (isset($expectingThenMap[$pendingStoreKey]) && $expectingThenMap[$pendingStoreKey]) { + $currentOnlyIf = $pendingStoreKey; + } elseif (isset($previousStoreKey)) { + if (isset($expectingThenMap[$previousStoreKey]) && $expectingThenMap[$previousStoreKey]) { + $currentOnlyIf = $previousStoreKey; + } + } + if (isset($expectingElseMap[$pendingStoreKey]) && $expectingElseMap[$pendingStoreKey]) { + $currentOnlyIfNot = $pendingStoreKey; + } elseif (isset($previousStoreKey)) { + if (isset($expectingElseMap[$previousStoreKey]) && $expectingElseMap[$previousStoreKey]) { + $currentOnlyIfNot = $previousStoreKey; + } + } + } + $opCharacter = $formula[$index]; // Get the first character of the value at the current index position if ((isset(self::$comparisonOperators[$opCharacter])) && (strlen($formula) > $index) && (isset(self::$comparisonOperators[$formula[$index + 1]]))) { $opCharacter .= $formula[++$index]; @@ -3338,10 +3419,12 @@ class Calculation $isOperandOrFunction = preg_match($regexpMatchString, substr($formula, $index), $match); if ($opCharacter == '-' && !$expectingOperator) { // Is it a negation instead of a minus? - $stack->push('Unary Operator', '~'); // Put a negation on the stack + // Put a negation on the stack + $stack->push('Unary Operator', '~', null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); ++$index; // and drop the negation symbol } elseif ($opCharacter == '%' && $expectingOperator) { - $stack->push('Unary Operator', '%'); // Put a percentage on the stack + // Put a percentage on the stack + $stack->push('Unary Operator', '%', null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); ++$index; } elseif ($opCharacter == '+' && !$expectingOperator) { // Positive (unary plus rather than binary operator plus) can be discarded? ++$index; // Drop the redundant plus symbol @@ -3354,7 +3437,10 @@ class Calculation @(self::$operatorAssociativity[$opCharacter] ? self::$operatorPrecedence[$opCharacter] < self::$operatorPrecedence[$o2['value']] : self::$operatorPrecedence[$opCharacter] <= self::$operatorPrecedence[$o2['value']])) { $output[] = $stack->pop(); // Swap operands and higher precedence operators from the stack to the output } - $stack->push('Binary Operator', $opCharacter); // Finally put our current operator onto the stack + + // Finally put our current operator onto the stack + $stack->push('Binary Operator', $opCharacter, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + ++$index; $expectingOperator = false; } elseif ($opCharacter == ')' && $expectingOperator) { // Are we expecting to close a parenthesis? @@ -3366,7 +3452,29 @@ class Calculation $output[] = $o2; } $d = $stack->last(2); + + // Branch pruning we decrease the depth whether is it a function + // call or a parenthesis + if (!empty($pendingStoreKey)) { + $parenthesisDepthMap[$pendingStoreKey] -= 1; + } + if (preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/i', $d['value'], $matches)) { // Did this parenthesis just close a function? + if (!empty($pendingStoreKey) && $parenthesisDepthMap[$pendingStoreKey] == -1) { + // we are closing an IF( + if ($d['value'] != 'IF(') { + return $this->raiseFormulaError('Parser bug we should be in an "IF("'); + } + if ($expectingConditionMap[$pendingStoreKey]) { + return $this->raiseFormulaError('We should not be expecting a condition'); + } + $expectingThenMap[$pendingStoreKey] = false; + $expectingElseMap[$pendingStoreKey] = false; + $parenthesisDepthMap[$pendingStoreKey] -= 1; + array_pop($pendingStoreKeysStack); + unset($pendingStoreKey); + } + $functionName = $matches[1]; // Get the function name $d = $stack->pop(); $argumentCount = $d['value']; // See how many arguments there were (argument count is the next value stored on the stack) @@ -3427,6 +3535,20 @@ class Calculation } ++$index; } elseif ($opCharacter == ',') { // Is this the separator for function arguments? + if (!empty($pendingStoreKey) && + $parenthesisDepthMap[$pendingStoreKey] == 0 + ) { + // We must go to the IF next argument + if ($expectingConditionMap[$pendingStoreKey]) { + $expectingConditionMap[$pendingStoreKey] = false; + $expectingThenMap[$pendingStoreKey] = true; + } elseif ($expectingThenMap[$pendingStoreKey]) { + $expectingThenMap[$pendingStoreKey] = false; + $expectingElseMap[$pendingStoreKey] = true; + } elseif ($expectingElseMap[$pendingStoreKey]) { + return $this->raiseFormulaError('Reaching fourth argument of an IF'); + } + } while (($o2 = $stack->pop()) && $o2['value'] != '(') { // Pop off the stack back to the last ( if ($o2 === null) { return $this->raiseFormulaError('Formula Error: Unexpected ,'); @@ -3444,13 +3566,19 @@ class Calculation return $this->raiseFormulaError('Formula Error: Unexpected ,'); } $d = $stack->pop(); - $stack->push($d['type'], ++$d['value'], $d['reference']); // increment the argument count - $stack->push('Brace', '('); // put the ( back on, we'll need to pop back to it again + $itemStoreKey = $d['storeKey'] ?? null; + $itemOnlyIf = $d['onlyIf'] ?? null; + $itemOnlyIfNot = $d['onlyIfNot'] ?? null; + $stack->push($d['type'], ++$d['value'], $d['reference'], $itemStoreKey, $itemOnlyIf, $itemOnlyIfNot); // increment the argument count + $stack->push('Brace', '(', null, $itemStoreKey, $itemOnlyIf, $itemOnlyIfNot); // put the ( back on, we'll need to pop back to it again $expectingOperator = false; $expectingOperand = true; ++$index; } elseif ($opCharacter == '(' && !$expectingOperator) { - $stack->push('Brace', '('); + if (!empty($pendingStoreKey)) { // Branch pruning: we go deeper + $parenthesisDepthMap[$pendingStoreKey] += 1; + } + $stack->push('Brace', '(', null, $currentCondition, $currentOnlyIf, $currentOnlyIf); ++$index; } elseif ($isOperandOrFunction && !$expectingOperator) { // do we now have a function/variable/number? $expectingOperator = true; @@ -3461,13 +3589,29 @@ class Calculation if (preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/i', $val, $matches)) { $val = preg_replace('/\s/u', '', $val); if (isset(self::$phpSpreadsheetFunctions[strtoupper($matches[1])]) || isset(self::$controlFunctions[strtoupper($matches[1])])) { // it's a function - $stack->push('Function', strtoupper($val)); + $valToUpper = strtoupper($val); + // here $matches[1] will contain values like "IF" + // and $val "IF(" + $injectStoreKey = null; + if ($this->branchPruningEnabled && ($valToUpper == 'IF(')) { // we handle a new if + $pendingStoreKey = $this->getUnusedBranchStoreKey(); + $pendingStoreKeysStack[] = $pendingStoreKey; + $expectingConditionMap[$pendingStoreKey] = true; + $parenthesisDepthMap[$pendingStoreKey] = 0; + } else { // this is not a if but we good deeper + if (!empty($pendingStoreKey) && array_key_exists($pendingStoreKey, $parenthesisDepthMap)) { + $parenthesisDepthMap[$pendingStoreKey] += 1; + } + } + + $stack->push('Function', $valToUpper, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + // tests if the function is closed right after opening $ax = preg_match('/^\s*(\s*\))/ui', substr($formula, $index + $length), $amatch); if ($ax) { - $stack->push('Operand Count for Function ' . strtoupper($val) . ')', 0); + $stack->push('Operand Count for Function ' . $valToUpper . ')', 0, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); $expectingOperator = true; } else { - $stack->push('Operand Count for Function ' . strtoupper($val) . ')', 1); + $stack->push('Operand Count for Function ' . $valToUpper . ')', 1, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); $expectingOperator = false; } $stack->push('Brace', '('); @@ -3495,7 +3639,9 @@ class Calculation } } - $output[] = ['type' => 'Cell Reference', 'value' => $val, 'reference' => $val]; + $outputItem = $stack->getStackItem('Cell Reference', $val, $val, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); + + $output[] = $outputItem; } else { // it's a variable, constant, string, number or boolean // If the last entry on the stack was a : operator, then we may have a row or column range reference $testPrevOp = $stack->last(1); @@ -3542,7 +3688,7 @@ class Calculation } elseif (($localeConstant = array_search(trim(strtoupper($val)), self::$localeBoolean)) !== false) { $val = self::$excelConstants[$localeConstant]; } - $details = ['type' => 'Value', 'value' => $val, 'reference' => null]; + $details = $stack->getStackItem('Value', $val, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot); if ($localeConstant) { $details['localeValue'] = $localeConstant; } @@ -3645,9 +3791,74 @@ class Calculation $pCellParent = ($pCell !== null) ? $pCell->getParent() : null; $stack = new Stack(); + // Stores branches that have been pruned + $fakedForBranchPruning = []; + // help us to know when pruning ['branchTestId' => true/false] + $branchStore = []; + // Loop through each token in turn foreach ($tokens as $tokenData) { $token = $tokenData['value']; + + // Branch pruning: skip useless resolutions + $storeKey = $tokenData['storeKey'] ?? null; + if ($this->branchPruningEnabled && isset($tokenData['onlyIf'])) { + $onlyIfStoreKey = $tokenData['onlyIf']; + $storeValue = $branchStore[$onlyIfStoreKey] ?? null; + if (is_array($storeValue)) { + $wrappedItem = end($storeValue); + $storeValue = end($wrappedItem); + } + + if (isset($storeValue) && (($storeValue !== true) + || ($storeValue === 'Pruned branch')) + ) { + // If branching value is not true, we don't need to compute + if (!isset($fakedForBranchPruning['onlyIf-' . $onlyIfStoreKey])) { + $stack->push('Value', 'Pruned branch (only if ' . $onlyIfStoreKey . ') ' . $token); + $fakedForBranchPruning['onlyIf-' . $onlyIfStoreKey] = true; + } + + if (isset($storeKey)) { + // We are processing an if condition + // We cascade the pruning to the depending branches + $branchStore[$storeKey] = 'Pruned branch'; + $fakedForBranchPruning['onlyIfNot-' . $storeKey] = true; + $fakedForBranchPruning['onlyIf-' . $storeKey] = true; + } + + continue; + } + } + + if ($this->branchPruningEnabled && isset($tokenData['onlyIfNot'])) { + $onlyIfNotStoreKey = $tokenData['onlyIfNot']; + $storeValue = $branchStore[$onlyIfNotStoreKey] ?? null; + if (is_array($storeValue)) { + $wrappedItem = end($storeValue); + $storeValue = end($wrappedItem); + } + if (isset($storeValue) && ($storeValue + || ($storeValue === 'Pruned branch')) + ) { + // If branching value is true, we don't need to compute + if (!isset($fakedForBranchPruning['onlyIfNot-' . $onlyIfNotStoreKey])) { + $stack->push('Value', 'Pruned branch (only if not ' . $onlyIfNotStoreKey . ') ' . $token); + $fakedForBranchPruning['onlyIfNot-' . $onlyIfNotStoreKey] = true; + } + + if (isset($storeKey)) { + // We are processing an if condition + // We cascade the pruning to the depending branches + $branchStore[$storeKey] = 'Pruned branch'; + $fakedForBranchPruning['onlyIfNot-' . $storeKey] = true; + $fakedForBranchPruning['onlyIf-' . $storeKey] = true; + } + + continue; + } + } + // if the token is a binary operator, pop the top two values off the stack, do the operation, and push the result back on the stack if (isset(self::$binaryOperators[$token])) { // We must have two operands, error if we don't @@ -3677,7 +3888,10 @@ class Calculation case '<=': // Less than or Equal to case '=': // Equality case '<>': // Inequality - $this->executeBinaryComparisonOperation($cellID, $operand1, $operand2, $token, $stack); + $result = $this->executeBinaryComparisonOperation($cellID, $operand1, $operand2, $token, $stack); + if (isset($storeKey)) { + $branchStore[$storeKey] = $result; + } break; // Binary Operators @@ -3733,23 +3947,38 @@ class Calculation break; case '+': // Addition - $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'plusEquals', $stack); + $result = $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'plusEquals', $stack); + if (isset($storeKey)) { + $branchStore[$storeKey] = $result; + } break; case '-': // Subtraction - $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'minusEquals', $stack); + $result = $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'minusEquals', $stack); + if (isset($storeKey)) { + $branchStore[$storeKey] = $result; + } break; case '*': // Multiplication - $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'arrayTimesEquals', $stack); + $result = $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'arrayTimesEquals', $stack); + if (isset($storeKey)) { + $branchStore[$storeKey] = $result; + } break; case '/': // Division - $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'arrayRightDivide', $stack); + $result = $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'arrayRightDivide', $stack); + if (isset($storeKey)) { + $branchStore[$storeKey] = $result; + } break; case '^': // Exponential - $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'power', $stack); + $result = $this->executeNumericBinaryOperation($operand1, $operand2, $token, 'power', $stack); + if (isset($storeKey)) { + $branchStore[$storeKey] = $result; + } break; case '&': // Concatenation @@ -3782,6 +4011,10 @@ class Calculation $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($result)); $stack->push('Value', $result); + if (isset($storeKey)) { + $branchStore[$storeKey] = $result; + } + break; case '|': // Intersect $rowIntersect = array_intersect_key($operand1, $operand2); @@ -3826,6 +4059,9 @@ class Calculation } $this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($result)); $stack->push('Value', $result); + if (isset($storeKey)) { + $branchStore[$storeKey] = $result; + } } else { $this->executeNumericBinaryOperation($multiplier, $arg, '*', 'arrayTimesEquals', $stack); } @@ -3899,9 +4135,20 @@ class Calculation } } $stack->push('Value', $cellValue, $cellRef); + if (isset($storeKey)) { + $branchStore[$storeKey] = $cellValue; + } - // if the token is a function, pop arguments off the stack, hand them to the function, and push the result back on + // if the token is a function, pop arguments off the stack, hand them to the function, and push the result back on } elseif (preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/i', $token, $matches)) { + if (($cellID == 'AC99') || (isset($pCell) && $pCell->getCoordinate() == 'AC99')) { + if (defined('RESOLVING')) { + define('RESOLVING2', true); + } else { + define('RESOLVING', true); + } + } + $functionName = $matches[1]; $argCount = $stack->pop(); $argCount = $argCount['value']; @@ -3944,6 +4191,7 @@ class Calculation } } } + // Reverse the order of the arguments krsort($args); @@ -3974,16 +4222,25 @@ class Calculation $this->debugLog->writeDebugLog('Evaluation Result for ', self::localeFunc($functionName), '() function call is ', $this->showTypeDetails($result)); } $stack->push('Value', self::wrapResult($result)); + if (isset($storeKey)) { + $branchStore[$storeKey] = $result; + } } } else { // if the token is a number, boolean, string or an Excel error, push it onto the stack if (isset(self::$excelConstants[strtoupper($token)])) { $excelConstant = strtoupper($token); $stack->push('Constant Value', self::$excelConstants[$excelConstant]); + if (isset($storeKey)) { + $branchStore[$storeKey] = self::$excelConstants[$excelConstant]; + } $this->debugLog->writeDebugLog('Evaluating Constant ', $excelConstant, ' as ', $this->showTypeDetails(self::$excelConstants[$excelConstant])); } elseif ((is_numeric($token)) || ($token === null) || (is_bool($token)) || ($token == '') || ($token[0] == '"') || ($token[0] == '#')) { $stack->push('Value', $token); - // if the token is a named range, push the named range name onto the stack + if (isset($storeKey)) { + $branchStore[$storeKey] = $token; + } + // if the token is a named range, push the named range name onto the stack } elseif (preg_match('/^' . self::CALCULATION_REGEXP_NAMEDRANGE . '$/i', $token, $matches)) { $namedRange = $matches[6]; $this->debugLog->writeDebugLog('Evaluating Named Range ', $namedRange); @@ -3992,6 +4249,9 @@ class Calculation $pCell->attach($pCellParent); $this->debugLog->writeDebugLog('Evaluation Result for named range ', $namedRange, ' is ', $this->showTypeDetails($cellValue)); $stack->push('Named Range', $cellValue, $namedRange); + if (isset($storeKey)) { + $branchStore[$storeKey] = $cellValue; + } } else { return $this->raiseFormulaError("undefined variable '$token'"); } @@ -4053,7 +4313,7 @@ class Calculation * @param Stack $stack * @param bool $recursingArrays * - * @return bool + * @return mixed */ private function executeBinaryComparisonOperation($cellID, $operand1, $operand2, $operation, Stack &$stack, $recursingArrays = false) { @@ -4090,7 +4350,7 @@ class Calculation // And push the result onto the stack $stack->push('Array', $result); - return true; + return $result; } // Simple validate the two operands if they are string values @@ -4180,7 +4440,7 @@ class Calculation // And push the result onto the stack $stack->push('Value', $result); - return true; + return $result; } /** @@ -4206,7 +4466,7 @@ class Calculation * @param string $matrixFunction * @param mixed $stack * - * @return bool + * @return bool|mixed */ private function executeNumericBinaryOperation($operand1, $operand2, $operation, $matrixFunction, &$stack) { @@ -4284,7 +4544,7 @@ class Calculation // And push the result onto the stack $stack->push('Value', $result); - return true; + return $result; } // trigger an error, but nicely, if need be @@ -4488,4 +4748,27 @@ class Calculation return $args; } + + private function getUnusedBranchStoreKey() + { + $storeKeyValue = 'storeKey-' . $this->branchStoreKeyCounter; + ++$this->branchStoreKeyCounter; + + return $storeKeyValue; + } + + private function getTokensAsString($tokens) + { + $tokensStr = array_map(function ($token) { + $value = $token['value'] ?? 'no value'; + while (is_array($value)) { + $value = array_pop($value); + } + + return $value; + }, $tokens); + $str = '[ ' . implode(' | ', $tokensStr) . ' ]'; + + return $str; + } } diff --git a/src/PhpSpreadsheet/Calculation/Token/Stack.php b/src/PhpSpreadsheet/Calculation/Token/Stack.php index b8dccf95..341a0179 100644 --- a/src/PhpSpreadsheet/Calculation/Token/Stack.php +++ b/src/PhpSpreadsheet/Calculation/Token/Stack.php @@ -36,14 +36,24 @@ class Stack * @param mixed $type * @param mixed $value * @param mixed $reference + * @param null|string $storeKey will store the result under this alias + * @param null|string $onlyIf will only run computation if the matching + * store key is true + * @param null|string $onlyIfNot will only run computation if the matching + * store key is false */ - public function push($type, $value, $reference = null) - { - $this->stack[$this->count++] = [ - 'type' => $type, - 'value' => $value, - 'reference' => $reference, - ]; + public function push( + $type, + $value, + $reference = null, + $storeKey = null, + $onlyIf = null, + $onlyIfNot = null + ) { + $stackItem = $this->getStackItem($type, $value, $reference, $storeKey, $onlyIf, $onlyIfNot); + + $this->stack[$this->count++] = $stackItem; + if ($type == 'Function') { $localeFunction = Calculation::localeFunc($value); if ($localeFunction != $value) { @@ -52,6 +62,35 @@ class Stack } } + public function getStackItem( + $type, + $value, + $reference = null, + $storeKey = null, + $onlyIf = null, + $onlyIfNot = null + ) { + $stackItem = [ + 'type' => $type, + 'value' => $value, + 'reference' => $reference, + ]; + + if (isset($storeKey)) { + $stackItem['storeKey'] = $storeKey; + } + + if (isset($onlyIf)) { + $stackItem['onlyIf'] = $onlyIf; + } + + if (isset($onlyIfNot)) { + $stackItem['onlyIfNot'] = $onlyIfNot; + } + + return $stackItem; + } + /** * Pop the last entry from the stack. * @@ -90,4 +129,21 @@ class Stack $this->stack = []; $this->count = 0; } + + public function __toString() + { + $str = 'Stack: '; + foreach ($this->stack as $index => $item) { + if ($index > $this->count - 1) { + break; + } + $value = $item['value'] ?? 'no value'; + while (is_array($value)) { + $value = array_pop($value); + } + $str .= $value . ' |> '; + } + + return $str; + } } diff --git a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php index 4b81fbf4..6bceee0f 100644 --- a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php @@ -163,4 +163,179 @@ class CalculationTest extends TestCase self::assertEquals("=cmd|'/C calc'!A0", $cell->getCalculatedValue()); } + + public function testBranchPruningFormulaParsingSimpleCase() + { + $calculation = Calculation::getInstance(); + $calculation->flushInstance(); // resets the ids + + // Very simple formula + $formula = '=IF(A1="please +",B1)'; + $tokens = $calculation->parseFormula($formula); + + $foundEqualAssociatedToStoreKey = false; + $foundConditionalOnB1 = false; + foreach ($tokens as $token) { + $isBinaryOperator = $token['type'] == 'Binary Operator'; + $isEqual = $token['value'] == '='; + $correctStoreKey = ($token['storeKey'] ?? '') == 'storeKey-0'; + $correctOnlyIf = ($token['onlyIf'] ?? '') == 'storeKey-0'; + $isB1Reference = ($token['reference'] ?? '') == 'B1'; + + $foundEqualAssociatedToStoreKey = $foundEqualAssociatedToStoreKey || + ($isBinaryOperator && $isEqual && $correctStoreKey); + + $foundConditionalOnB1 = $foundConditionalOnB1 || + ($isB1Reference && $correctOnlyIf); + } + $this->assertTrue($foundEqualAssociatedToStoreKey); + $this->assertTrue($foundConditionalOnB1); + } + + public function testBranchPruningFormulaParsingMultipleIfsCase() + { + $calculation = Calculation::getInstance(); + $calculation->flushInstance(); // resets the ids + + // + // Internal operation + $formula = '=IF(A1="please +",SUM(B1:B3))+IF(A2="please *",PRODUCT(C1:C3), C1)'; + $tokens = $calculation->parseFormula($formula); + + $plusGotTagged = false; + $productFunctionCorrectlyTagged = false; + foreach ($tokens as $token) { + $isBinaryOperator = $token['type'] == 'Binary Operator'; + $isPlus = $token['value'] == '+'; + $anyStoreKey = isset($token['storeKey']); + $anyOnlyIf = isset($token['onlyIf']); + $anyOnlyIfNot = isset($token['onlyIfNot']); + $plusGotTagged = $plusGotTagged || + ($isBinaryOperator && $isPlus && + ($anyStoreKey || $anyOnlyIfNot || $anyOnlyIf)); + + $isFunction = $token['type'] == 'Function'; + $isProductFunction = $token['value'] == 'PRODUCT('; + $correctOnlyIf = ($token['onlyIf'] ?? '') == 'storeKey-1'; + $productFunctionCorrectlyTagged = $productFunctionCorrectlyTagged || ($isFunction && $isProductFunction && $correctOnlyIf); + } + $this->assertFalse($plusGotTagged, 'chaining IF( should not affect the external operators'); + $this->assertTrue($productFunctionCorrectlyTagged, 'function nested inside if should be tagged to be processed only if parent branching requires it'); + } + + public function testBranchPruningFormulaParingNestedIfCase() + { + $calculation = Calculation::getInstance(); + $calculation->flushInstance(); // resets the ids + + $formula = '=IF(A1="please +",SUM(B1:B3),1+IF(NOT(A2="please *"),C2-C1,PRODUCT(C1:C3)))'; + $tokens = $calculation->parseFormula($formula); + + $plusCorrectlyTagged = false; + $productFunctionCorrectlyTagged = false; + $notFunctionCorrectlyTagged = false; + $findOneOperandCountTagged = false; + foreach ($tokens as $token) { + $value = $token['value']; + $isPlus = $value == '+'; + $isProductFunction = $value == 'PRODUCT('; + $isNotFunction = $value == 'NOT('; + $isIfOperand = $token['type'] == 'Operand Count for Function IF()'; + $isOnlyIfNotDepth1 = (array_key_exists('onlyIfNot', $token) ? $token['onlyIfNot'] : null) == 'storeKey-1'; + $isStoreKeyDepth1 = (array_key_exists('storeKey', $token) ? $token['storeKey'] : null) == 'storeKey-1'; + $isOnlyIfNotDepth0 = (array_key_exists('onlyIfNot', $token) ? $token['onlyIfNot'] : null) == 'storeKey-0'; + + $plusCorrectlyTagged = $plusCorrectlyTagged || ($isPlus && $isOnlyIfNotDepth0); + $notFunctionCorrectlyTagged = $notFunctionCorrectlyTagged || ($isNotFunction && $isOnlyIfNotDepth0 && $isStoreKeyDepth1); + $productFunctionCorrectlyTagged = $productFunctionCorrectlyTagged || ($isProductFunction && $isOnlyIfNotDepth1 && !$isStoreKeyDepth1 && !$isOnlyIfNotDepth0); + $findOneOperandCountTagged = $findOneOperandCountTagged || ($isIfOperand && $isOnlyIfNotDepth0); + } + $this->assertTrue($plusCorrectlyTagged); + $this->assertTrue($productFunctionCorrectlyTagged); + $this->assertTrue($notFunctionCorrectlyTagged); + } + + public function testBranchPruningFormulaParsingNoArgumentFunctionCase() + { + $calculation = Calculation::getInstance(); + $calculation->flushInstance(); // resets the ids + + $formula = '=IF(AND(TRUE(),A1="please +"),2,3)'; + // this used to raise a parser error, we keep it even though we don't + // test the output + $calculation->parseFormula($formula); + } + + public function testBranchPruningFormulaParsingInequalitiesConditionsCase() + { + $calculation = Calculation::getInstance(); + $calculation->flushInstance(); // resets the ids + + $formula = '=IF(A1="flag",IF(A2<10, 0) + IF(A3<10000, 0))'; + $tokens = $calculation->parseFormula($formula); + $properlyTaggedPlus = false; + foreach ($tokens as $token) { + $isPlus = $token['value'] === '+'; + $hasOnlyIf = !empty($token['onlyIf']); + + $properlyTaggedPlus = $properlyTaggedPlus || + ($isPlus && $hasOnlyIf); + } + $this->assertTrue($properlyTaggedPlus); + } + + /** + * @param $expectedResult + * @param $dataArray + * @param string $formula + * @param string $cellCoordinates where to put the formula + * @param string[] $shouldBeSetInCacheCells coordinates of cells that must + * be set in cache + * @param string[] $shouldNotBeSetInCacheCells coordinates of cells that must + * not be set in cache because of pruning + * + * @throws \PhpOffice\PhpSpreadsheet\Exception + * @dataProvider dataProviderBranchPruningFullExecution + */ + public function testFullExecution( + $expectedResult, + $dataArray, + $formula, + $cellCoordinates, + $shouldBeSetInCacheCells = [], + $shouldNotBeSetInCacheCells = [] + ) { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + + $sheet->fromArray($dataArray); + $cell = $sheet->getCell($cellCoordinates); + $calculation = Calculation::getInstance($cell->getWorksheet()->getParent()); + + $cell->setValue($formula); + $calculated = $cell->getCalculatedValue(); + $this->assertEquals($expectedResult, $calculated); + + // this mostly to ensure that at least some cells are cached + foreach ($shouldBeSetInCacheCells as $setCell) { + unset($inCache); + $calculation->getValueFromCache('Worksheet!' . $setCell, $inCache); + $this->assertNotEmpty($inCache); + } + + foreach ($shouldNotBeSetInCacheCells as $notSetCell) { + unset($inCache); + $calculation->getValueFromCache('Worksheet!' . $notSetCell, $inCache); + $this->assertEmpty($inCache); + } + + $calculation->disableBranchPruning(); + $calculated = $cell->getCalculatedValue(); + $this->assertEquals($expectedResult, $calculated); + } + + public function dataProviderBranchPruningFullExecution() + { + return require 'data/Calculation/Calculation.php'; + } } diff --git a/tests/data/Calculation/Calculation.php b/tests/data/Calculation/Calculation.php new file mode 100644 index 00000000..09538c3c --- /dev/null +++ b/tests/data/Calculation/Calculation.php @@ -0,0 +1,63 @@ +3,C1,0)', 'E5']; + + $dataArray4 = [ + ['noflag', 0, 0], + [127000, 0, 0], + [ 10000, 0.03, 0], + [ 20000, 0.06, 0], + [ 40000, 0.09, 0], + [ 70000, 0.12, 0], + [ 90000, 0.03, 0] + ]; + $formula2 = '=IF(A1="flag",IF(A2<10, 0) + IF(A3<10000, 0))'; + $set7 = [false, $dataArray4, $formula2, 'E5']; + + $dataArray5 = [ + [1, 2], + [3, 4], + ['=A1+A2', '=SUM(B1:B2)'], + [ 'take A', 0] + ]; + $formula3 = '=IF(A4="take A", A3, B3)'; + $set8 = [4, $dataArray5, $formula3, 'E5', ['A3'], ['B3']]; + + return [$set0, $set1, $set2, $set3, $set4, $set5, $set6, $set7, $set8]; +}; + +return calculationTestDataGenerator();