You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by ce...@apache.org on 2016/07/31 17:19:27 UTC
svn commit: r1754674 - in /poi/trunk: src/java/org/apache/poi/ss/formula/
src/java/org/apache/poi/ss/formula/functions/
src/ooxml/testcases/org/apache/poi/xssf/usermodel/
src/testcases/org/apache/poi/ss/formula/functions/ test-data/spreadsheet/
Author: centic
Date: Sun Jul 31 17:19:27 2016
New Revision: 1754674
URL: http://svn.apache.org/viewvc?rev=1754674&view=rev
Log:
Bug 59736: Incorrect evaluation of SUBTOTAL with composite interval
Added:
poi/trunk/test-data/spreadsheet/59736.xlsx
Modified:
poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java
poi/trunk/src/java/org/apache/poi/ss/formula/functions/Subtotal.java
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
poi/trunk/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java?rev=1754674&r1=1754673&r2=1754674&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/LazyRefEval.java Sun Jul 31 17:19:27 2016
@@ -27,7 +27,7 @@ import org.apache.poi.ss.util.CellRefere
/**
* 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 RefEvalB
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() +
+ "]";
}
}
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/functions/Subtotal.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/functions/Subtotal.java?rev=1754674&r1=1754673&r2=1754674&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/functions/Subtotal.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/functions/Subtotal.java Sun Jul 31 17:19:27 2016
@@ -19,6 +19,7 @@ package org.apache.poi.ss.formula.functi
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.No
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<p>
*
@@ -61,7 +67,6 @@ import org.apache.poi.ss.formula.eval.Va
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 Functio
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 Functio
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<ValueEval> list = new ArrayList<ValueEval>(Arrays.asList(args).subList(1, args.length));
+
+ Iterator<ValueEval> 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(innerArgs, srcRowIndex, srcColumnIndex);
+ return innerFunc.evaluate(list.toArray(new ValueEval[list.size()]), srcRowIndex, srcColumnIndex);
}
}
Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java?rev=1754674&r1=1754673&r2=1754674&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java Sun Jul 31 17:19:27 2016
@@ -154,7 +154,9 @@ public final class TestXSSFFormulaEvalua
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<String,FormulaEvaluator> evaluators = new HashMap<String, FormulaEvaluator>();
@@ -196,7 +198,9 @@ public final class TestXSSFFormulaEvalua
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 TestXSSFFormulaEvalua
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);
+ }
}
Modified: poi/trunk/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java?rev=1754674&r1=1754673&r2=1754674&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/formula/functions/TestSubtotal.java Sun Jul 31 17:19:27 2016
@@ -20,9 +20,8 @@ package org.apache.poi.ss.formula.functi
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
}
public void testAvg(){
-
Workbook wb = new HSSFWorkbook();
FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator();
@@ -95,16 +93,18 @@ public final class TestSubtotal extends
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
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
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
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
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
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
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
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));
+ }
}
Added: poi/trunk/test-data/spreadsheet/59736.xlsx
URL: http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/59736.xlsx?rev=1754674&view=auto
==============================================================================
Binary files poi/trunk/test-data/spreadsheet/59736.xlsx (added) and poi/trunk/test-data/spreadsheet/59736.xlsx Sun Jul 31 17:19:27 2016 differ
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org