diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 86f2696b39..ea93e88316 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 51498 - fixed evaluation of blank cells in COUNTIF 52576 - support changing external file references in HSSFWorkbook 49896 - support external references in FormulaRenderer 52527 - avoid exception when matching shared formula records in HSSF diff --git a/src/java/org/apache/poi/ss/formula/functions/Countif.java b/src/java/org/apache/poi/ss/formula/functions/Countif.java index 173f6a1e77..ef524acde3 100644 --- a/src/java/org/apache/poi/ss/formula/functions/Countif.java +++ b/src/java/org/apache/poi/ss/formula/functions/Countif.java @@ -217,6 +217,14 @@ public final class Countif extends Fixed2ArgFunction { } else if((x instanceof NumberEval)) { NumberEval ne = (NumberEval) x; testValue = ne.getNumberValue(); + } else if((x instanceof BlankEval)) { + switch (getCode()) { + case CmpOp.NE: + // Excel counts blank values in range as not equal to any value. See Bugzilla 51498 + return true; + default: + return false; + } } else { return false; } @@ -258,7 +266,23 @@ public final class Countif extends Fixed2ArgFunction { } else if((x instanceof BoolEval)) { BoolEval be = (BoolEval) x; testValue = boolToInt(be.getBooleanValue()); - } else { + } else if((x instanceof BlankEval)) { + switch (getCode()) { + case CmpOp.NE: + // Excel counts blank values in range as not equal to any value. See Bugzilla 51498 + return true; + default: + return false; + } + } else if((x instanceof NumberEval)) { + switch (getCode()) { + case CmpOp.NE: + // not-equals comparison of a number to boolean always returnes false + return true; + default: + return false; + } + } else { return false; } return evaluate(testValue - _value); @@ -318,6 +342,10 @@ public final class Countif extends Fixed2ArgFunction { case CmpOp.NONE: case CmpOp.EQ: return _value.length() == 0; + case CmpOp.NE: + // pred '<>' matches empty string but not blank cell + // pred '<>ABC' matches blank and 'not ABC' + return _value.length() != 0; } // no other criteria matches a blank cell return false; @@ -342,7 +370,9 @@ public final class Countif extends Fixed2ArgFunction { if (_pattern != null) { return evaluate(_pattern.matcher(testedValue).matches()); } - return evaluate(testedValue.compareTo(_value)); + // String criteria in COUNTIF are case insensitive: + // for example, the string "apples" and the string "APPLES" will match the same cells. + return evaluate(testedValue.compareToIgnoreCase(_value)); } /** * Translates Excel countif wildcard strings into java regex strings @@ -394,7 +424,7 @@ public final class Countif extends Fixed2ArgFunction { sb.append(ch); } if (hasWildCard) { - return Pattern.compile(sb.toString()); + return Pattern.compile(sb.toString(), Pattern.CASE_INSENSITIVE); } return null; } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index 5499af4a3f..6f1fdcde2e 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -2220,7 +2220,9 @@ if(1==2) { public void test49896() { HSSFWorkbook wb = openSample("49896.xls"); HSSFCell cell = wb.getSheetAt(0).getRow(1).getCell(1); - assertEquals("VLOOKUP(A2,'[C:Documents and Settings/Yegor/My Documents/csco.xls]Sheet1'!$A$2:$B$3,2,FALSE)", + String PATH_SEPARATOR = System.getProperty("file.separator"); + assertEquals("VLOOKUP(A2,'[C:Documents and Settings" + PATH_SEPARATOR+"Yegor"+PATH_SEPARATOR + +"My Documents"+PATH_SEPARATOR+"csco.xls]Sheet1'!$A$2:$B$3,2,FALSE)", cell.getCellFormula()); } diff --git a/src/testcases/org/apache/poi/ss/formula/functions/TestCountFuncs.java b/src/testcases/org/apache/poi/ss/formula/functions/TestCountFuncs.java index a01a92aa00..40525480f5 100644 --- a/src/testcases/org/apache/poi/ss/formula/functions/TestCountFuncs.java +++ b/src/testcases/org/apache/poi/ss/formula/functions/TestCountFuncs.java @@ -40,6 +40,7 @@ import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.ss.usermodel.CellValue; import org.apache.poi.ss.usermodel.FormulaEvaluator; +import org.apache.poi.ss.util.CellReference; /** * Test cases for COUNT(), COUNTA() COUNTIF(), COUNTBLANK() @@ -196,6 +197,27 @@ public final class TestCountFuncs extends TestCase { confirmCountIf(4, range, new StringEval("<>111")); } + /** + * String criteria in COUNTIF are case insensitive; + * for example, the string "apples" and the string "APPLES" will match the same cells. + */ + public void testCaseInsensitiveStringComparison() { + AreaEval range; + ValueEval[] values; + + values = new ValueEval[] { + new StringEval("no"), + new StringEval("NO"), + new StringEval("No"), + new StringEval("Yes") + }; + + range = EvalFactory.createAreaEval("A1:A4", values); + confirmCountIf(3, range, new StringEval("no")); + confirmCountIf(3, range, new StringEval("NO")); + confirmCountIf(3, range, new StringEval("No")); + } + /** * special case where the criteria argument is a cell reference */ @@ -365,27 +387,48 @@ public final class TestCountFuncs extends TestCase { * Bug #51498 - Check that CountIf behaves correctly for GTE, LTE * and NEQ cases */ - public void testCountifLTEGTE() throws Exception { + public void testCountifBug51498() throws Exception { final int REF_COL = 4; final int EVAL_COL = 3; - // Note - POI currently agrees with OpenOffice on certain blank cell cases, - // while Excel can differ. This is the list of checks to skip - List skipRowsPendingExcelVsOpenOffice = Arrays.asList( - new Integer[] {3}); - - HSSFWorkbook workbook = HSSFTestDataSamples.openSampleWorkbook("51498.xls"); + HSSFWorkbook workbook = HSSFTestDataSamples.openSampleWorkbook("51498.xls"); FormulaEvaluator evaluator = workbook.getCreationHelper().createFormulaEvaluator(); HSSFSheet sheet = workbook.getSheetAt(0); - for (int i = 0; i < 8; i++) { - if (skipRowsPendingExcelVsOpenOffice.contains(i)) { - // Skip the check for now - continue; - } + + // numeric criteria + for (int i = 0; i < 8; i++) { CellValue expected = evaluator.evaluate(sheet.getRow(i).getCell(REF_COL)); CellValue actual = evaluator.evaluate(sheet.getRow(i).getCell(EVAL_COL)); assertEquals(expected.formatAsString(), actual.formatAsString()); } + + // boolean criteria + for (int i = 0; i < 8; i++) { + HSSFCell cellFmla = sheet.getRow(i).getCell(8); + HSSFCell cellRef = sheet.getRow(i).getCell(9); + + double expectedValue = cellRef.getNumericCellValue(); + double actualValue = evaluator.evaluate(cellFmla).getNumberValue(); + + assertEquals( + "Problem with a formula at " + + new CellReference(cellFmla).formatAsString() + "[" + cellFmla.getCellFormula()+"] ", + expectedValue, actualValue, 0.0001); + } + + // string criteria + for (int i = 1; i < 9; i++) { + HSSFCell cellFmla = sheet.getRow(i).getCell(13); + HSSFCell cellRef = sheet.getRow(i).getCell(14); + + double expectedValue = cellRef.getNumericCellValue(); + double actualValue = evaluator.evaluate(cellFmla).getNumberValue(); + + assertEquals( + "Problem with a formula at " + + new CellReference(cellFmla).formatAsString() + "[" + cellFmla.getCellFormula()+"] ", + expectedValue, actualValue, 0.0001); + } } public void testWildCards() { @@ -456,6 +499,53 @@ public final class TestCountFuncs extends TestCase { testCountFunctionFromSpreadsheet("countifExamples.xls", 1, 2, 3, "countif"); } + /** + * Two COUNTIF examples taken from + * http://office.microsoft.com/en-us/excel-help/countif-function-HP010069840.aspx?CTT=5&origin=HA010277524 + */ + public void testCountifExamples() { + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("countifExamples.xls"); + HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb); + + HSSFSheet sheet1 = wb.getSheet("MSDN Example 1"); + for (int rowIx=7; rowIx<=12; rowIx++) { + HSSFRow row = sheet1.getRow(rowIx-1); + HSSFCell cellA = row.getCell(0); // cell containing a formula with COUNTIF + assertEquals(HSSFCell.CELL_TYPE_FORMULA, cellA.getCellType()); + HSSFCell cellC = row.getCell(2); // cell with a reference value + assertEquals(HSSFCell.CELL_TYPE_NUMERIC, cellC.getCellType()); + + CellValue cv = fe.evaluate(cellA); + double actualValue = cv.getNumberValue(); + double expectedValue = cellC.getNumericCellValue(); + assertEquals( + "Problem with a formula at " + new CellReference(cellA).formatAsString() + + ": " + cellA.getCellFormula() + " :" + + "Expected = (" + expectedValue + ") Actual=(" + actualValue + ") ", + expectedValue, actualValue, 0.0001); + } + + HSSFSheet sheet2 = wb.getSheet("MSDN Example 2"); + for (int rowIx=9; rowIx<=14; rowIx++) { + HSSFRow row = sheet2.getRow(rowIx-1); + HSSFCell cellA = row.getCell(0); // cell containing a formula with COUNTIF + assertEquals(HSSFCell.CELL_TYPE_FORMULA, cellA.getCellType()); + HSSFCell cellC = row.getCell(2); // cell with a reference value + assertEquals(HSSFCell.CELL_TYPE_NUMERIC, cellC.getCellType()); + + CellValue cv = fe.evaluate(cellA); + double actualValue = cv.getNumberValue(); + double expectedValue = cellC.getNumericCellValue(); + + assertEquals( + "Problem with a formula at " + + new CellReference(cellA).formatAsString() + "[" + cellA.getCellFormula()+"]: " + + "Expected = (" + expectedValue + ") Actual=(" + actualValue + ") ", + expectedValue, actualValue, 0.0001); + + } + } + public void testCountBlankFromSpreadsheet() { testCountFunctionFromSpreadsheet("countblankExamples.xls", 1, 3, 4, "countblank"); } diff --git a/test-data/spreadsheet/51498.xls b/test-data/spreadsheet/51498.xls index fc50d21d2c..b5d2d65999 100644 Binary files a/test-data/spreadsheet/51498.xls and b/test-data/spreadsheet/51498.xls differ diff --git a/test-data/spreadsheet/countifExamples.xls b/test-data/spreadsheet/countifExamples.xls index b15bd162a2..6285423c0b 100644 Binary files a/test-data/spreadsheet/countifExamples.xls and b/test-data/spreadsheet/countifExamples.xls differ