Apache
/
poi
mirror of https://github.com/apache/poi.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Bug #56822 fix COUNTIFS() 

includes unit test from the issue.  Verified unit test results in Excel vs. incorrect previous POI results.  Test passes new code, as do existing tests.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1783037 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Greg Woolsey 2017-02-14 22:05:49 +00:00
parent 3c2c45daa7
commit 161facc089
5 changed files with 263 additions and 163 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

183
src/java/org/apache/poi/ss/formula/functions/Baseifs.java Normal file
View File

@ -0,0 +1,183 @@
/*
* ====================================================================
* 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.apache.poi.ss.formula.OperationEvaluationContext;
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;
/**
* Base class for SUMIFS() and COUNTIFS() functions, as they share much of the same logic,
* the difference being the source of the totals.
*/
/*package*/ abstract class Baseifs implements FreeRefFunction {
/**
* Implementations must be stateless.
* @return true if there should be a range argument before the criteria pairs
*/
protected abstract boolean hasInitialRange();
public ValueEval evaluate(ValueEval[] args, OperationEvaluationContext ec) {
final boolean hasInitialRange = hasInitialRange();
final int firstCriteria = hasInitialRange ? 1 : 0;
if( args.length < (2+firstCriteria) || args.length % 2 != firstCriteria ) {
return ErrorEval.VALUE_INVALID;
}
try {
AreaEval sumRange = null;
if (hasInitialRange) {
sumRange = convertRangeArg(args[0]);
}
// collect pairs of ranges and criteria
AreaEval[] ae = new AreaEval[(args.length - firstCriteria)/2];
I_MatchPredicate[] mp = new I_MatchPredicate[ae.length];
for(int i = firstCriteria, 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(sumRange, ae);
validateCriteria(mp);
double result = aggregateMatchingCells(sumRange, ae, mp);
return new NumberEval(result);
} catch (EvaluationException e) {
return e.getErrorEval();
}
}
/**
* Verify that each <code>criteriaRanges</code> argument contains the same number of rows and columns
* including the <code>sumRange</code> argument if present
* @param sumRange if used, it must match the shape of the criteriaRanges
* @param criteriaRanges to check
* @throws EvaluationException if the ranges do not match.
*/
private static void validateCriteriaRanges(AreaEval sumRange, AreaEval[] criteriaRanges) throws EvaluationException {
int h = criteriaRanges[0].getHeight();
int w = criteriaRanges[0].getWidth();
if (sumRange != null
&& (sumRange.getHeight() != h
|| sumRange.getWidth() != w) ) {
throw EvaluationException.invalidValue();
}
for(AreaEval r : criteriaRanges){
if(r.getHeight() != h ||
r.getWidth() != w ) {
throw EvaluationException.invalidValue();
}
}
}
/**
* Verify that each <code>criteria</code> predicate is valid, i.e. not an error
* @param criteria to check
*
* @throws EvaluationException if there are criteria which resulted in Errors.
*/
private static 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 sumRange the range to sum, if used (uses 1 for each match if not present)
* @param ranges criteria ranges
* @param predicates array of predicates, a predicate for each value in <code>ranges</code>
* @return the computed value
*/
private static double aggregateMatchingCells(AreaEval sumRange, AreaEval[] ranges, I_MatchPredicate[] predicates) {
int height = ranges[0].getHeight();
int width = ranges[0].getWidth();
double result = 0.0;
for (int r = 0; r < height; r++) {
for (int c = 0; c < width; c++) {
boolean matches = true;
for(int i = 0; i < ranges.length; i++){
AreaEval aeRange = ranges[i];
I_MatchPredicate mp = predicates[i];
if (!mp.matches(aeRange.getRelativeValue(r, c))) {
matches = false;
break;
}
}
if(matches) { // sum only if all of the corresponding criteria specified are true for that cell.
result += accumulate(sumRange, r, c);
}
}
}
return result;
}
/**
* For counts, this would return 1, for sums it returns a cell value or zero.
* This is only called after all the criteria are confirmed true for the coordinates.
* @param sumRange if used
* @param relRowIndex
* @param relColIndex
* @return the aggregate input value corresponding to the given range coordinates
*/
private static double accumulate(AreaEval sumRange, int relRowIndex, int relColIndex) {
if (sumRange == null) return 1.0; // count
ValueEval addend = sumRange.getRelativeValue(relRowIndex, relColIndex);
if (addend instanceof NumberEval) {
return ((NumberEval)addend).getNumberValue();
}
// everything else (including string and boolean values) counts as zero
return 0.0;
}
protected static AreaEval convertRangeArg(ValueEval eval) throws EvaluationException {
if (eval instanceof AreaEval) {
return (AreaEval) eval;
}
if (eval instanceof RefEval) {
return ((RefEval)eval).offset(0, 0, 0, 0);
}
throw new EvaluationException(ErrorEval.VALUE_INVALID);
}
}

42
src/java/org/apache/poi/ss/formula/functions/Countifs.java
View File

@ -18,11 +18,6 @@
package org.apache.poi.ss.formula.functions;
import org.apache.poi.ss.formula.OperationEvaluationContext;
import org.apache.poi.ss.formula.eval.ErrorEval;
import org.apache.poi.ss.formula.eval.NumberEval;
import org.apache.poi.ss.formula.eval.ValueEval;
/**
* Implementation for the function COUNTIFS
* <p>
@ -30,33 +25,20 @@ import org.apache.poi.ss.formula.eval.ValueEval;
* </p>
*/
public class Countifs implements FreeRefFunction {
public class Countifs extends Baseifs {
/**
* Singleton
*/
public static final FreeRefFunction instance = new Countifs();
public ValueEval evaluate(ValueEval[] args, OperationEvaluationContext ec) {
// https://support.office.com/en-us/article/COUNTIFS-function-dda3dc6e-f74e-4aee-88bc-aa8c2a866842?ui=en-US&rs=en-US&ad=US
// COUNTIFS(criteria_range1, criteria1, [criteria_range2, criteria2]...)
// need at least 2 arguments and need to have an even number of arguments (criteria_range1, criteria1 plus x*(criteria_range, criteria))
if (args.length < 2 || args.length % 2 != 0) {
return ErrorEval.VALUE_INVALID;
}
Double result = null;
// for each (criteria_range, criteria) pair
for (int i = 0; i < args.length; i += 2) {
ValueEval firstArg = args[i];
ValueEval secondArg = args[i + 1];
NumberEval evaluate = (NumberEval) new Countif().evaluate(
new ValueEval[] {firstArg, secondArg},
ec.getRowIndex(),
ec.getColumnIndex());
if (result == null) {
result = evaluate.getNumberValue();
} else if (evaluate.getNumberValue() < result) {
result = evaluate.getNumberValue();
}
}
return new NumberEval(result == null ? 0 : result);
/**
* https://support.office.com/en-us/article/COUNTIFS-function-dda3dc6e-f74e-4aee-88bc-aa8c2a866842?ui=en-US&rs=en-US&ad=US
* COUNTIFS(criteria_range1, criteria1, [criteria_range2, criteria2]...)
* need at least 2 arguments and need to have an even number of arguments (criteria_range1, criteria1 plus x*(criteria_range, criteria))
* @see org.apache.poi.ss.formula.functions.Baseifs#hasInitialRange()
*/
protected boolean hasInitialRange() {
return false;
}
}

138
src/java/org/apache/poi/ss/formula/functions/Sumifs.java
View File

@ -19,16 +19,6 @@
package org.apache.poi.ss.formula.functions;
import org.apache.poi.ss.formula.OperationEvaluationContext;
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>
*
@ -48,127 +38,21 @@ import org.apache.poi.ss.formula.functions.Countif.ErrorMatcher;
* </ul>
* </p>
*
* @author Yegor Kozlov
*/
public final class Sumifs implements FreeRefFunction {
public final class Sumifs extends Baseifs {
/**
* Singleton
*/
public static final FreeRefFunction instance = new Sumifs();
public ValueEval evaluate(ValueEval[] args, OperationEvaluationContext ec) {
// https://support.office.com/en-us/article/SUMIFS-function-c9e748f5-7ea7-455d-9406-611cebce642b
// COUNTIFS(sum_range, criteria_range1, criteria1, [criteria_range2, criteria2], ...
// need at least 3 arguments and need to have an odd number of arguments (sum-range plus x*(criteria_range, criteria))
if(args.length < 3 || args.length % 2 == 0) {
return ErrorEval.VALUE_INVALID;
}
try {
AreaEval sumRange = convertRangeArg(args[0]);
// collect pairs of ranges and criteria
AreaEval[] ae = new AreaEval[(args.length - 1)/2];
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);
} catch (EvaluationException e) {
return e.getErrorEval();
}
}
/**
* Verify that each <code>criteriaRanges</code> argument contains the same number of rows and columns
* as the <code>sumRange</code> argument
*
* @throws EvaluationException if the ranges do not match.
* https://support.office.com/en-us/article/SUMIFS-function-c9e748f5-7ea7-455d-9406-611cebce642b
* COUNTIFS(sum_range, criteria_range1, criteria1, [criteria_range2, criteria2], ...
* need at least 3 arguments and need to have an odd number of arguments (sum-range plus x*(criteria_range, criteria))
* @see org.apache.poi.ss.formula.functions.Baseifs#hasInitialRange()
*/
private void validateCriteriaRanges(AreaEval[] criteriaRanges, AreaEval sumRange) throws EvaluationException {
for(AreaEval r : criteriaRanges){
if(r.getHeight() != sumRange.getHeight() ||
r.getWidth() != sumRange.getWidth() ) {
throw EvaluationException.invalidValue();
@Override
protected boolean hasInitialRange() {
return true;
}
}
}
/**
* 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>
* @param predicates array of predicates, a predicate for each value in <code>ranges</code>
* @param aeSum the range to sum
*
* @return the computed value
*/
private static double sumMatchingCells(AreaEval[] ranges, I_MatchPredicate[] predicates, AreaEval aeSum) {
int height = aeSum.getHeight();
int width = aeSum.getWidth();
double result = 0.0;
for (int r = 0; r < height; r++) {
for (int c = 0; c < width; c++) {
boolean matches = true;
for(int i = 0; i < ranges.length; i++){
AreaEval aeRange = ranges[i];
I_MatchPredicate mp = predicates[i];
if (!mp.matches(aeRange.getRelativeValue(r, c))) {
matches = false;
break;
}
}
if(matches) { // sum only if all of the corresponding criteria specified are true for that cell.
result += accumulate(aeSum, r, c);
}
}
}
return result;
}
private static double accumulate(AreaEval aeSum, int relRowIndex,
int relColIndex) {
ValueEval addend = aeSum.getRelativeValue(relRowIndex, relColIndex);
if (addend instanceof NumberEval) {
return ((NumberEval)addend).getNumberValue();
}
// everything else (including string and boolean values) counts as zero
return 0.0;
}
private static AreaEval convertRangeArg(ValueEval eval) throws EvaluationException {
if (eval instanceof AreaEval) {
return (AreaEval) eval;
}
if (eval instanceof RefEval) {
return ((RefEval)eval).offset(0, 0, 0, 0);
}
throw new EvaluationException(ErrorEval.VALUE_INVALID);
}
}

61
src/testcases/org/apache/poi/ss/formula/functions/CountifsTests.java → src/ooxml/testcases/org/apache/poi/ss/formula/functions/CountifsTests.java
View File

@ -18,6 +18,9 @@
package org.apache.poi.ss.formula.functions;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.ss.usermodel.Cell;
import org.apache.poi.ss.usermodel.CellType;
@ -25,13 +28,44 @@ import org.apache.poi.ss.usermodel.CellValue;
import org.apache.poi.ss.usermodel.FormulaEvaluator;
import org.apache.poi.ss.usermodel.Row;
import org.apache.poi.ss.usermodel.Sheet;
import org.apache.poi.ss.usermodel.Workbook;
import org.apache.poi.ss.util.SheetUtil;
import org.apache.poi.util.IOUtils;
import org.apache.poi.xssf.XSSFTestDataSamples;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import junit.framework.TestCase;
/**
* Test the COUNTIFS() function
*/
public class CountifsTests {
public class CountifsTests extends TestCase {
private Workbook workbook;
/**
* initialize a workbook
*/
@Before
public void before() {
// not sure why we allow this, COUNTIFS() is only available
// in OOXML, it was introduced with Office 2007
workbook = new HSSFWorkbook();
}
/**
* Close the workbook if needed
*/
@After
public void after() {
IOUtils.closeQuietly(workbook);
}
/**
* Basic call
*/
@Test
public void testCallFunction() {
HSSFWorkbook workbook = new HSSFWorkbook();
Sheet sheet = workbook.createSheet("test");
Row row1 = sheet.createRow(0);
Cell cellA1 = row1.createCell(0, CellType.FORMULA);
@ -47,11 +81,14 @@ public class CountifsTests extends TestCase {
cellA1.setCellFormula("COUNTIFS(B1:C1,1, D1:E1,2)");
FormulaEvaluator evaluator = workbook.getCreationHelper().createFormulaEvaluator();
CellValue evaluate = evaluator.evaluate(cellA1);
assertEquals(1.0d, evaluate.getNumberValue());
assertEquals(1.0d, evaluate.getNumberValue(), 0.000000000000001);
}
/**
* Test argument count check
*/
@Test
public void testCallFunction_invalidArgs() {
HSSFWorkbook workbook = new HSSFWorkbook();
Sheet sheet = workbook.createSheet("test");
Row row1 = sheet.createRow(0);
Cell cellA1 = row1.createCell(0, CellType.FORMULA);
@ -68,4 +105,18 @@ public class CountifsTests extends TestCase {
evaluate = evaluator.evaluate(cellA1);
assertEquals(15, evaluate.getErrorValue());
}
/**
* the bug returned the wrong count, this verifies the fix
* @throws Exception if the file can't be read
*/
@Test
public void testBug56822() throws Exception {
workbook = XSSFTestDataSamples.openSampleWorkbook("56822-Countifs.xlsx");
FormulaEvaluator evaluator = workbook.getCreationHelper().createFormulaEvaluator();
Cell cell = SheetUtil.getCell(workbook.getSheetAt(0), 0, 3);
assertNotNull("Test workbook missing cell D1", cell);
CellValue evaluate = evaluator.evaluate(cell);
assertEquals(2.0d, evaluate.getNumberValue(), 0.00000000000001);
}
}

BIN
test-data/spreadsheet/56822-Countifs.xlsx Normal file
View File

Binary file not shown.