]> source.dussan.org Git - poi.git/commitdiff
follow-up to Bug 62904. More tests and improved evaluation of IF in array mode
authorYegor Kozlov <yegor@apache.org>
Mon, 14 Jan 2019 14:48:21 +0000 (14:48 +0000)
committerYegor Kozlov <yegor@apache.org>
Mon, 14 Jan 2019 14:48:21 +0000 (14:48 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1851263 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
src/java/org/apache/poi/ss/formula/eval/UnaryMinusEval.java
src/java/org/apache/poi/ss/formula/eval/UnaryPlusEval.java
src/java/org/apache/poi/ss/formula/functions/ArrayFunction.java
src/java/org/apache/poi/ss/formula/functions/BooleanFunction.java
src/java/org/apache/poi/ss/formula/functions/IfFunc.java
src/java/org/apache/poi/ss/formula/functions/LogicalFunction.java
src/testcases/org/apache/poi/ss/formula/functions/TestBooleanFunctionsFromSpreadsheet.java [new file with mode: 0644]
test-data/spreadsheet/BooleanFunctionsTestCaseData.xls [new file with mode: 0644]
test-data/spreadsheet/IfFunctionTestCaseData.xls
test-data/spreadsheet/RowFunctionTestCaseData.xls [new file with mode: 0644]

index e91c0e109528eb53cf34df818b18470a9264a211..bab3e4e66eaff3c138c780fa852759023e07ee00 100644 (file)
@@ -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<ValueEval> 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) {
index ac1ee34118090bd86e53628b94ff1abff785a77c..8107c48544e416b5e353424b95d67e377e17c342 100644 (file)
@@ -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)
                );
        }
index d5cb586a85f57d632843d132329e6060d0eb0bb9..81b36aad407c676b0fbfeaf4859d11dbff8e4649 100644 (file)
@@ -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)
                );
        }
index 088d7b34fdcf603ae17c1272cc8614c44273aca2..7f8a359d3a0c762826f0edbe4839c596f54084d5 100644 (file)
@@ -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<ValueEval, ValueEval> 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);
             }
index 5d011e3b208312b163f38ee9c1f65a7690f244b3..5b5b539feffc423126927e159ad4872f72ef4df9 100644 (file)
@@ -37,7 +37,7 @@ import org.apache.poi.ss.formula.eval.ValueEval;
  *
  * @author Amol S. Deshmukh &lt; amolweb at ya hoo dot com &gt;
  */
-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
index effff27815e295c2a6ce7ddee6d4e6965a8a8e24..7f928813536394a8b5e88c66aeeedfc308e19c6d 100644 (file)
 
 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
  * <p>
@@ -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);
        }
 
 }
index 5cb7d25505a8c029c8fd4729bdb8ab7edc2657b5..9df2803c3c9d9f4086a54f71ccec889410a85791 100644 (file)
@@ -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 (file)
index 0000000..ef69a9c
--- /dev/null
@@ -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.<p>
+ */
+public class TestBooleanFunctionsFromSpreadsheet extends BaseTestFunctionsFromSpreadsheet {
+    @Parameters(name="{0}")
+    public static Collection<Object[]> 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 (file)
index 0000000..74798eb
Binary files /dev/null and b/test-data/spreadsheet/BooleanFunctionsTestCaseData.xls differ
index 1d289d792ca951c88466b6c1be5b34e2cebb5e4c..fb648da613214dcb1cf64b3a01eb22a6471468c5 100644 (file)
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 (file)
index 0000000..0579605
Binary files /dev/null and b/test-data/spreadsheet/RowFunctionTestCaseData.xls differ