Changed HSSFEvaluationWorkbook to avoid re-parsing cell formulas during execution. (working towards fix for bug 45865)

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@699178 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/hssf/record/formula/AreaI.java

@@ -50,10 +50,10 @@ public interface AreaI {
 
 		public OffsetArea(int baseRow, int baseColumn, int relFirstRowIx, int relLastRowIx,
 				int relFirstColIx, int relLastColIx) {
-			_firstRow = baseRow + relFirstRowIx;
+			_firstRow = baseRow + Math.min(relFirstRowIx, relLastRowIx);
-			_lastRow = baseRow + relLastRowIx;
+			_lastRow = baseRow + Math.max(relFirstRowIx, relLastRowIx);
-			_firstColumn = baseColumn + relFirstColIx;
+			_firstColumn = baseColumn + Math.min(relFirstColIx, relLastColIx);
-			_lastColumn = baseColumn + relLastColIx;
+			_lastColumn = baseColumn + Math.max(relFirstColIx, relLastColIx);
 		}
 
 		public int getFirstColumn() {
@@ -72,5 +72,4 @@ public interface AreaI {
 			return _lastRow;
 		}
 	}
-
 }

src/java/org/apache/poi/hssf/record/formula/AttrPtg.java

@@ -50,6 +50,8 @@ public final class AttrPtg extends ControlPtg {
     private static final BitField baxcel       = BitFieldFactory.getInstance(0x20); // 'assignment-style formula in a macro sheet'
     private static final BitField space        = BitFieldFactory.getInstance(0x40);
 
+    public static final AttrPtg SUM = new AttrPtg(0x0010, 0, null, -1);
+
     public static final class SpaceType {
         private SpaceType() {
             // no instances of this class

src/java/org/apache/poi/hssf/record/formula/eval/RangeEval.java

@@ -0,0 +1,71 @@
+/* ====================================================================
+   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.hssf.record.formula.eval;
+
+
+/**
+ * 
+ * @author Josh Micich 
+ */
+public final class RangeEval implements OperationEval {
+
+	public static final OperationEval instance = new RangeEval();
+	
+	private RangeEval() {
+	}
+
+	public Eval evaluate(Eval[] args, int srcCellRow, short srcCellCol) {
+		if(args.length != 2) {
+			return ErrorEval.VALUE_INVALID;
+		}
+
+		try {
+			RefEval reA = evaluateRef(args[0]);
+			RefEval reB = evaluateRef(args[1]);
+			return resolveRange(reA, reB);
+		} catch (EvaluationException e) {
+			return e.getErrorEval();
+		}
+	}
+
+	private static AreaEval resolveRange(RefEval reA, RefEval reB) {
+		
+		int height = reB.getRow() - reA.getRow();
+		int width = reB.getColumn() - reA.getColumn();
+		
+		return reA.offset(0, height, 0, width);
+	}
+
+	private static RefEval evaluateRef(Eval arg) throws EvaluationException {
+		if (arg instanceof RefEval) {
+			return (RefEval) arg;
+		}
+		if (arg instanceof ErrorEval) {
+			throw new EvaluationException((ErrorEval)arg);
+		}
+		throw new IllegalArgumentException("Unexpected ref arg class (" + arg.getClass().getName() + ")");
+	}
+
+	public int getNumberOfOperands() {
+		return 2;
+	}
+
+	public int getType() {
+		throw new RuntimeException("obsolete code should not be called");
+	}
+}

src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java

@@ -2,7 +2,9 @@ package org.apache.poi.hssf.usermodel;
 
 import org.apache.poi.hssf.model.HSSFFormulaParser;
 import org.apache.poi.hssf.model.Workbook;
+import org.apache.poi.hssf.record.FormulaRecord;
 import org.apache.poi.hssf.record.NameRecord;
+import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate;
 import org.apache.poi.hssf.record.formula.NamePtg;
 import org.apache.poi.hssf.record.formula.NameXPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
@@ -94,7 +96,14 @@ public final class HSSFEvaluationWorkbook implements FormulaRenderingWorkbook, E
 		return new Name(_iBook.getNameRecord(ix), ix);
 	}
 	public Ptg[] getFormulaTokens(HSSFCell cell) {
-		return HSSFFormulaParser.parse(cell.getCellFormula(), _uBook);
+		if (false) {
+			// re-parsing the formula text also works, but is a waste of time
+			// It is useful from time to time to run all unit tests with this code
+			// to make sure that all formulas POI can evaluate can also be parsed.
+			return HSSFFormulaParser.parse(cell.getCellFormula(), _uBook);
+		}
+		FormulaRecord fr = ((FormulaRecordAggregate) cell.getCellValueRecord()).getFormulaRecord();
+		return fr.getParsedExpression();
 	}
 
 	private static final class Name implements EvaluationName {

src/java/org/apache/poi/ss/formula/OperationEvaluatorFactory.java

@@ -40,6 +40,7 @@ import org.apache.poi.hssf.record.formula.OperationPtg;
 import org.apache.poi.hssf.record.formula.PercentPtg;
 import org.apache.poi.hssf.record.formula.PowerPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.RangePtg;
 import org.apache.poi.hssf.record.formula.SubtractPtg;
 import org.apache.poi.hssf.record.formula.UnaryMinusPtg;
 import org.apache.poi.hssf.record.formula.UnaryPlusPtg;
@@ -57,6 +58,7 @@ import org.apache.poi.hssf.record.formula.eval.NotEqualEval;
 import org.apache.poi.hssf.record.formula.eval.OperationEval;
 import org.apache.poi.hssf.record.formula.eval.PercentEval;
 import org.apache.poi.hssf.record.formula.eval.PowerEval;
+import org.apache.poi.hssf.record.formula.eval.RangeEval;
 import org.apache.poi.hssf.record.formula.eval.SubtractEval;
 import org.apache.poi.hssf.record.formula.eval.UnaryMinusEval;
 import org.apache.poi.hssf.record.formula.eval.UnaryPlusEval;
@@ -101,6 +103,7 @@ final class OperationEvaluatorFactory {
 		add(m, SubtractPtg.class, SubtractEval.instance);
 		add(m, UnaryMinusPtg.class, UnaryMinusEval.instance);
 		add(m, UnaryPlusPtg.class, UnaryPlusEval.instance);
+		add(m, RangePtg.class, RangeEval.instance);
 		return m;
 	}
 

src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java

@@ -22,12 +22,18 @@ import java.util.Map;
 import java.util.Stack;
 
 import org.apache.poi.hssf.record.formula.Area3DPtg;
+import org.apache.poi.hssf.record.formula.AreaErrPtg;
 import org.apache.poi.hssf.record.formula.AreaPtg;
+import org.apache.poi.hssf.record.formula.AttrPtg;
 import org.apache.poi.hssf.record.formula.BoolPtg;
 import org.apache.poi.hssf.record.formula.ControlPtg;
+import org.apache.poi.hssf.record.formula.DeletedArea3DPtg;
+import org.apache.poi.hssf.record.formula.DeletedRef3DPtg;
 import org.apache.poi.hssf.record.formula.ErrPtg;
+import org.apache.poi.hssf.record.formula.FuncVarPtg;
 import org.apache.poi.hssf.record.formula.IntPtg;
 import org.apache.poi.hssf.record.formula.MemErrPtg;
+import org.apache.poi.hssf.record.formula.MemFuncPtg;
 import org.apache.poi.hssf.record.formula.MissingArgPtg;
 import org.apache.poi.hssf.record.formula.NamePtg;
 import org.apache.poi.hssf.record.formula.NameXPtg;
@@ -35,6 +41,7 @@ import org.apache.poi.hssf.record.formula.NumberPtg;
 import org.apache.poi.hssf.record.formula.OperationPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.record.formula.Ref3DPtg;
+import org.apache.poi.hssf.record.formula.RefErrorPtg;
 import org.apache.poi.hssf.record.formula.RefPtg;
 import org.apache.poi.hssf.record.formula.StringPtg;
 import org.apache.poi.hssf.record.formula.UnionPtg;
@@ -181,10 +188,10 @@ public class WorkbookEvaluator {
 				isPlainFormulaCell = false;
 				Ptg[] ptgs = _workbook.getFormulaTokens(srcCell);
 				if(evalListener == null) {
-					result = evaluateCell(sheetIndex, rowIndex, (short)columnIndex, ptgs, tracker);
+					result = evaluateFormula(sheetIndex, rowIndex, (short)columnIndex, ptgs, tracker);
 				} else {
 					evalListener.onStartEvaluate(sheetIndex, rowIndex, columnIndex, ptgs);
-					result = evaluateCell(sheetIndex, rowIndex, (short)columnIndex, ptgs, tracker);
+					result = evaluateFormula(sheetIndex, rowIndex, (short)columnIndex, ptgs, tracker);
 					evalListener.onEndEvaluate(sheetIndex, rowIndex, columnIndex, result);
 				}
 			}
@@ -225,17 +232,31 @@ public class WorkbookEvaluator {
 		}
 		throw new RuntimeException("Unexpected cell type (" + cellType + ")");
 	}
-	private ValueEval evaluateCell(int sheetIndex, int srcRowNum, short srcColNum, Ptg[] ptgs, EvaluationTracker tracker) {
+	// visibility raised for testing
+	/* package */ ValueEval evaluateFormula(int sheetIndex, int srcRowNum, int srcColNum, Ptg[] ptgs, EvaluationTracker tracker) {
 
 		Stack stack = new Stack();
 		for (int i = 0, iSize = ptgs.length; i < iSize; i++) {
 
 			// since we don't know how to handle these yet :(
 			Ptg ptg = ptgs[i];
+			if (ptg instanceof AttrPtg) {
+				AttrPtg attrPtg = (AttrPtg) ptg;
+				if (attrPtg.isSum()) {
+					// Excel prefers to encode 'SUM()' as a tAttr token, but this evaluator
+					// expects the equivalent function token
+					byte nArgs = 1;  // tAttrSum always has 1 parameter
+					ptg = new FuncVarPtg("SUM", nArgs); 
+				}
+			}
 			if (ptg instanceof ControlPtg) {
 				// skip Parentheses, Attr, etc
 				continue;
 			}
+			if (ptg instanceof MemFuncPtg) {
+				// can ignore, rest of tokens for this expression are in OK RPN order
+				continue;
+			}
 			if (ptg instanceof MemErrPtg) { continue; }
 			if (ptg instanceof MissingArgPtg) {
 				// TODO - might need to push BlankEval or MissingArgEval
@@ -289,7 +310,7 @@ public class WorkbookEvaluator {
 	 * @return a <tt>NumberEval</tt>, <tt>StringEval</tt>, <tt>BoolEval</tt>,
 	 *  <tt>BlankEval</tt> or <tt>ErrorEval</tt>. Never <code>null</code>.
 	 */
-	private static ValueEval dereferenceValue(ValueEval evaluationResult, int srcRowNum, short srcColNum) {
+	private static ValueEval dereferenceValue(ValueEval evaluationResult, int srcRowNum, int srcColNum) {
 		if (evaluationResult instanceof RefEval) {
 			RefEval rv = (RefEval) evaluationResult;
 			return rv.getInnerValueEval();
@@ -361,6 +382,10 @@ public class WorkbookEvaluator {
 		if (ptg instanceof ErrPtg) {
 			return ErrorEval.valueOf(((ErrPtg) ptg).getErrorCode());
 		}
+		if (ptg instanceof AreaErrPtg ||ptg instanceof RefErrorPtg 
+				|| ptg instanceof DeletedArea3DPtg || ptg instanceof DeletedRef3DPtg) {
+				return ErrorEval.REF_INVALID;
+		}
 		if (ptg instanceof Ref3DPtg) {
 			Ref3DPtg refPtg = (Ref3DPtg) ptg;
 			int otherSheetIndex = _workbook.convertFromExternSheetIndex(refPtg.getExternSheetIndex());

src/testcases/org/apache/poi/hssf/HSSFTests.java

@@ -28,7 +28,7 @@ import org.apache.poi.hssf.model.AllModelTests;
 import org.apache.poi.hssf.record.AllRecordTests;
 import org.apache.poi.hssf.usermodel.AllUserModelTests;
 import org.apache.poi.hssf.util.AllHSSFUtilTests;
-import org.apache.poi.ss.formula.TestEvaluationCache;
+import org.apache.poi.ss.formula.AllSSFormulaTests;
 
 /**
  * Test Suite for all sub-packages of org.apache.poi.hssf<br/>
@@ -53,7 +53,7 @@ public final class HSSFTests {
         }
         suite.addTest(new TestSuite(TestEventRecordFactory.class));
         suite.addTest(new TestSuite(TestModelFactory.class));
-        suite.addTest(new TestSuite(TestEvaluationCache.class));
+        suite.addTest(AllSSFormulaTests.suite());
         return suite;
     }
 }

src/testcases/org/apache/poi/hssf/record/formula/eval/AllFormulaEvalTests.java

@@ -37,6 +37,7 @@ public class AllFormulaEvalTests {
 		result.addTestSuite(TestFormulaBugs.class);
 		result.addTestSuite(TestFormulasFromSpreadsheet.class);
 		result.addTestSuite(TestPercentEval.class);
+		result.addTestSuite(TestRangeEval.class);
 		result.addTestSuite(TestUnaryPlusEval.class);
 		return result;
 	}

src/testcases/org/apache/poi/hssf/record/formula/eval/TestRangeEval.java

@@ -0,0 +1,95 @@
+/* ====================================================================
+   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.hssf.record.formula.eval;
+
+import org.apache.poi.hssf.record.formula.AreaI;
+import org.apache.poi.hssf.record.formula.AreaI.OffsetArea;
+import org.apache.poi.hssf.util.AreaReference;
+import org.apache.poi.hssf.util.CellReference;
+
+import junit.framework.TestCase;
+
+/**
+ * Test for unary plus operator evaluator.
+ *
+ * @author Josh Micich
+ */
+public final class TestRangeEval extends TestCase {
+	
+	public void testPermutations() {
+		
+		confirm("B3", "D7", "B3:D7");
+		confirm("B1", "B1", "B1:B1");
+		
+		confirm("B7", "D3", "B3:D7");
+		confirm("D3", "B7", "B3:D7");
+		confirm("D7", "B3", "B3:D7");
+	}
+
+	private static void confirm(String refA, String refB, String expectedAreaRef) {
+		
+		Eval[] args = {
+			createRefEval(refA),	
+			createRefEval(refB),	
+		};
+		AreaReference ar = new AreaReference(expectedAreaRef);
+		Eval result = RangeEval.instance.evaluate(args, 0, (short)0);
+		assertTrue(result instanceof AreaEval);
+		AreaEval ae = (AreaEval) result;
+		assertEquals(ar.getFirstCell().getRow(), ae.getFirstRow());
+		assertEquals(ar.getLastCell().getRow(), ae.getLastRow());
+		assertEquals(ar.getFirstCell().getCol(), ae.getFirstColumn());
+		assertEquals(ar.getLastCell().getCol(), ae.getLastColumn());
+	}
+
+	private static Eval createRefEval(String refStr) {
+		CellReference cr = new CellReference(refStr);
+		return new MockRefEval(cr.getRow(), cr.getCol());
+		
+	}
+
+	private static final class MockRefEval extends RefEvalBase {
+
+		public MockRefEval(int rowIndex, int columnIndex) {
+			super(rowIndex, columnIndex);
+		}
+		public ValueEval getInnerValueEval() {
+			throw new RuntimeException("not expected to be called during this test");
+		}
+		public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx,
+				int relLastColIx) {
+			AreaI area = new OffsetArea(getRow(), getColumn(),
+					relFirstRowIx, relLastRowIx, relFirstColIx, relLastColIx);
+			return new MockAreaEval(area);
+		}
+	}
+
+	private static final class MockAreaEval extends AreaEvalBase {
+
+		public MockAreaEval(AreaI ptg) {
+			super(ptg);
+		}
+		public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) {
+			throw new RuntimeException("not expected to be called during this test");
+		}
+		public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx,
+				int relLastColIx) {
+			throw new RuntimeException("not expected to be called during this test");
+		}
+	}
+}

src/testcases/org/apache/poi/ss/formula/AllSSFormulaTests.java

@@ -0,0 +1,34 @@
+/* ====================================================================
+   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;
+
+import junit.framework.Test;
+import junit.framework.TestSuite;
+/**
+ * Test suite for org.apache.poi.ss.formula
+ * 
+ * @author Josh Micich
+ */
+public final class AllSSFormulaTests {
+    public static Test suite() {
+        TestSuite result = new TestSuite(AllSSFormulaTests.class.getName());
+        result.addTestSuite(TestEvaluationCache.class);
+        result.addTestSuite(TestWorkbookEvaluator.class);
+        return result;
+    }
+}

src/testcases/org/apache/poi/ss/formula/TestWorkbookEvaluator.java

@@ -0,0 +1,92 @@
+/* ====================================================================
+   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;
+
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.record.formula.AreaErrPtg;
+import org.apache.poi.hssf.record.formula.AttrPtg;
+import org.apache.poi.hssf.record.formula.DeletedArea3DPtg;
+import org.apache.poi.hssf.record.formula.DeletedRef3DPtg;
+import org.apache.poi.hssf.record.formula.IntPtg;
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.RefErrorPtg;
+import org.apache.poi.hssf.record.formula.eval.ErrorEval;
+import org.apache.poi.hssf.record.formula.eval.NumberEval;
+import org.apache.poi.hssf.record.formula.eval.ValueEval;
+
+/**
+ * Tests {@link WorkbookEvaluator}. 
+ *
+ * @author Josh Micich
+ */
+public class TestWorkbookEvaluator extends TestCase {
+	
+	/**
+	 * Make sure that the evaluator can directly handle tAttrSum (instead of relying on re-parsing
+	 * the whole formula which converts tAttrSum to tFuncVar("SUM") ) 
+	 */
+	public void testAttrSum() {
+		
+		Ptg[] ptgs = {
+			new IntPtg(42),
+			AttrPtg.SUM,
+		};
+		
+		ValueEval result = new WorkbookEvaluator(null).evaluateFormula(0, 0, 0, ptgs, null);
+		assertEquals(42, ((NumberEval)result).getNumberValue(), 0.0);
+	}
+	
+	/**
+	 * Make sure that the evaluator can directly handle (deleted) ref error tokens
+	 * (instead of relying on re-parsing the whole formula which converts these 
+	 * to the error constant #REF! ) 
+	 */
+	public void testRefErr() {
+		
+		confirmRefErr(new RefErrorPtg());
+		confirmRefErr(new AreaErrPtg());
+		confirmRefErr(new DeletedRef3DPtg(0));
+		confirmRefErr(new DeletedArea3DPtg(0));
+	}
+	private static void confirmRefErr(Ptg ptg) {
+		Ptg[] ptgs = {
+			ptg,
+		};
+		
+		ValueEval result = new WorkbookEvaluator(null).evaluateFormula(0, 0, 0, ptgs, null);
+		assertEquals(ErrorEval.REF_INVALID, result);
+	}
+	
+	/**
+	 * Make sure that the evaluator can directly handle tAttrSum (instead of relying on re-parsing
+	 * the whole formula which converts tAttrSum to tFuncVar("SUM") ) 
+	 */
+	public void testMemFunc() {
+		
+		Ptg[] ptgs = {
+			new IntPtg(42),
+			AttrPtg.SUM,
+		};
+		
+		ValueEval result = new WorkbookEvaluator(null).evaluateFormula(0, 0, 0, ptgs, null);
+		assertEquals(42, ((NumberEval)result).getNumberValue(), 0.0);
+	}
+	
+	
+}