From: Yegor Kozlov Date: Mon, 25 Jul 2011 12:55:32 +0000 (+0000) Subject: Bug 50209 - Fixed evaluation of Subtotals to ignore nested subtotals X-Git-Tag: REL_3_8_BETA4~68 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=23d2678a0ee61c0709d2e6d6bef243ca81bfb05e;p=poi.git Bug 50209 - Fixed evaluation of Subtotals to ignore nested subtotals git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1150673 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 62fb95d3c4..deb1511a0f 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 50209 - Fixed evaluation of Subtotals to ignore nested subtotals 44431 - HWPFDocument.write destroys fields 50401 - fixed EscherProperty to return property name instead of 'unknown' for complex properties Initial support for endnotes and footnotes in HWPF diff --git a/src/java/org/apache/poi/ss/formula/LazyAreaEval.java b/src/java/org/apache/poi/ss/formula/LazyAreaEval.java index f66d6b1961..1ba1e1a3f2 100644 --- a/src/java/org/apache/poi/ss/formula/LazyAreaEval.java +++ b/src/java/org/apache/poi/ss/formula/LazyAreaEval.java @@ -22,6 +22,9 @@ import org.apache.poi.ss.formula.ptg.AreaI.OffsetArea; import org.apache.poi.ss.formula.eval.AreaEval; import org.apache.poi.ss.formula.eval.AreaEvalBase; import org.apache.poi.ss.formula.eval.ValueEval; +import org.apache.poi.ss.formula.ptg.FuncVarPtg; +import org.apache.poi.ss.formula.ptg.Ptg; +import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.util.CellReference; /** @@ -87,4 +90,12 @@ final class LazyAreaEval extends AreaEvalBase { sb.append("]"); return sb.toString(); } + + /** + * @return whether cell at rowIndex and columnIndex is a subtotal + */ + public boolean isSubTotal(int rowIndex, int columnIndex){ + // delegate the query to the sheet evaluator which has access to internal ptgs + return _evaluator.isSubTotal(rowIndex, columnIndex); + } } diff --git a/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java b/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java index e6d15861ef..1746690050 100644 --- a/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java +++ b/src/java/org/apache/poi/ss/formula/SheetRefEvaluator.java @@ -18,6 +18,10 @@ package org.apache.poi.ss.formula; import org.apache.poi.ss.formula.eval.ValueEval; +import org.apache.poi.ss.formula.ptg.FuncVarPtg; +import org.apache.poi.ss.formula.ptg.Ptg; +import org.apache.poi.ss.usermodel.Cell; + /** * * @@ -53,4 +57,27 @@ final class SheetRefEvaluator { } return _sheet; } + + /** + * @return whether cell at rowIndex and columnIndex is a subtotal + * @see org.apache.poi.ss.formula.functions.Subtotal + */ + public boolean isSubTotal(int rowIndex, int columnIndex){ + boolean subtotal = false; + EvaluationCell cell = getSheet().getCell(rowIndex, columnIndex); + if(cell != null && cell.getCellType() == Cell.CELL_TYPE_FORMULA){ + EvaluationWorkbook wb = _bookEvaluator.getWorkbook(); + for(Ptg ptg : wb.getFormulaTokens(cell)){ + if(ptg instanceof FuncVarPtg){ + FuncVarPtg f = (FuncVarPtg)ptg; + if("SUBTOTAL".equals(f.getName())) { + subtotal = true; + break; + } + } + } + } + return subtotal; + } + } diff --git a/src/java/org/apache/poi/ss/formula/TwoDEval.java b/src/java/org/apache/poi/ss/formula/TwoDEval.java index 7b9411ae01..44ad917eeb 100644 --- a/src/java/org/apache/poi/ss/formula/TwoDEval.java +++ b/src/java/org/apache/poi/ss/formula/TwoDEval.java @@ -59,4 +59,11 @@ public interface TwoDEval extends ValueEval { * @return a single column {@link TwoDEval} */ TwoDEval getColumn(int columnIndex); + + + /** + * @return true if the cell at row and col is a subtotal + */ + boolean isSubTotal(int rowIndex, int columnIndex); + } diff --git a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java index c33fbd49f0..1328c850b4 100644 --- a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java +++ b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java @@ -129,6 +129,10 @@ public final class WorkbookEvaluator { return _workbook.getSheet(sheetIndex); } + /* package */ EvaluationWorkbook getWorkbook() { + return _workbook; + } + /* package */ EvaluationName getName(String name, int sheetIndex) { NamePtg namePtg = _workbook.getName(name, sheetIndex).createPtg(); diff --git a/src/java/org/apache/poi/ss/formula/eval/AreaEvalBase.java b/src/java/org/apache/poi/ss/formula/eval/AreaEvalBase.java index 6b81636e8d..f9c7a11cd2 100644 --- a/src/java/org/apache/poi/ss/formula/eval/AreaEvalBase.java +++ b/src/java/org/apache/poi/ss/formula/eval/AreaEvalBase.java @@ -114,4 +114,13 @@ public abstract class AreaEvalBase implements AreaEval { public int getWidth() { return _lastColumn-_firstColumn+1; } + + /** + * @return whether cell at rowIndex and columnIndex is a subtotal. + * By default return false which means 'don't care about subtotals' + */ + public boolean isSubTotal(int rowIndex, int columnIndex) { + return false; + } + } diff --git a/src/java/org/apache/poi/ss/formula/functions/AggregateFunction.java b/src/java/org/apache/poi/ss/formula/functions/AggregateFunction.java index 3c465f17b2..52f12210fe 100644 --- a/src/java/org/apache/poi/ss/formula/functions/AggregateFunction.java +++ b/src/java/org/apache/poi/ss/formula/functions/AggregateFunction.java @@ -80,11 +80,41 @@ public abstract class AggregateFunction extends MultiOperandNumericFunction { } } - protected AggregateFunction() { - super(false, false); - } + protected AggregateFunction() { + super(false, false); + } + + /** + * Create an instance to use in the {@link Subtotal} function. + * + *

+ * If there are other subtotals within argument refs (or nested subtotals), + * these nested subtotals are ignored to avoid double counting. + *

+ * + * @param func the function to wrap + * @return wrapped instance. The actual math is delegated to the argument function. + */ + /*package*/ static Function subtotalInstance(Function func) { + final AggregateFunction arg = (AggregateFunction)func; + return new AggregateFunction() { + @Override + protected double evaluate(double[] values) throws EvaluationException { + return arg.evaluate(values); + } + + /** + * ignore nested subtotals. + */ + @Override + public boolean isSubtotalCounted(){ + return false; + } + + }; + } - public static final Function AVEDEV = new AggregateFunction() { + public static final Function AVEDEV = new AggregateFunction() { protected double evaluate(double[] values) { return StatsLib.avedev(values); } diff --git a/src/java/org/apache/poi/ss/formula/functions/Count.java b/src/java/org/apache/poi/ss/formula/functions/Count.java index 8a1e41dfb7..b8de432edc 100644 --- a/src/java/org/apache/poi/ss/formula/functions/Count.java +++ b/src/java/org/apache/poi/ss/formula/functions/Count.java @@ -17,11 +17,13 @@ package org.apache.poi.ss.formula.functions; +import org.apache.poi.ss.formula.TwoDEval; import org.apache.poi.ss.formula.eval.ErrorEval; import org.apache.poi.ss.formula.eval.MissingArgEval; import org.apache.poi.ss.formula.eval.NumberEval; 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.CountUtils.I_MatchAreaPredicate; /** * Counts the number of cells that contain numeric data within @@ -35,6 +37,15 @@ import org.apache.poi.ss.formula.functions.CountUtils.I_MatchPredicate; * like formula cells, error cells etc */ public final class Count implements Function { + private final I_MatchPredicate _predicate; + + public Count(){ + _predicate = defaultPredicate; + } + + private Count(I_MatchPredicate criteriaPredicate){ + _predicate = criteriaPredicate; + } public ValueEval evaluate(ValueEval[] args, int srcCellRow, int srcCellCol) { int nArgs = args.length; @@ -51,13 +62,13 @@ public final class Count implements Function { int temp = 0; for(int i=0; i + * If there are other subtotals within argument refs (or nested subtotals), + * these nested subtotals are ignored to avoid double counting. + *

+ * + * @see Subtotal + */ + public static Count subtotalInstance() { + return new Count(subtotalPredicate ); + } } diff --git a/src/java/org/apache/poi/ss/formula/functions/CountUtils.java b/src/java/org/apache/poi/ss/formula/functions/CountUtils.java index de56130189..54a4e7c6ac 100644 --- a/src/java/org/apache/poi/ss/formula/functions/CountUtils.java +++ b/src/java/org/apache/poi/ss/formula/functions/CountUtils.java @@ -38,6 +38,10 @@ final class CountUtils { public interface I_MatchPredicate { boolean matches(ValueEval x); } + public interface I_MatchAreaPredicate extends I_MatchPredicate { + boolean matches(TwoDEval x, int rowIndex, int columnIndex); + } + /** * @return the number of evaluated cells in the range that match the specified criteria */ @@ -49,6 +53,12 @@ final class CountUtils { for (int rrIx=0; rrIx @@ -58,16 +59,17 @@ import org.apache.poi.ss.formula.eval.NotImplementedException; public class Subtotal implements Function { private static Function findFunction(int functionCode) throws EvaluationException { - switch (functionCode) { - case 1: return AggregateFunction.AVERAGE; - case 2: return new Count(); - case 3: return new Counta(); - case 4: return AggregateFunction.MAX; - case 5: return AggregateFunction.MIN; - case 6: return AggregateFunction.PRODUCT; - case 7: return AggregateFunction.STDEV; + Function func; + switch (functionCode) { + case 1: return subtotalInstance(AggregateFunction.AVERAGE); + case 2: return Count.subtotalInstance(); + case 3: return Counta.subtotalInstance(); + case 4: return subtotalInstance(AggregateFunction.MAX); + case 5: return subtotalInstance(AggregateFunction.MIN); + case 6: return subtotalInstance(AggregateFunction.PRODUCT); + case 7: return subtotalInstance(AggregateFunction.STDEV); case 8: throw new NotImplementedException("STDEVP"); - case 9: return AggregateFunction.SUM; + case 9: return subtotalInstance(AggregateFunction.SUM); case 10: throw new NotImplementedException("VAR"); case 11: throw new NotImplementedException("VARP"); } diff --git a/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java b/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java index 54cf5b774a..bd16d6a533 100644 --- a/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java +++ b/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java @@ -17,11 +17,21 @@ package org.apache.poi.ss.formula.functions; +import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.hssf.usermodel.HSSFCell; +import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator; +import org.apache.poi.hssf.usermodel.HSSFSheet; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; +import org.apache.poi.ss.formula.OperationEvaluationContext; import org.apache.poi.ss.formula.eval.AreaEval; import org.apache.poi.ss.formula.eval.NumberEval; import org.apache.poi.ss.formula.eval.ValueEval; import junit.framework.TestCase; +import org.apache.poi.ss.usermodel.*; +import org.apache.poi.ss.util.SheetBuilder; + +import java.util.Date; /** * Tests for {@link Subtotal} @@ -69,4 +79,259 @@ public final class TestSubtotal extends TestCase { confirmSubtotal(FUNCTION_PRODUCT, 3628800.0); confirmSubtotal(FUNCTION_STDEV, 3.0276503540974917); } + + public void testAvg(){ + + Workbook wb = new HSSFWorkbook(); + + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + + Sheet sh = wb.createSheet(); + Cell a1 = sh.createRow(0).createCell(0); + a1.setCellValue(1); + Cell a2 = sh.createRow(1).createCell(0); + a2.setCellValue(3); + Cell a3 = sh.createRow(2).createCell(0); + a3.setCellFormula("SUBTOTAL(1,A1:A2)"); + Cell a4 = sh.createRow(3).createCell(0); + a4.setCellValue(1); + Cell a5 = sh.createRow(4).createCell(0); + a5.setCellValue(7); + Cell a6 = sh.createRow(5).createCell(0); + a6.setCellFormula("SUBTOTAL(1,A1:A5)*2 + 2"); + Cell a7 = sh.createRow(6).createCell(0); + a7.setCellFormula("SUBTOTAL(1,A1:A6)"); + + fe.evaluateAll(); + + assertEquals(2.0, a3.getNumericCellValue()); + assertEquals(8.0, a6.getNumericCellValue()); + assertEquals(3.0, a7.getNumericCellValue()); + } + + public void testSum(){ + + Workbook wb = new HSSFWorkbook(); + + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + + Sheet sh = wb.createSheet(); + Cell a1 = sh.createRow(0).createCell(0); + a1.setCellValue(1); + Cell a2 = sh.createRow(1).createCell(0); + a2.setCellValue(3); + Cell a3 = sh.createRow(2).createCell(0); + a3.setCellFormula("SUBTOTAL(9,A1:A2)"); + Cell a4 = sh.createRow(3).createCell(0); + a4.setCellValue(1); + Cell a5 = sh.createRow(4).createCell(0); + a5.setCellValue(7); + Cell a6 = sh.createRow(5).createCell(0); + a6.setCellFormula("SUBTOTAL(9,A1:A5)*2 + 2"); + Cell a7 = sh.createRow(6).createCell(0); + a7.setCellFormula("SUBTOTAL(9,A1:A6)"); + + fe.evaluateAll(); + + assertEquals(4.0, a3.getNumericCellValue()); + assertEquals(26.0, a6.getNumericCellValue()); + assertEquals(12.0, a7.getNumericCellValue()); + } + + public void testCount(){ + + Workbook wb = new HSSFWorkbook(); + + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + + Sheet sh = wb.createSheet(); + Cell a1 = sh.createRow(0).createCell(0); + a1.setCellValue(1); + Cell a2 = sh.createRow(1).createCell(0); + a2.setCellValue(3); + Cell a3 = sh.createRow(2).createCell(0); + a3.setCellFormula("SUBTOTAL(2,A1:A2)"); + Cell a4 = sh.createRow(3).createCell(0); + a4.setCellValue("POI"); // A4 is string and not counted + Cell a5 = sh.createRow(4).createCell(0); // A5 is blank and not counted + + Cell a6 = sh.createRow(5).createCell(0); + a6.setCellFormula("SUBTOTAL(2,A1:A5)*2 + 2"); + Cell a7 = sh.createRow(6).createCell(0); + a7.setCellFormula("SUBTOTAL(2,A1:A6)"); + + fe.evaluateAll(); + + assertEquals(2.0, a3.getNumericCellValue()); + assertEquals(6.0, a6.getNumericCellValue()); + assertEquals(2.0, a7.getNumericCellValue()); + } + + public void testCounta(){ + + Workbook wb = new HSSFWorkbook(); + + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + + Sheet sh = wb.createSheet(); + Cell a1 = sh.createRow(0).createCell(0); + a1.setCellValue(1); + Cell a2 = sh.createRow(1).createCell(0); + a2.setCellValue(3); + Cell a3 = sh.createRow(2).createCell(0); + a3.setCellFormula("SUBTOTAL(3,A1:A2)"); + Cell a4 = sh.createRow(3).createCell(0); + a4.setCellValue("POI"); // A4 is string and not counted + Cell a5 = sh.createRow(4).createCell(0); // A5 is blank and not counted + + Cell a6 = sh.createRow(5).createCell(0); + a6.setCellFormula("SUBTOTAL(3,A1:A5)*2 + 2"); + Cell a7 = sh.createRow(6).createCell(0); + a7.setCellFormula("SUBTOTAL(3,A1:A6)"); + + fe.evaluateAll(); + + assertEquals(2.0, a3.getNumericCellValue()); + assertEquals(8.0, a6.getNumericCellValue()); + assertEquals(3.0, a7.getNumericCellValue()); + } + + public void testMax(){ + + Workbook wb = new HSSFWorkbook(); + + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + + Sheet sh = wb.createSheet(); + Cell a1 = sh.createRow(0).createCell(0); + a1.setCellValue(1); + Cell a2 = sh.createRow(1).createCell(0); + a2.setCellValue(3); + Cell a3 = sh.createRow(2).createCell(0); + a3.setCellFormula("SUBTOTAL(4,A1:A2)"); + Cell a4 = sh.createRow(3).createCell(0); + a4.setCellValue(1); + Cell a5 = sh.createRow(4).createCell(0); + a5.setCellValue(7); + Cell a6 = sh.createRow(5).createCell(0); + a6.setCellFormula("SUBTOTAL(4,A1:A5)*2 + 2"); + Cell a7 = sh.createRow(6).createCell(0); + a7.setCellFormula("SUBTOTAL(4,A1:A6)"); + + fe.evaluateAll(); + + assertEquals(3.0, a3.getNumericCellValue()); + assertEquals(16.0, a6.getNumericCellValue()); + assertEquals(7.0, a7.getNumericCellValue()); + } + + public void testMin(){ + + Workbook wb = new HSSFWorkbook(); + + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + + Sheet sh = wb.createSheet(); + Cell a1 = sh.createRow(0).createCell(0); + a1.setCellValue(1); + Cell a2 = sh.createRow(1).createCell(0); + a2.setCellValue(3); + Cell a3 = sh.createRow(2).createCell(0); + a3.setCellFormula("SUBTOTAL(5,A1:A2)"); + Cell a4 = sh.createRow(3).createCell(0); + a4.setCellValue(1); + Cell a5 = sh.createRow(4).createCell(0); + a5.setCellValue(7); + Cell a6 = sh.createRow(5).createCell(0); + a6.setCellFormula("SUBTOTAL(5,A1:A5)*2 + 2"); + Cell a7 = sh.createRow(6).createCell(0); + a7.setCellFormula("SUBTOTAL(5,A1:A6)"); + + fe.evaluateAll(); + + assertEquals(1.0, a3.getNumericCellValue()); + assertEquals(4.0, a6.getNumericCellValue()); + assertEquals(1.0, a7.getNumericCellValue()); + } + + public void testStdev(){ + + Workbook wb = new HSSFWorkbook(); + + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + + Sheet sh = wb.createSheet(); + Cell a1 = sh.createRow(0).createCell(0); + a1.setCellValue(1); + Cell a2 = sh.createRow(1).createCell(0); + a2.setCellValue(3); + Cell a3 = sh.createRow(2).createCell(0); + a3.setCellFormula("SUBTOTAL(7,A1:A2)"); + Cell a4 = sh.createRow(3).createCell(0); + a4.setCellValue(1); + Cell a5 = sh.createRow(4).createCell(0); + a5.setCellValue(7); + Cell a6 = sh.createRow(5).createCell(0); + a6.setCellFormula("SUBTOTAL(7,A1:A5)*2 + 2"); + Cell a7 = sh.createRow(6).createCell(0); + a7.setCellFormula("SUBTOTAL(7,A1:A6)"); + + fe.evaluateAll(); + + assertEquals(1.41421, a3.getNumericCellValue(), 0.0001); + assertEquals(7.65685, a6.getNumericCellValue(), 0.0001); + assertEquals(2.82842, a7.getNumericCellValue(), 0.0001); + } + + public void test50209(){ + Workbook wb = new HSSFWorkbook(); + Sheet sh = wb.createSheet(); + Cell a1 = sh.createRow(0).createCell(0); + a1.setCellValue(1); + Cell a2 = sh.createRow(1).createCell(0); + a2.setCellFormula("SUBTOTAL(9,A1)"); + Cell a3 = sh.createRow(2).createCell(0); + a3.setCellFormula("SUBTOTAL(9,A1:A2)"); + + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + fe.evaluateAll(); + assertEquals(1.0, a2.getNumericCellValue()); + assertEquals(1.0, a3.getNumericCellValue()); + } + + private static void confirmExpectedResult(FormulaEvaluator evaluator, String msg, Cell cell, double expected) { + + CellValue value = evaluator.evaluate(cell); + if (value.getErrorValue() != 0) + throw new RuntimeException(msg + ": " + value.formatAsString()); + assertEquals(msg, expected, value.getNumberValue()); + } + + public void testFunctionsFromTestSpreadsheet() { + HSSFWorkbook workbook = HSSFTestDataSamples.openSampleWorkbook("SubtotalsNested.xls"); + HSSFSheet sheet = workbook.getSheetAt(0); + FormulaEvaluator evaluator = workbook.getCreationHelper().createFormulaEvaluator(); + + assertEquals("A1", 10.0, sheet.getRow(0).getCell(0).getNumericCellValue()); + assertEquals("A2", 20.0, sheet.getRow(1).getCell(0).getNumericCellValue()); + + //Test simple subtotal over one area + Cell cellA3 = sheet.getRow(2).getCell(0); + confirmExpectedResult(evaluator, "A3", cellA3, 30.0); + + //Test existence of the second area + assertNotNull("B1 must not be null", sheet.getRow(0).getCell(1)); + assertEquals("B1", 7.0, sheet.getRow(0).getCell(1).getNumericCellValue()); + + Cell cellC1 = sheet.getRow(0).getCell(2); + Cell cellC2 = sheet.getRow(1).getCell(2); + Cell cellC3 = sheet.getRow(2).getCell(2); + + //Test Functions SUM, COUNT and COUNTA calculation of SUBTOTAL + //a) areas A and B are used + //b) first 2 subtotals don't consider the value of nested subtotal in A3 + confirmExpectedResult(evaluator, "SUBTOTAL(SUM;A1:A7;B1:B7)", cellC1, 37.0); + confirmExpectedResult(evaluator, "SUBTOTAL(COUNT;A1:A7;B1:B7)", cellC2, 3.0); + confirmExpectedResult(evaluator, "SUBTOTAL(COUNTA;A1:A7;B1:B7)", cellC3, 5.0); + } } diff --git a/test-data/spreadsheet/SubtotalsNested.xls b/test-data/spreadsheet/SubtotalsNested.xls new file mode 100644 index 0000000000..91d7463dcf Binary files /dev/null and b/test-data/spreadsheet/SubtotalsNested.xls differ