Bug 56655: Fix Sumifs for cases where the criteria is in error.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1686610 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/ss/formula/functions/Countif.java

@@ -30,7 +30,7 @@ import org.apache.poi.ss.formula.eval.RefEval;
 import org.apache.poi.ss.formula.eval.StringEval;
 import org.apache.poi.ss.formula.eval.ValueEval;
 import org.apache.poi.ss.formula.functions.CountUtils.I_MatchPredicate;
-import org.apache.poi.ss.usermodel.ErrorConstants;
+import org.apache.poi.ss.usermodel.FormulaError;
 
 /**
  * Implementation for the function COUNTIF
@@ -254,6 +254,7 @@ public final class Countif extends Fixed2ArgFunction {
 					// boolean values when the target(x) is a string
 					return false;
 				}
+				@SuppressWarnings("unused")
 				StringEval se = (StringEval)x;
 				Boolean val = parseBoolean(se.getStringValue());
 				if(val == null) {
@@ -286,7 +287,7 @@ public final class Countif extends Fixed2ArgFunction {
 			return evaluate(testValue - _value);
 		}
 	}
-	private static final class ErrorMatcher extends MatcherBase {
+	public static final class ErrorMatcher extends MatcherBase {
 
 		private final int _value;
 
@@ -296,7 +297,7 @@ public final class Countif extends Fixed2ArgFunction {
 		}
 		@Override
 		protected String getValueText() {
-			return ErrorConstants.getText(_value);
+			return FormulaError.forInt(_value).getString();
 		}
 
 		public boolean matches(ValueEval x) {
@@ -306,6 +307,10 @@ public final class Countif extends Fixed2ArgFunction {
 			}
 			return false;
 		}
+		
+		public int getValue() {
+		    return _value;
+		}
 	}
 	public static final class StringMatcher extends MatcherBase {
 

src/java/org/apache/poi/ss/formula/functions/Sumifs.java

@@ -20,8 +20,14 @@
 package org.apache.poi.ss.formula.functions;
 
 import org.apache.poi.ss.formula.OperationEvaluationContext;
-import org.apache.poi.ss.formula.eval.*;
+import org.apache.poi.ss.formula.eval.AreaEval;
+import org.apache.poi.ss.formula.eval.ErrorEval;
+import org.apache.poi.ss.formula.eval.EvaluationException;
+import org.apache.poi.ss.formula.eval.NumberEval;
+import org.apache.poi.ss.formula.eval.RefEval;
+import org.apache.poi.ss.formula.eval.ValueEval;
 import org.apache.poi.ss.formula.functions.CountUtils.I_MatchPredicate;
+import org.apache.poi.ss.formula.functions.Countif.ErrorMatcher;
 
 /**
  * Implementation for the Excel function SUMIFS<p>
@@ -60,10 +66,12 @@ public final class Sumifs implements FreeRefFunction {
             I_MatchPredicate[] mp = new I_MatchPredicate[ae.length];
             for(int i = 1, k=0; i < args.length; i += 2, k++){
                 ae[k] = convertRangeArg(args[i]);
+                
                 mp[k] = Countif.createCriteriaPredicate(args[i+1], ec.getRowIndex(), ec.getColumnIndex());
             }
 
             validateCriteriaRanges(ae, sumRange);
+            validateCriteria(mp);
 
             double result = sumMatchingCells(ae, mp, sumRange);
             return new NumberEval(result);
@@ -76,7 +84,7 @@ public final class Sumifs implements FreeRefFunction {
      * Verify that each <code>criteriaRanges</code> argument contains the same number of rows and columns
      * as the <code>sumRange</code> argument
      *
-     * @throws EvaluationException if
+     * @throws EvaluationException if the ranges do not match.
      */
     private void validateCriteriaRanges(AreaEval[] criteriaRanges, AreaEval sumRange) throws EvaluationException {
         for(AreaEval r : criteriaRanges){
@@ -87,6 +95,22 @@ public final class Sumifs implements FreeRefFunction {
         }
     }
 
+    /**
+     * Verify that each <code>criteria</code> predicate is valid, i.e. not an error
+     *
+     * @throws EvaluationException if there are criteria which resulted in Errors.
+     */
+    private void validateCriteria(I_MatchPredicate[] criteria) throws EvaluationException {
+        for(I_MatchPredicate predicate : criteria) {
+            
+            // check for errors in predicate and return immediately using this error code
+            if(predicate instanceof ErrorMatcher) {
+                throw new EvaluationException(ErrorEval.valueOf(((ErrorMatcher)predicate).getValue()));
+            }
+        }
+    }
+
+
     /**
      *
      * @param ranges  criteria ranges, each range must be of the same dimensions as <code>aeSum</code>

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java

@@ -34,7 +34,6 @@ import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.CellStyle;
 import org.apache.poi.ss.usermodel.DataFormatter;
 import org.apache.poi.ss.usermodel.DateUtil;
-import org.apache.poi.ss.usermodel.FormulaError;
 import org.apache.poi.ss.usermodel.RichTextString;
 import org.apache.poi.ss.usermodel.Row;
 import org.apache.poi.ss.usermodel.Sheet;
@@ -219,68 +218,6 @@ public final class TestUnfixedBugs extends TestCase {
         assertEquals(4, strBack.getIndexOfFormattingRun(2));
     }
 
-    @Test
-    public void testBug56655() {
-        XSSFWorkbook wb =  new XSSFWorkbook();
-        Sheet sheet = wb.createSheet();
-        
-        setCellFormula(sheet, 0, 0, "#VALUE!");
-        setCellFormula(sheet, 0, 1, "SUMIFS(A:A,A:A,#VALUE!)");
-
-        XSSFFormulaEvaluator.evaluateAllFormulaCells(wb);
-
-        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 0,0).getCachedFormulaResultType());
-        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 0,0).getErrorCellValue());
-        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 0,1).getCachedFormulaResultType());
-        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 0,1).getErrorCellValue());
-    }
-
-    @Test
-    public void testBug56655a() {
-        XSSFWorkbook wb =  new XSSFWorkbook();
-        Sheet sheet = wb.createSheet();
-        
-        setCellFormula(sheet, 0, 0, "B1*C1");
-        sheet.getRow(0).createCell(1).setCellValue("A");
-        setCellFormula(sheet, 1, 0, "B1*C1");
-        sheet.getRow(1).createCell(1).setCellValue("A");
-        setCellFormula(sheet, 0, 3, "SUMIFS(A:A,A:A,A2)");
-
-        XSSFFormulaEvaluator.evaluateAllFormulaCells(wb);
-
-        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 0, 0).getCachedFormulaResultType());
-        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 0, 0).getErrorCellValue());
-        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 1, 0).getCachedFormulaResultType());
-        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 1, 0).getErrorCellValue());
-        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 0, 3).getCachedFormulaResultType());
-        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 0, 3).getErrorCellValue());
-    }
-
-    /**
-    * @param row 0-based
-    * @param column 0-based
-    */
-   private void setCellFormula(Sheet sheet, int row, int column, String formula) {
-       Row r = sheet.getRow(row);
-       if (r == null) {
-           r = sheet.createRow(row);
-       }
-       Cell cell = r.getCell(column);
-       if (cell == null) {
-           cell = r.createCell(column);
-       }
-       cell.setCellType(XSSFCell.CELL_TYPE_FORMULA);
-       cell.setCellFormula(formula);
-   }
-
-   /**
-    * @param rowNo 0-based
-    * @param column 0-based
-    */
-   private Cell getCell(Sheet sheet, int rowNo, int column) {
-       return sheet.getRow(rowNo).getCell(column);
-   }
-
    @Test
    public void testBug55752() throws IOException {
        Workbook wb = new XSSFWorkbook();

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java

@@ -25,6 +25,7 @@ import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.ss.usermodel.BaseTestFormulaEvaluator;
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.CellValue;
+import org.apache.poi.ss.usermodel.FormulaError;
 import org.apache.poi.ss.usermodel.FormulaEvaluator;
 import org.apache.poi.ss.usermodel.Row;
 import org.apache.poi.ss.usermodel.Sheet;
@@ -186,6 +187,7 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator {
         
         // Link and re-try
         Workbook alt = new XSSFWorkbook();
+        try {
             alt.createSheet().createRow(0).createCell(0).setCellValue("In another workbook");
             // TODO Implement the rest of this, see bug #57184
 /*
@@ -208,6 +210,9 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator {
             evaluator.evaluate(cXSLX_nw_cell);
             assertEquals("In another workbook", cXSLX_nw_cell.getStringCellValue());
 */
+        } finally {
+        	alt.close();
+        }
     }
     
     /**
@@ -519,7 +524,6 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator {
         }
     }
     
-
     public void testBug55843f() throws IOException {
         XSSFWorkbook wb = new XSSFWorkbook();
         try {
@@ -539,4 +543,68 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator {
             wb.close();
         }
     }    
+
+    public void testBug56655() throws IOException {
+        Workbook wb =  new XSSFWorkbook();
+        Sheet sheet = wb.createSheet();
+        
+        setCellFormula(sheet, 0, 0, "#VALUE!");
+        setCellFormula(sheet, 0, 1, "SUMIFS(A:A,A:A,#VALUE!)");
+
+        wb.getCreationHelper().createFormulaEvaluator().evaluateAll();
+
+        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 0,0).getCachedFormulaResultType());
+        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 0,0).getErrorCellValue());
+        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 0,1).getCachedFormulaResultType());
+        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 0,1).getErrorCellValue());
+        
+        wb.close();
+    }
+
+    public void testBug56655a() throws IOException {
+        Workbook wb =  new XSSFWorkbook();
+        Sheet sheet = wb.createSheet();
+        
+        setCellFormula(sheet, 0, 0, "B1*C1");
+        sheet.getRow(0).createCell(1).setCellValue("A");
+        setCellFormula(sheet, 1, 0, "B1*C1");
+        sheet.getRow(1).createCell(1).setCellValue("A");
+        setCellFormula(sheet, 0, 3, "SUMIFS(A:A,A:A,A2)");
+
+        wb.getCreationHelper().createFormulaEvaluator().evaluateAll();
+
+        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 0, 0).getCachedFormulaResultType());
+        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 0, 0).getErrorCellValue());
+        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 1, 0).getCachedFormulaResultType());
+        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 1, 0).getErrorCellValue());
+        assertEquals(XSSFCell.CELL_TYPE_ERROR, getCell(sheet, 0, 3).getCachedFormulaResultType());
+        assertEquals(FormulaError.VALUE.getCode(), getCell(sheet, 0, 3).getErrorCellValue());
+        
+        wb.close();
+    }
+
+    /**
+    * @param row 0-based
+    * @param column 0-based
+    */
+   private void setCellFormula(Sheet sheet, int row, int column, String formula) {
+       Row r = sheet.getRow(row);
+       if (r == null) {
+           r = sheet.createRow(row);
+       }
+       Cell cell = r.getCell(column);
+       if (cell == null) {
+           cell = r.createCell(column);
+       }
+       cell.setCellType(XSSFCell.CELL_TYPE_FORMULA);
+       cell.setCellFormula(formula);
+   }
+
+   /**
+    * @param rowNo 0-based
+    * @param column 0-based
+    */
+   private Cell getCell(Sheet sheet, int rowNo, int column) {
+       return sheet.getRow(rowNo).getCell(column);
+   }
 }

src/testcases/org/apache/poi/ss/formula/functions/TestSumifs.java

@@ -21,6 +21,7 @@ package org.apache.poi.ss.formula.functions;
 
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
+
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.usermodel.*;
 import org.apache.poi.ss.formula.OperationEvaluationContext;
@@ -264,4 +265,84 @@ public final class TestSumifs extends TestCase {
         assertEquals(625000., ex5cell.getNumericCellValue());
 
     }
+    
+    public void testBug56655() {
+        ValueEval[] a2a9 = new ValueEval[] {
+                new NumberEval(5),
+                new NumberEval(4),
+                new NumberEval(15),
+                new NumberEval(3),
+                new NumberEval(22),
+                new NumberEval(12),
+                new NumberEval(10),
+                new NumberEval(33)
+        };
+
+        ValueEval[] args = new ValueEval[]{
+                EvalFactory.createAreaEval("A2:A9", a2a9),
+                ErrorEval.VALUE_INVALID,
+                new StringEval("A*"),
+        };
+        
+        ValueEval result = invokeSumifs(args, EC);
+        assertTrue("Expect to have an error when an input is an invalid value, but had: " + result.getClass(), result instanceof ErrorEval);
+
+        args = new ValueEval[]{
+                EvalFactory.createAreaEval("A2:A9", a2a9),
+                EvalFactory.createAreaEval("A2:A9", a2a9),
+                ErrorEval.VALUE_INVALID,
+        };
+        
+        result = invokeSumifs(args, EC);
+        assertTrue("Expect to have an error when an input is an invalid value, but had: " + result.getClass(), result instanceof ErrorEval);
+    }
+
+    public void testBug56655b() {
+/*
+        setCellFormula(sheet, 0, 0, "B1*C1");
+        sheet.getRow(0).createCell(1).setCellValue("A");
+        setCellFormula(sheet, 1, 0, "B1*C1");
+        sheet.getRow(1).createCell(1).setCellValue("A");
+        setCellFormula(sheet, 0, 3, "SUMIFS(A:A,A:A,A2)");
+ */
+    	ValueEval[] a0a1 = new ValueEval[] {
+                NumberEval.ZERO,
+                NumberEval.ZERO
+        };
+
+        ValueEval[] args = new ValueEval[]{
+                EvalFactory.createAreaEval("A0:A1", a0a1),
+                EvalFactory.createAreaEval("A0:A1", a0a1),
+                ErrorEval.VALUE_INVALID
+        };
+        
+        ValueEval result = invokeSumifs(args, EC);
+        assertTrue("Expect to have an error when an input is an invalid value, but had: " + result.getClass(), result instanceof ErrorEval);
+        assertEquals(ErrorEval.VALUE_INVALID, result);
+    }
+
+
+    public void testBug56655c() {
+/*
+        setCellFormula(sheet, 0, 0, "B1*C1");
+        sheet.getRow(0).createCell(1).setCellValue("A");
+        setCellFormula(sheet, 1, 0, "B1*C1");
+        sheet.getRow(1).createCell(1).setCellValue("A");
+        setCellFormula(sheet, 0, 3, "SUMIFS(A:A,A:A,A2)");
+ */
+        ValueEval[] a0a1 = new ValueEval[] {
+                NumberEval.ZERO,
+                NumberEval.ZERO
+        };
+
+        ValueEval[] args = new ValueEval[]{
+                EvalFactory.createAreaEval("A0:A1", a0a1),
+                EvalFactory.createAreaEval("A0:A1", a0a1),
+                ErrorEval.NAME_INVALID
+        };
+        
+        ValueEval result = invokeSumifs(args, EC);
+        assertTrue("Expect to have an error when an input is an invalid value, but had: " + result.getClass(), result instanceof ErrorEval);
+        assertEquals(ErrorEval.NAME_INVALID, result);
+    }
 }