You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by jo...@apache.org on 2009/03/30 22:46:52 UTC
svn commit: r760162 - in /poi/trunk/src: documentation/content/xdocs/
java/org/apache/poi/ss/formula/
testcases/org/apache/poi/hssf/record/formula/eval/
Author: josh
Date: Mon Mar 30 20:46:51 2009
New Revision: 760162
URL: http://svn.apache.org/viewvc?rev=760162&view=rev
Log:
Fix for bug 46898 - Formula evaluator should not cache intermediate circular-reference error results
Modified:
poi/trunk/src/documentation/content/xdocs/changes.xml
poi/trunk/src/documentation/content/xdocs/status.xml
poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java
poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java
Modified: poi/trunk/src/documentation/content/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/changes.xml?rev=760162&r1=760161&r2=760162&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/changes.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/changes.xml Mon Mar 30 20:46:51 2009
@@ -37,6 +37,7 @@
<!-- Don't forget to update status.xml too! -->
<release version="3.5-beta6" date="2009-??-??">
+ <action dev="POI-DEVELOPERS" type="fix">46898 - Fixed formula evaluator to not cache intermediate circular-reference error results</action>
<action dev="POI-DEVELOPERS" type="fix">46917 - Fixed PageItemRecord(SXPI) to allow multiple field infos</action>
<action dev="POI-DEVELOPERS" type="fix">46904 - Fix POIFS issue with duplicate block 0 references on very old BIFF5/BIFF7 files</action>
<action dev="POI-DEVELOPERS" type="fix">46840 - PageSettingsBlock should include HEADERFOOTER record</action>
Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=760162&r1=760161&r2=760162&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Mon Mar 30 20:46:51 2009
@@ -34,6 +34,7 @@
<!-- Don't forget to update changes.xml too! -->
<changes>
<release version="3.5-beta6" date="2009-??-??">
+ <action dev="POI-DEVELOPERS" type="fix">46898 - Fixed formula evaluator to not cache intermediate circular-reference error results</action>
<action dev="POI-DEVELOPERS" type="fix">46917 - Fixed PageItemRecord(SXPI) to allow multiple field infos</action>
<action dev="POI-DEVELOPERS" type="fix">46904 - Fix POIFS issue with duplicate block 0 references on very old BIFF5/BIFF7 files</action>
<action dev="POI-DEVELOPERS" type="fix">46840 - PageSettingsBlock should include HEADERFOOTER record</action>
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java?rev=760162&r1=760161&r2=760162&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationTracker.java Mon Mar 30 20:46:51 2009
@@ -23,6 +23,7 @@
import java.util.Set;
import org.apache.poi.hssf.record.formula.eval.BlankEval;
+import org.apache.poi.hssf.record.formula.eval.ErrorEval;
import org.apache.poi.hssf.record.formula.eval.ValueEval;
import org.apache.poi.hssf.usermodel.HSSFCell;
@@ -80,6 +81,15 @@
throw new IllegalStateException("Call to endEvaluate without matching call to startEvaluate");
}
CellEvaluationFrame frame = _evaluationFrames.get(nFrames-1);
+ if (result == ErrorEval.CIRCULAR_REF_ERROR && nFrames > 1) {
+ // Don't cache a circular ref error result if this cell is not the top evaluated cell.
+ // A true circular ref error will propagate all the way around the loop. However, it's
+ // possible to have parts of the formula tree (/ parts of the loop) to evaluate to
+ // CIRCULAR_REF_ERROR, and that value not get used in the final cell result (see the
+ // unit tests for a simple example). Thus, the only CIRCULAR_REF_ERROR result that can
+ // safely be cached is that of the top evaluated cell.
+ return;
+ }
frame.updateFormulaResult(result);
}
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java?rev=760162&r1=760161&r2=760162&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java Mon Mar 30 20:46:51 2009
@@ -203,13 +203,13 @@
tracker.acceptFormulaDependency(cce);
}
IEvaluationListener evalListener = _evaluationListener;
+ ValueEval result;
if (cce.getValue() == null) {
if (!tracker.startEvaluate(cce)) {
return ErrorEval.CIRCULAR_REF_ERROR;
}
try {
- ValueEval result;
Ptg[] ptgs = _workbook.getFormulaTokens(srcCell);
if (evalListener == null) {
@@ -235,9 +235,13 @@
if (isDebugLogEnabled()) {
String sheetName = getSheetName(sheetIndex);
CellReference cr = new CellReference(rowIndex, columnIndex);
- logDebug("Evaluated " + sheetName + "!" + cr.formatAsString() + " to " + cce.getValue().toString());
+ logDebug("Evaluated " + sheetName + "!" + cr.formatAsString() + " to " + result.toString());
}
- return cce.getValue();
+ // Usually (result === cce.getValue())
+ // But sometimes: (result==ErrorEval.CIRCULAR_REF_ERROR, cce.getValue()==null)
+ // When circular references are detected, the cache entry is only updated for
+ // the top evaluation frame
+ return result;
}
/**
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java?rev=760162&r1=760161&r2=760162&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/formula/eval/TestCircularReferences.java Mon Mar 30 20:46:51 2009
@@ -25,7 +25,9 @@
import org.apache.poi.hssf.usermodel.HSSFRow;
import org.apache.poi.hssf.usermodel.HSSFSheet;
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.ss.usermodel.Cell;
import org.apache.poi.ss.usermodel.CellValue;
+import org.apache.poi.ss.usermodel.ErrorConstants;
/**
* Tests HSSFFormulaEvaluator for its handling of cell formula circular references.
*
@@ -118,4 +120,51 @@
confirmCycleErrorCode(cellValue);
}
+
+ public void testIntermediateCircularReferenceResults_bug46898() {
+ HSSFWorkbook wb = new HSSFWorkbook();
+ HSSFSheet sheet = wb.createSheet("Sheet1");
+
+ HSSFRow row = sheet.createRow(0);
+
+ HSSFCell cellA1 = row.createCell(0);
+ HSSFCell cellB1 = row.createCell(1);
+ HSSFCell cellC1 = row.createCell(2);
+ HSSFCell cellD1 = row.createCell(3);
+ HSSFCell cellE1 = row.createCell(4);
+
+ cellA1.setCellFormula("IF(FALSE, 1+B1, 42)");
+ cellB1.setCellFormula("1+C1");
+ cellC1.setCellFormula("1+D1");
+ cellD1.setCellFormula("1+E1");
+ cellE1.setCellFormula("1+A1");
+
+ HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb);
+ CellValue cv;
+
+ // Happy day flow - evaluate A1 first
+ cv = fe.evaluate(cellA1);
+ assertEquals(Cell.CELL_TYPE_NUMERIC, cv.getCellType());
+ assertEquals(42.0, cv.getNumberValue(), 0.0);
+ cv = fe.evaluate(cellB1); // no circ-ref-error because A1 result is cached
+ assertEquals(Cell.CELL_TYPE_NUMERIC, cv.getCellType());
+ assertEquals(46.0, cv.getNumberValue(), 0.0);
+
+ // Show the bug - evaluate another cell from the loop first
+ fe.clearAllCachedResultValues();
+ cv = fe.evaluate(cellB1);
+ if (cv.getCellType() == ErrorEval.CIRCULAR_REF_ERROR.getErrorCode()) {
+ throw new AssertionFailedError("Identified bug 46898");
+ }
+ assertEquals(Cell.CELL_TYPE_NUMERIC, cv.getCellType());
+ assertEquals(46.0, cv.getNumberValue(), 0.0);
+
+ // start evaluation on another cell
+ fe.clearAllCachedResultValues();
+ cv = fe.evaluate(cellE1);
+ assertEquals(Cell.CELL_TYPE_NUMERIC, cv.getCellType());
+ assertEquals(43.0, cv.getNumberValue(), 0.0);
+
+
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org