From 0b387e767e99a9042886ea673d9b9905c7951e87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A4ntz=20Miccoli?= Date: Tue, 27 Nov 2018 16:55:06 +0100 Subject: [PATCH] Branch pruning around IF function calls to avoid resolution of every branches Calculation engine was resolving every function by first resolving its arguments including IFs, this was causing significant over evaluation when IFs were used as it meant for every case to be evaluated. Introduce elements to identify ifs and enable better branch resolution (pruning). We tag parsed tokens to associate a branch identifier to them. Closes #844 --- CHANGELOG.md | 1 + .../Calculation/Calculation.php | 331 ++++++++++++++++-- .../Calculation/Token/Stack.php | 70 +++- .../Calculation/CalculationTest.php | 175 +++++++++ tests/data/Calculation/Calculation.php | 63 ++++ 5 files changed, 609 insertions(+), 31 deletions(-) create mode 100644 tests/data/Calculation/Calculation.php 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();