diff --git a/src/java/org/apache/poi/ss/formula/LazyRefEval.java b/src/java/org/apache/poi/ss/formula/LazyRefEval.java index e0b5e19f9..c71b0e977 100644 --- a/src/java/org/apache/poi/ss/formula/LazyRefEval.java +++ b/src/java/org/apache/poi/ss/formula/LazyRefEval.java @@ -27,7 +27,7 @@ import org.apache.poi.ss.util.CellReference; /** * Provides Lazy Evaluation to a 3D Reference */ -final class LazyRefEval extends RefEvalBase { +public final class LazyRefEval extends RefEvalBase { private final SheetRangeEvaluator _evaluator; public LazyRefEval(int rowIndex, int columnIndex, SheetRangeEvaluator sre) { @@ -47,14 +47,17 @@ final class LazyRefEval extends RefEvalBase { return new LazyAreaEval(area, _evaluator); } + public boolean isSubTotal() { + SheetRefEvaluator sheetEvaluator = _evaluator.getSheetEvaluator(getFirstSheetIndex()); + return sheetEvaluator.isSubTotal(getRow(), getColumn()); + } + public String toString() { CellReference cr = new CellReference(getRow(), getColumn()); - StringBuffer sb = new StringBuffer(); - sb.append(getClass().getName()).append("["); - sb.append(_evaluator.getSheetNameRange()); - sb.append('!'); - sb.append(cr.formatAsString()); - sb.append("]"); - return sb.toString(); + return getClass().getName() + "[" + + _evaluator.getSheetNameRange() + + '!' + + cr.formatAsString() + + "]"; } } diff --git a/src/java/org/apache/poi/ss/formula/functions/Subtotal.java b/src/java/org/apache/poi/ss/formula/functions/Subtotal.java index fca55c502..f3b24cd1c 100644 --- a/src/java/org/apache/poi/ss/formula/functions/Subtotal.java +++ b/src/java/org/apache/poi/ss/formula/functions/Subtotal.java @@ -19,6 +19,7 @@ package org.apache.poi.ss.formula.functions; import static org.apache.poi.ss.formula.functions.AggregateFunction.subtotalInstance; +import org.apache.poi.ss.formula.LazyRefEval; import org.apache.poi.ss.formula.eval.ErrorEval; import org.apache.poi.ss.formula.eval.EvaluationException; import org.apache.poi.ss.formula.eval.NotImplementedException; @@ -26,6 +27,11 @@ import org.apache.poi.ss.formula.eval.NotImplementedFunctionException; import org.apache.poi.ss.formula.eval.OperandResolver; import org.apache.poi.ss.formula.eval.ValueEval; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; + /** * Implementation for the Excel function SUBTOTAL

* @@ -61,7 +67,6 @@ import org.apache.poi.ss.formula.eval.ValueEval; public class Subtotal implements Function { private static Function findFunction(int functionCode) throws EvaluationException { - Function func; switch (functionCode) { case 1: return subtotalInstance(AggregateFunction.AVERAGE); case 2: return Count.subtotalInstance(); @@ -87,7 +92,7 @@ public class Subtotal implements Function { return ErrorEval.VALUE_INVALID; } - Function innerFunc; + final Function innerFunc; try { ValueEval ve = OperandResolver.getSingleValue(args[0], srcRowIndex, srcColumnIndex); int functionCode = OperandResolver.coerceValueToInt(ve); @@ -96,9 +101,24 @@ public class Subtotal implements Function { return e.getErrorEval(); } - ValueEval[] innerArgs = new ValueEval[nInnerArgs]; - System.arraycopy(args, 1, innerArgs, 0, nInnerArgs); + // ignore the first arg, this is the function-type, we check for the length above + final List list = new ArrayList(Arrays.asList(args).subList(1, args.length)); - return innerFunc.evaluate(innerArgs, srcRowIndex, srcColumnIndex); + Iterator it = list.iterator(); + + // See https://support.office.com/en-us/article/SUBTOTAL-function-7b027003-f060-4ade-9040-e478765b9939 + // "If there are other subtotals within ref1, ref2,... (or nested subtotals), these nested subtotals are ignored to avoid double counting." + // For array references it is handled in other evaluation steps, but we need to handle this here for references to subtotal-functions + while(it.hasNext()) { + ValueEval eval = it.next(); + if(eval instanceof LazyRefEval) { + LazyRefEval lazyRefEval = (LazyRefEval) eval; + if(lazyRefEval.isSubTotal()) { + it.remove(); + } + } + } + + return innerFunc.evaluate(list.toArray(new ValueEval[list.size()]), srcRowIndex, srcColumnIndex); } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java index 566944d18..9885fd886 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java @@ -154,7 +154,9 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator { evaluator.evaluate(cXSL_cell); fail("Without a fix for #56752, shouldn't be able to evaluate a " + "reference to a non-provided linked workbook"); - } catch(Exception e) {} + } catch(Exception e) { + // expected here + } // Setup the environment Map evaluators = new HashMap(); @@ -196,7 +198,9 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator { try { cXSLX_nw_cell.setCellFormula("[alt.xlsx]Sheet1!$A$1"); fail("New workbook not linked, shouldn't be able to add"); - } catch (Exception e) {} + } catch (Exception e) { + // expected here + } // Link and re-try Workbook alt = new XSSFWorkbook(); @@ -651,4 +655,20 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator { private Cell getCell(Sheet sheet, int rowNo, int column) { return sheet.getRow(rowNo).getCell(column); } + + @Test + public void test59736() { + Workbook wb = XSSFTestDataSamples.openSampleWorkbook("59736.xlsx"); + FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator(); + Cell cell = wb.getSheetAt(0).getRow(0).getCell(0); + assertEquals(1, cell.getNumericCellValue(), 0.001); + + cell = wb.getSheetAt(0).getRow(1).getCell(0); + CellValue value = evaluator.evaluate(cell); + assertEquals(1, value.getNumberValue(), 0.001); + + cell = wb.getSheetAt(0).getRow(2).getCell(0); + value = evaluator.evaluate(cell); + assertEquals(1, value.getNumberValue(), 0.001); + } } 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 7b63eaf79..f2750b591 100644 --- a/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java +++ b/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java @@ -20,9 +20,8 @@ package org.apache.poi.ss.formula.functions; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.hssf.usermodel.HSSFWorkbook; -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 org.apache.poi.ss.formula.FormulaParseException; +import org.apache.poi.ss.formula.eval.*; import junit.framework.TestCase; import org.apache.poi.ss.usermodel.*; @@ -75,7 +74,6 @@ public final class TestSubtotal extends TestCase { } public void testAvg(){ - Workbook wb = new HSSFWorkbook(); FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); @@ -95,16 +93,18 @@ public final class TestSubtotal extends TestCase { a6.setCellFormula("SUBTOTAL(1,B2:B6)*2 + 2"); Cell a7 = sh.createRow(7).createCell(1); a7.setCellFormula("SUBTOTAL(1,B2:B7)"); + Cell a8 = sh.createRow(8).createCell(1); + a8.setCellFormula("SUBTOTAL(1,B2,B3,B4,B5,B6,B7,B8)"); fe.evaluateAll(); assertEquals(2.0, a3.getNumericCellValue()); assertEquals(8.0, a6.getNumericCellValue()); assertEquals(3.0, a7.getNumericCellValue()); + assertEquals(3.0, a8.getNumericCellValue()); } public void testSum(){ - Workbook wb = new HSSFWorkbook(); FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); @@ -124,12 +124,15 @@ public final class TestSubtotal extends TestCase { a6.setCellFormula("SUBTOTAL(9,B2:B6)*2 + 2"); Cell a7 = sh.createRow(7).createCell(1); a7.setCellFormula("SUBTOTAL(9,B2:B7)"); + Cell a8 = sh.createRow(8).createCell(1); + a8.setCellFormula("SUBTOTAL(9,B2,B3,B4,B5,B6,B7,B8)"); fe.evaluateAll(); assertEquals(4.0, a3.getNumericCellValue()); assertEquals(26.0, a6.getNumericCellValue()); assertEquals(12.0, a7.getNumericCellValue()); + assertEquals(12.0, a8.getNumericCellValue()); } public void testCount(){ @@ -147,18 +150,21 @@ public final class TestSubtotal extends TestCase { a3.setCellFormula("SUBTOTAL(2,B2:B3)"); Cell a4 = sh.createRow(4).createCell(1); a4.setCellValue("POI"); // A4 is string and not counted - Cell a5 = sh.createRow(5).createCell(1); // A5 is blank and not counted + /*Cell a5 =*/ sh.createRow(5).createCell(1); // A5 is blank and not counted Cell a6 = sh.createRow(6).createCell(1); a6.setCellFormula("SUBTOTAL(2,B2:B6)*2 + 2"); Cell a7 = sh.createRow(7).createCell(1); a7.setCellFormula("SUBTOTAL(2,B2:B7)"); + Cell a8 = sh.createRow(8).createCell(1); + a8.setCellFormula("SUBTOTAL(2,B2,B3,B4,B5,B6,B7,B8)"); fe.evaluateAll(); assertEquals(2.0, a3.getNumericCellValue()); assertEquals(6.0, a6.getNumericCellValue()); assertEquals(2.0, a7.getNumericCellValue()); + assertEquals(2.0, a8.getNumericCellValue()); } public void testCounta(){ @@ -176,18 +182,21 @@ public final class TestSubtotal extends TestCase { a3.setCellFormula("SUBTOTAL(3,B2:B3)"); Cell a4 = sh.createRow(4).createCell(1); a4.setCellValue("POI"); // A4 is string and not counted - Cell a5 = sh.createRow(5).createCell(1); // A5 is blank and not counted + /*Cell a5 =*/ sh.createRow(5).createCell(1); // A5 is blank and not counted Cell a6 = sh.createRow(6).createCell(1); a6.setCellFormula("SUBTOTAL(3,B2:B6)*2 + 2"); Cell a7 = sh.createRow(7).createCell(1); a7.setCellFormula("SUBTOTAL(3,B2:B7)"); + Cell a8 = sh.createRow(8).createCell(1); + a8.setCellFormula("SUBTOTAL(3,B2,B3,B4,B5,B6,B7,B8)"); fe.evaluateAll(); assertEquals(2.0, a3.getNumericCellValue()); assertEquals(8.0, a6.getNumericCellValue()); assertEquals(3.0, a7.getNumericCellValue()); + assertEquals(3.0, a8.getNumericCellValue()); } public void testMax(){ @@ -211,12 +220,15 @@ public final class TestSubtotal extends TestCase { a6.setCellFormula("SUBTOTAL(4,B2:B6)*2 + 2"); Cell a7 = sh.createRow(7).createCell(1); a7.setCellFormula("SUBTOTAL(4,B2:B7)"); + Cell a8 = sh.createRow(8).createCell(1); + a8.setCellFormula("SUBTOTAL(4,B2,B3,B4,B5,B6,B7,B8)"); fe.evaluateAll(); assertEquals(3.0, a3.getNumericCellValue()); assertEquals(16.0, a6.getNumericCellValue()); assertEquals(7.0, a7.getNumericCellValue()); + assertEquals(7.0, a8.getNumericCellValue()); } public void testMin(){ @@ -240,12 +252,15 @@ public final class TestSubtotal extends TestCase { a6.setCellFormula("SUBTOTAL(5,B2:B6)*2 + 2"); Cell a7 = sh.createRow(7).createCell(1); a7.setCellFormula("SUBTOTAL(5,B2:B7)"); + Cell a8 = sh.createRow(8).createCell(1); + a8.setCellFormula("SUBTOTAL(5,B2,B3,B4,B5,B6,B7,B8)"); fe.evaluateAll(); assertEquals(1.0, a3.getNumericCellValue()); assertEquals(4.0, a6.getNumericCellValue()); assertEquals(1.0, a7.getNumericCellValue()); + assertEquals(1.0, a8.getNumericCellValue()); } public void testStdev(){ @@ -269,12 +284,15 @@ public final class TestSubtotal extends TestCase { a6.setCellFormula("SUBTOTAL(7,B2:B6)*2 + 2"); Cell a7 = sh.createRow(7).createCell(1); a7.setCellFormula("SUBTOTAL(7,B2:B7)"); + Cell a8 = sh.createRow(8).createCell(1); + a8.setCellFormula("SUBTOTAL(7,B2,B3,B4,B5,B6,B7,B8)"); fe.evaluateAll(); assertEquals(1.41421, a3.getNumericCellValue(), 0.0001); assertEquals(7.65685, a6.getNumericCellValue(), 0.0001); assertEquals(2.82842, a7.getNumericCellValue(), 0.0001); + assertEquals(2.82842, a8.getNumericCellValue(), 0.0001); } public void test50209(){ @@ -328,4 +346,69 @@ public final class TestSubtotal extends TestCase { confirmExpectedResult(evaluator, "SUBTOTAL(COUNT;B2:B8,C2:C8)", cellC2, 3.0); confirmExpectedResult(evaluator, "SUBTOTAL(COUNTA;B2:B8,C2:C8)", cellC3, 5.0); } + + public void testUnimplemented(){ + Workbook wb = new HSSFWorkbook(); + + FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator(); + + Sheet sh = wb.createSheet(); + Cell a3 = sh.createRow(3).createCell(1); + a3.setCellFormula("SUBTOTAL(8,B2:B3)"); + + try { + fe.evaluateAll(); + fail("Should catch an NotImplementedFunctionException here, adjust these tests if it was actually implemented"); + } catch (NotImplementedException e) { + // expected here + } + + a3.setCellFormula("SUBTOTAL(10,B2:B3)"); + + try { + fe.evaluateAll(); + fail("Should catch an NotImplementedFunctionException here, adjust these tests if it was actually implemented"); + } catch (NotImplementedException e) { + // expected here + } + + a3.setCellFormula("SUBTOTAL(11,B2:B3)"); + + try { + fe.evaluateAll(); + fail("Should catch an NotImplementedFunctionException here, adjust these tests if it was actually implemented"); + } catch (NotImplementedException e) { + // expected here + } + + a3.setCellFormula("SUBTOTAL(107,B2:B3)"); + + try { + fe.evaluateAll(); + fail("Should catch an NotImplementedFunctionException here, adjust these tests if it was actually implemented"); + } catch (NotImplementedException e) { + // expected here + } + + a3.setCellFormula("SUBTOTAL(0,B2:B3)"); + fe.evaluateAll(); + assertEquals(FormulaError.VALUE.getCode(), a3.getErrorCellValue()); + + try { + a3.setCellFormula("SUBTOTAL(9)"); + fail("Should catch an exception here"); + } catch (FormulaParseException e) { + // expected here + } + + try { + a3.setCellFormula("SUBTOTAL()"); + fail("Should catch an exception here"); + } catch (FormulaParseException e) { + // expected here + } + + Subtotal subtotal = new Subtotal(); + assertEquals(ErrorEval.VALUE_INVALID, subtotal.evaluate(new ValueEval[] {}, 0, 0)); + } } diff --git a/test-data/spreadsheet/59736.xlsx b/test-data/spreadsheet/59736.xlsx new file mode 100644 index 000000000..1f29d54c5 Binary files /dev/null and b/test-data/spreadsheet/59736.xlsx differ