From: Yegor Kozlov Date: Mon, 14 Jan 2019 14:48:21 +0000 (+0000) Subject: follow-up to Bug 62904. More tests and improved evaluation of IF in array mode X-Git-Tag: REL_4_1_0~104 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=698d8eb0066ff7fa3fa0addcc042ea260e5756e4;p=poi.git follow-up to Bug 62904. More tests and improved evaluation of IF in array mode git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1851263 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java index e91c0e1095..bab3e4e66e 100644 --- a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java +++ b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java @@ -398,6 +398,9 @@ public final class WorkbookEvaluator { dbgEvaluationOutputIndent++; } + EvaluationSheet evalSheet = ec.getWorkbook().getSheet(ec.getSheetIndex()); + EvaluationCell evalCell = evalSheet.getCell(ec.getRowIndex(), ec.getColumnIndex()); + Stack stack = new Stack<>(); for (int i = 0, iSize = ptgs.length; i < iSize; i++) { // since we don't know how to handle these yet :( @@ -436,46 +439,41 @@ public final class WorkbookEvaluator { continue; } if (attrPtg.isOptimizedIf()) { - ValueEval arg0 = stack.pop(); - boolean evaluatedPredicate; - try { - evaluatedPredicate = IfFunc.evaluateFirstArg(arg0, ec.getRowIndex(), ec.getColumnIndex()); - } catch (EvaluationException e) { - stack.push(e.getErrorEval()); - int dist = attrPtg.getData(); - i+= countTokensToBeSkipped(ptgs, i, dist); - attrPtg = (AttrPtg) ptgs[i]; - dist = attrPtg.getData()+1; - i+= countTokensToBeSkipped(ptgs, i, dist); - continue; - } - if (evaluatedPredicate) { - // nothing to skip - true param follows - } else { - int dist = attrPtg.getData(); - Ptg currPtg = ptgs[i+1]; - i+= countTokensToBeSkipped(ptgs, i, dist); - Ptg nextPtg = ptgs[i+1]; - - if (ptgs[i] instanceof AttrPtg && nextPtg instanceof FuncVarPtg && - // in order to verify that there is no third param, we need to check - // if we really have the IF next or some other FuncVarPtg as third param, e.g. ROW()/COLUMN()! - ((FuncVarPtg)nextPtg).getFunctionIndex() == FunctionMetadataRegistry.FUNCTION_INDEX_IF) { - // this is an if statement without a false param (as opposed to MissingArgPtg as the false param) - //i++; - stack.push(arg0); - if(currPtg instanceof AreaPtg){ - // IF in array mode. See Bug 62904 - ValueEval currEval = getEvalForPtg(currPtg, ec); - stack.push(currEval); - } else { + if(!evalCell.isPartOfArrayFormulaGroup()) { + ValueEval arg0 = stack.pop(); + boolean evaluatedPredicate; + + try { + evaluatedPredicate = IfFunc.evaluateFirstArg(arg0, ec.getRowIndex(), ec.getColumnIndex()); + } catch (EvaluationException e) { + stack.push(e.getErrorEval()); + int dist = attrPtg.getData(); + i += countTokensToBeSkipped(ptgs, i, dist); + attrPtg = (AttrPtg) ptgs[i]; + dist = attrPtg.getData() + 1; + i += countTokensToBeSkipped(ptgs, i, dist); + continue; + } + if (evaluatedPredicate) { + // nothing to skip - true param follows + } else { + int dist = attrPtg.getData(); + i += countTokensToBeSkipped(ptgs, i, dist); + Ptg nextPtg = ptgs[i + 1]; + if (ptgs[i] instanceof AttrPtg && nextPtg instanceof FuncVarPtg && + // in order to verify that there is no third param, we need to check + // if we really have the IF next or some other FuncVarPtg as third param, e.g. ROW()/COLUMN()! + ((FuncVarPtg) nextPtg).getFunctionIndex() == FunctionMetadataRegistry.FUNCTION_INDEX_IF) { + // this is an if statement without a false param (as opposed to MissingArgPtg as the false param) + //i++; + stack.push(arg0); stack.push(BoolEval.FALSE); } } } continue; } - if (attrPtg.isSkip()) { + if (attrPtg.isSkip() && !evalCell.isPartOfArrayFormulaGroup()) { int dist = attrPtg.getData()+1; i+= countTokensToBeSkipped(ptgs, i, dist); if (stack.peek() == MissingArgEval.instance) { diff --git a/src/java/org/apache/poi/ss/formula/eval/UnaryMinusEval.java b/src/java/org/apache/poi/ss/formula/eval/UnaryMinusEval.java index ac1ee34118..8107c48544 100644 --- a/src/java/org/apache/poi/ss/formula/eval/UnaryMinusEval.java +++ b/src/java/org/apache/poi/ss/formula/eval/UnaryMinusEval.java @@ -48,7 +48,10 @@ public final class UnaryMinusEval extends Fixed1ArgFunction implements ArrayFun @Override public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex){ - return evaluateOneArrayArg(args, srcRowIndex, srcColumnIndex, (valA) -> + if (args.length != 1) { + return ErrorEval.VALUE_INVALID; + } + return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex, (valA) -> evaluate(srcRowIndex, srcColumnIndex, valA) ); } diff --git a/src/java/org/apache/poi/ss/formula/eval/UnaryPlusEval.java b/src/java/org/apache/poi/ss/formula/eval/UnaryPlusEval.java index d5cb586a85..81b36aad40 100644 --- a/src/java/org/apache/poi/ss/formula/eval/UnaryPlusEval.java +++ b/src/java/org/apache/poi/ss/formula/eval/UnaryPlusEval.java @@ -52,7 +52,10 @@ public final class UnaryPlusEval extends Fixed1ArgFunction implements ArrayFunc @Override public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex){ - return evaluateOneArrayArg(args, srcRowIndex, srcColumnIndex, (valA) -> + if (args.length != 1) { + return ErrorEval.VALUE_INVALID; + } + return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex, (valA) -> evaluate(srcRowIndex, srcColumnIndex, valA) ); } diff --git a/src/java/org/apache/poi/ss/formula/functions/ArrayFunction.java b/src/java/org/apache/poi/ss/formula/functions/ArrayFunction.java index 088d7b34fd..7f8a359d3a 100644 --- a/src/java/org/apache/poi/ss/formula/functions/ArrayFunction.java +++ b/src/java/org/apache/poi/ss/formula/functions/ArrayFunction.java @@ -111,6 +111,12 @@ public interface ArrayFunction { vA = ErrorEval.NAME_INVALID; } catch (EvaluationException e) { vA = e.getErrorEval(); + } catch (RuntimeException e) { + if(e.getMessage().startsWith("Don't now how to evaluate name")){ + vA = ErrorEval.NAME_INVALID; + } else { + throw e; + } } ValueEval vB; try { @@ -119,6 +125,12 @@ public interface ArrayFunction { vB = ErrorEval.NAME_INVALID; } catch (EvaluationException e) { vB = e.getErrorEval(); + } catch (RuntimeException e) { + if(e.getMessage().startsWith("Don't now how to evaluate name")){ + vB = ErrorEval.NAME_INVALID; + } else { + throw e; + } } if(vA instanceof ErrorEval){ vals[idx++] = vA; @@ -138,10 +150,8 @@ public interface ArrayFunction { return new CacheAreaEval(srcRowIndex, srcColumnIndex, srcRowIndex + height - 1, srcColumnIndex + width - 1, vals); } - default ValueEval evaluateOneArrayArg(ValueEval[] args, int srcRowIndex, int srcColumnIndex, + default ValueEval evaluateOneArrayArg(ValueEval arg0, int srcRowIndex, int srcColumnIndex, java.util.function.Function evalFunc){ - ValueEval arg0 = args[0]; - int w1, w2, h1, h2; int a1FirstCol = 0, a1FirstRow = 0; if (arg0 instanceof AreaEval) { @@ -178,6 +188,12 @@ public interface ArrayFunction { vA = ErrorEval.NAME_INVALID; } catch (EvaluationException e) { vA = e.getErrorEval(); + } catch (RuntimeException e) { + if(e.getMessage().startsWith("Don't now how to evaluate name")){ + vA = ErrorEval.NAME_INVALID; + } else { + throw e; + } } vals[idx++] = evalFunc.apply(vA); } diff --git a/src/java/org/apache/poi/ss/formula/functions/BooleanFunction.java b/src/java/org/apache/poi/ss/formula/functions/BooleanFunction.java index 5d011e3b20..5b5b539fef 100644 --- a/src/java/org/apache/poi/ss/formula/functions/BooleanFunction.java +++ b/src/java/org/apache/poi/ss/formula/functions/BooleanFunction.java @@ -37,7 +37,7 @@ import org.apache.poi.ss.formula.eval.ValueEval; * * @author Amol S. Deshmukh < amolweb at ya hoo dot com > */ -public abstract class BooleanFunction implements Function { +public abstract class BooleanFunction implements Function,ArrayFunction { public final ValueEval evaluate(ValueEval[] args, int srcRow, int srcCol) { if (args.length < 1) { @@ -142,7 +142,20 @@ public abstract class BooleanFunction implements Function { return BoolEval.TRUE; } }; - public static final Function NOT = new Fixed1ArgFunction() { + + abstract static class Boolean1ArgFunction extends Fixed1ArgFunction implements ArrayFunction { + @Override + public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex) { + if (args.length != 1) { + return ErrorEval.VALUE_INVALID; + } + return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex, + vA -> evaluate(srcRowIndex, srcColumnIndex, vA)); + } + + } + + public static final Function NOT = new Boolean1ArgFunction() { public ValueEval evaluate(int srcRowIndex, int srcColumnIndex, ValueEval arg0) { boolean boolArgVal; try { @@ -156,4 +169,13 @@ public abstract class BooleanFunction implements Function { return BoolEval.valueOf(!boolArgVal); } }; -} + + @Override + public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex) { + if (args.length != 1) { + return ErrorEval.VALUE_INVALID; + } + return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex, + vA -> evaluate(new ValueEval[]{vA}, srcRowIndex, srcColumnIndex)); + } +} \ No newline at end of file diff --git a/src/java/org/apache/poi/ss/formula/functions/IfFunc.java b/src/java/org/apache/poi/ss/formula/functions/IfFunc.java index effff27815..7f92881353 100644 --- a/src/java/org/apache/poi/ss/formula/functions/IfFunc.java +++ b/src/java/org/apache/poi/ss/formula/functions/IfFunc.java @@ -17,10 +17,14 @@ package org.apache.poi.ss.formula.functions; +import org.apache.poi.ss.formula.CacheAreaEval; +import org.apache.poi.ss.formula.FormulaParseException; import org.apache.poi.ss.formula.eval.*; import org.apache.poi.ss.formula.ptg.Ptg; import org.apache.poi.ss.formula.ptg.RefPtg; +import java.util.function.BiFunction; + /** * Implementation for the Excel function IF *

@@ -84,25 +88,120 @@ public final class IfFunc extends Var2or3ArgFunction implements ArrayFunction { @Override public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex) { + if (args.length < 2 || args.length > 3) { + return ErrorEval.VALUE_INVALID; + } + ValueEval arg0 = args[0]; ValueEval arg1 = args[1]; - return evaluateTwoArrayArgs(arg0, arg1, srcRowIndex, srcColumnIndex, - (vA, vB) -> { + ValueEval arg2 = args.length == 2 ? BoolEval.FALSE : args[2]; + return evaluateArrayArgs(arg0, arg1, arg2, srcRowIndex, srcColumnIndex); + } + + ValueEval evaluateArrayArgs(ValueEval arg0, ValueEval arg1, ValueEval arg2, int srcRowIndex, int srcColumnIndex) { + int w1, w2, h1, h2; + int a1FirstCol = 0, a1FirstRow = 0; + if (arg0 instanceof AreaEval) { + AreaEval ae = (AreaEval)arg0; + w1 = ae.getWidth(); + h1 = ae.getHeight(); + a1FirstCol = ae.getFirstColumn(); + a1FirstRow = ae.getFirstRow(); + } else if (arg0 instanceof RefEval){ + RefEval ref = (RefEval)arg0; + w1 = 1; + h1 = 1; + a1FirstCol = ref.getColumn(); + a1FirstRow = ref.getRow(); + } else { + w1 = 1; + h1 = 1; + } + int a2FirstCol = 0, a2FirstRow = 0; + if (arg1 instanceof AreaEval) { + AreaEval ae = (AreaEval)arg1; + w2 = ae.getWidth(); + h2 = ae.getHeight(); + a2FirstCol = ae.getFirstColumn(); + a2FirstRow = ae.getFirstRow(); + } else if (arg1 instanceof RefEval){ + RefEval ref = (RefEval)arg1; + w2 = 1; + h2 = 1; + a2FirstCol = ref.getColumn(); + a2FirstRow = ref.getRow(); + } else { + w2 = 1; + h2 = 1; + } + + int a3FirstCol = 0, a3FirstRow = 0; + if (arg2 instanceof AreaEval) { + AreaEval ae = (AreaEval)arg2; + a3FirstCol = ae.getFirstColumn(); + a3FirstRow = ae.getFirstRow(); + } else if (arg2 instanceof RefEval){ + RefEval ref = (RefEval)arg2; + a3FirstCol = ref.getColumn(); + a3FirstRow = ref.getRow(); + } + + int width = Math.max(w1, w2); + int height = Math.max(h1, h2); + + ValueEval[] vals = new ValueEval[height * width]; + + int idx = 0; + for(int i = 0; i < height; i++){ + for(int j = 0; j < width; j++){ + ValueEval vA; + try { + vA = OperandResolver.getSingleValue(arg0, a1FirstRow + i, a1FirstCol + j); + } catch (FormulaParseException e) { + vA = ErrorEval.NAME_INVALID; + } catch (EvaluationException e) { + vA = e.getErrorEval(); + } + ValueEval vB; + try { + vB = OperandResolver.getSingleValue(arg1, a2FirstRow + i, a2FirstCol + j); + } catch (FormulaParseException e) { + vB = ErrorEval.NAME_INVALID; + } catch (EvaluationException e) { + vB = e.getErrorEval(); + } + + ValueEval vC; + try { + vC = OperandResolver.getSingleValue(arg2, a3FirstRow + i, a3FirstCol + j); + } catch (FormulaParseException e) { + vC = ErrorEval.NAME_INVALID; + } catch (EvaluationException e) { + vC = e.getErrorEval(); + } + + if(vA instanceof ErrorEval){ + vals[idx++] = vA; + } else if (vB instanceof ErrorEval) { + vals[idx++] = vB; + } else { Boolean b; try { b = OperandResolver.coerceValueToBoolean(vA, false); + vals[idx++] = b != null && b ? vB : vC; } catch (EvaluationException e) { - return e.getErrorEval(); - } - if (b != null && b) { - if (vB == MissingArgEval.instance) { - return BlankEval.instance; - } - return vB; + vals[idx++] = e.getErrorEval(); } - return BoolEval.FALSE; } - ); + + } + } + + if (vals.length == 1) { + return vals[0]; + } + + return new CacheAreaEval(srcRowIndex, srcColumnIndex, srcRowIndex + height - 1, srcColumnIndex + width - 1, vals); } } diff --git a/src/java/org/apache/poi/ss/formula/functions/LogicalFunction.java b/src/java/org/apache/poi/ss/formula/functions/LogicalFunction.java index 5cb7d25505..9df2803c3c 100644 --- a/src/java/org/apache/poi/ss/formula/functions/LogicalFunction.java +++ b/src/java/org/apache/poi/ss/formula/functions/LogicalFunction.java @@ -44,7 +44,10 @@ public abstract class LogicalFunction extends Fixed1ArgFunction implements Array @Override public ValueEval evaluateArray(ValueEval[] args, int srcRowIndex, int srcColumnIndex){ - return evaluateOneArrayArg(args, srcRowIndex, srcColumnIndex, (valA) -> + if (args.length != 1) { + return ErrorEval.VALUE_INVALID; + } + return evaluateOneArrayArg(args[0], srcRowIndex, srcColumnIndex, (valA) -> BoolEval.valueOf(evaluate(valA)) ); } diff --git a/src/testcases/org/apache/poi/ss/formula/functions/TestBooleanFunctionsFromSpreadsheet.java b/src/testcases/org/apache/poi/ss/formula/functions/TestBooleanFunctionsFromSpreadsheet.java new file mode 100644 index 0000000000..ef69a9c65b --- /dev/null +++ b/src/testcases/org/apache/poi/ss/formula/functions/TestBooleanFunctionsFromSpreadsheet.java @@ -0,0 +1,32 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.ss.formula.functions; + +import org.junit.runners.Parameterized.Parameters; + +import java.util.Collection; + +/** + * Tests boolean functions as loaded from a test data spreadsheet.

+ */ +public class TestBooleanFunctionsFromSpreadsheet extends BaseTestFunctionsFromSpreadsheet { + @Parameters(name="{0}") + public static Collection data() throws Exception { + return data(TestBooleanFunctionsFromSpreadsheet.class, "BooleanFunctionsTestCaseData.xls"); + } +} diff --git a/test-data/spreadsheet/BooleanFunctionsTestCaseData.xls b/test-data/spreadsheet/BooleanFunctionsTestCaseData.xls new file mode 100644 index 0000000000..74798eb567 Binary files /dev/null and b/test-data/spreadsheet/BooleanFunctionsTestCaseData.xls differ diff --git a/test-data/spreadsheet/IfFunctionTestCaseData.xls b/test-data/spreadsheet/IfFunctionTestCaseData.xls index 1d289d792c..fb648da613 100644 Binary files a/test-data/spreadsheet/IfFunctionTestCaseData.xls and b/test-data/spreadsheet/IfFunctionTestCaseData.xls differ diff --git a/test-data/spreadsheet/RowFunctionTestCaseData.xls b/test-data/spreadsheet/RowFunctionTestCaseData.xls new file mode 100644 index 0000000000..0579605609 Binary files /dev/null and b/test-data/spreadsheet/RowFunctionTestCaseData.xls differ