Found some optimizations in the general evaluation framework related to blank cells in rows beyond the last defined row of a sheet. I don't see any issue with passing a bit of context down deeper into this framework, as it's all POI-internal and only had one calling path. See the above bug for the performance analysis. Not specifically related to VLOOKUP, but improves that case by more than 2/3 as well. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1817252 13f79535-47bb-0310-9956-ffa450edef68tags/REL_4_0_0_FINAL
@@ -28,14 +28,25 @@ import org.apache.poi.util.Internal; | |||
final class HSSFEvaluationSheet implements EvaluationSheet { | |||
private final HSSFSheet _hs; | |||
private int _lastDefinedRow = -1; | |||
public HSSFEvaluationSheet(HSSFSheet hs) { | |||
_hs = hs; | |||
_lastDefinedRow = _hs.getLastRowNum(); | |||
} | |||
public HSSFSheet getHSSFSheet() { | |||
return _hs; | |||
} | |||
/* (non-Javadoc) | |||
* @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum() | |||
* @since POI 4.0.0 | |||
*/ | |||
public int getlastRowNum() { | |||
return _lastDefinedRow; | |||
} | |||
@Override | |||
public EvaluationCell getCell(int rowIndex, int columnIndex) { | |||
HSSFRow row = _hs.getRow(rowIndex); | |||
@@ -54,6 +65,6 @@ final class HSSFEvaluationSheet implements EvaluationSheet { | |||
*/ | |||
@Override | |||
public void clearAllCachedResultValues() { | |||
// nothing to do | |||
_lastDefinedRow = _hs.getLastRowNum(); | |||
} | |||
} |
@@ -64,11 +64,11 @@ final class CellEvaluationFrame { | |||
_sensitiveInputCells.toArray(result); | |||
return result; | |||
} | |||
public void addUsedBlankCell(int bookIndex, int sheetIndex, int rowIndex, int columnIndex) { | |||
public void addUsedBlankCell(EvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex, int rowIndex, int columnIndex) { | |||
if (_usedBlankCellGroup == null) { | |||
_usedBlankCellGroup = new FormulaUsedBlankCellSet(); | |||
} | |||
_usedBlankCellGroup.addCell(bookIndex, sheetIndex, rowIndex, columnIndex); | |||
_usedBlankCellGroup.addCell(evalWorkbook, bookIndex, sheetIndex, rowIndex, columnIndex); | |||
} | |||
public void updateFormulaResult(ValueEval result) { |
@@ -17,6 +17,7 @@ | |||
package org.apache.poi.ss.formula; | |||
import org.apache.poi.ss.usermodel.Sheet; | |||
import org.apache.poi.util.Internal; | |||
/** | |||
@@ -42,4 +43,10 @@ public interface EvaluationSheet { | |||
* @since POI 3.15 beta 3 | |||
*/ | |||
public void clearAllCachedResultValues(); | |||
/** | |||
* @return last row index referenced on this sheet, for evaluation optimization | |||
* @since POI 4.0.0 | |||
*/ | |||
public int getlastRowNum(); | |||
} |
@@ -131,7 +131,7 @@ final class EvaluationTracker { | |||
} | |||
} | |||
public void acceptPlainValueDependency(int bookIndex, int sheetIndex, | |||
public void acceptPlainValueDependency(EvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex, | |||
int rowIndex, int columnIndex, ValueEval value) { | |||
// Tell the currently evaluating cell frame that it has a dependency on the specified | |||
int prevFrameIndex = _evaluationFrames.size() - 1; | |||
@@ -140,7 +140,7 @@ final class EvaluationTracker { | |||
} else { | |||
CellEvaluationFrame consumingFrame = _evaluationFrames.get(prevFrameIndex); | |||
if (value == BlankEval.instance) { | |||
consumingFrame.addUsedBlankCell(bookIndex, sheetIndex, rowIndex, columnIndex); | |||
consumingFrame.addUsedBlankCell(evalWorkbook, bookIndex, sheetIndex, rowIndex, columnIndex); | |||
} else { | |||
PlainValueCellCacheEntry cce = _cache.getPlainValueEntry(bookIndex, sheetIndex, | |||
rowIndex, columnIndex, value); |
@@ -57,13 +57,17 @@ final class FormulaUsedBlankCellSet { | |||
private int _firstColumnIndex; | |||
private int _lastColumnIndex; | |||
private BlankCellRectangleGroup _currentRectangleGroup; | |||
private int _lastDefinedRow; | |||
public BlankCellSheetGroup() { | |||
public BlankCellSheetGroup(int lastDefinedRow) { | |||
_rectangleGroups = new ArrayList<>(); | |||
_currentRowIndex = -1; | |||
_lastDefinedRow = lastDefinedRow; | |||
} | |||
public void addCell(int rowIndex, int columnIndex) { | |||
if (rowIndex > _lastDefinedRow) return; | |||
if (_currentRowIndex == -1) { | |||
_currentRowIndex = rowIndex; | |||
_firstColumnIndex = columnIndex; | |||
@@ -89,6 +93,8 @@ final class FormulaUsedBlankCellSet { | |||
} | |||
public boolean containsCell(int rowIndex, int columnIndex) { | |||
if (rowIndex > _lastDefinedRow) return true; | |||
for (int i=_rectangleGroups.size()-1; i>=0; i--) { | |||
BlankCellRectangleGroup bcrg = _rectangleGroups.get(i); | |||
if (bcrg.containsCell(rowIndex, columnIndex)) { | |||
@@ -167,17 +173,17 @@ final class FormulaUsedBlankCellSet { | |||
_sheetGroupsByBookSheet = new HashMap<>(); | |||
} | |||
public void addCell(int bookIndex, int sheetIndex, int rowIndex, int columnIndex) { | |||
BlankCellSheetGroup sbcg = getSheetGroup(bookIndex, sheetIndex); | |||
public void addCell(EvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex, int rowIndex, int columnIndex) { | |||
BlankCellSheetGroup sbcg = getSheetGroup(evalWorkbook, bookIndex, sheetIndex); | |||
sbcg.addCell(rowIndex, columnIndex); | |||
} | |||
private BlankCellSheetGroup getSheetGroup(int bookIndex, int sheetIndex) { | |||
private BlankCellSheetGroup getSheetGroup(EvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex) { | |||
BookSheetKey key = new BookSheetKey(bookIndex, sheetIndex); | |||
BlankCellSheetGroup result = _sheetGroupsByBookSheet.get(key); | |||
if (result == null) { | |||
result = new BlankCellSheetGroup(); | |||
result = new BlankCellSheetGroup(evalWorkbook.getSheet(sheetIndex).getlastRowNum()); | |||
_sheetGroupsByBookSheet.put(key, result); | |||
} | |||
return result; |
@@ -253,7 +253,7 @@ public final class WorkbookEvaluator { | |||
if (srcCell == null || srcCell.getCellType() != CellType.FORMULA) { | |||
ValueEval result = getValueFromNonFormulaCell(srcCell); | |||
if (shouldCellDependencyBeRecorded) { | |||
tracker.acceptPlainValueDependency(_workbookIx, sheetIndex, rowIndex, columnIndex, result); | |||
tracker.acceptPlainValueDependency(_workbook, _workbookIx, sheetIndex, rowIndex, columnIndex, result); | |||
} | |||
return result; | |||
} |
@@ -42,6 +42,7 @@ import org.apache.poi.util.Internal; | |||
final class ForkedEvaluationSheet implements EvaluationSheet { | |||
private final EvaluationSheet _masterSheet; | |||
/** | |||
* Only cells which have been split are put in this map. (This has been done to conserve memory). | |||
*/ | |||
@@ -51,7 +52,15 @@ final class ForkedEvaluationSheet implements EvaluationSheet { | |||
_masterSheet = masterSheet; | |||
_sharedCellsByRowCol = new HashMap<>(); | |||
} | |||
/* (non-Javadoc) | |||
* @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum() | |||
* @since POI 4.0.0 | |||
*/ | |||
public int getlastRowNum() { | |||
return _masterSheet.getlastRowNum(); | |||
} | |||
@Override | |||
public EvaluationCell getCell(int rowIndex, int columnIndex) { | |||
RowColKey key = new RowColKey(rowIndex, columnIndex); |
@@ -27,14 +27,25 @@ import org.apache.poi.util.Internal; | |||
@Internal | |||
final class SXSSFEvaluationSheet implements EvaluationSheet { | |||
private final SXSSFSheet _xs; | |||
private int _lastDefinedRow = -1; | |||
public SXSSFEvaluationSheet(SXSSFSheet sheet) { | |||
_xs = sheet; | |||
_lastDefinedRow = _xs.getLastRowNum(); | |||
} | |||
public SXSSFSheet getSXSSFSheet() { | |||
return _xs; | |||
} | |||
/* (non-Javadoc) | |||
* @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum() | |||
* @since POI 4.0.0 | |||
*/ | |||
public int getlastRowNum() { | |||
return _lastDefinedRow; | |||
} | |||
@Override | |||
public EvaluationCell getCell(int rowIndex, int columnIndex) { | |||
SXSSFRow row = _xs.getRow(rowIndex); | |||
@@ -56,6 +67,6 @@ final class SXSSFEvaluationSheet implements EvaluationSheet { | |||
*/ | |||
@Override | |||
public void clearAllCachedResultValues() { | |||
// nothing to do | |||
_lastDefinedRow = _xs.getLastRowNum(); | |||
} | |||
} |
@@ -34,25 +34,40 @@ final class XSSFEvaluationSheet implements EvaluationSheet { | |||
private final XSSFSheet _xs; | |||
private Map<CellKey, EvaluationCell> _cellCache; | |||
private int _lastDefinedRow = -1; | |||
public XSSFEvaluationSheet(XSSFSheet sheet) { | |||
_xs = sheet; | |||
_lastDefinedRow = _xs.getLastRowNum(); | |||
} | |||
public XSSFSheet getXSSFSheet() { | |||
return _xs; | |||
} | |||
/* (non-Javadoc) | |||
* @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum() | |||
* @since POI 4.0.0 | |||
*/ | |||
public int getlastRowNum() { | |||
return _lastDefinedRow; | |||
} | |||
/* (non-JavaDoc), inherit JavaDoc from EvaluationWorkbook | |||
* @since POI 3.15 beta 3 | |||
*/ | |||
@Override | |||
public void clearAllCachedResultValues() { | |||
_cellCache = null; | |||
_lastDefinedRow = _xs.getLastRowNum(); | |||
} | |||
@Override | |||
public EvaluationCell getCell(int rowIndex, int columnIndex) { | |||
// shortcut evaluation if reference is outside the bounds of existing data | |||
// see issue #61841 for impact on VLOOKUP in particular | |||
if (rowIndex > _lastDefinedRow) return null; | |||
// cache for performance: ~30% speedup due to caching | |||
if (_cellCache == null) { | |||
_cellCache = new HashMap<>(_xs.getLastRowNum() * 3); |
@@ -0,0 +1,25 @@ | |||
package org.apache.poi.ss.formula.functions; | |||
import org.apache.poi.ss.usermodel.CellType; | |||
import org.apache.poi.ss.usermodel.FormulaEvaluator; | |||
import org.apache.poi.ss.usermodel.Workbook; | |||
import org.apache.poi.xssf.XSSFTestDataSamples; | |||
import org.junit.Test; | |||
import junit.framework.TestCase; | |||
/** | |||
* Test the VLOOKUP function | |||
*/ | |||
public class TestVlookup extends TestCase { | |||
@Test | |||
public void testFullColumnAreaRef61841() { | |||
final Workbook wb = XSSFTestDataSamples.openSampleWorkbook("VLookupFullColumn.xlsx"); | |||
FormulaEvaluator feval = wb.getCreationHelper().createFormulaEvaluator(); | |||
feval.evaluateAll(); | |||
assertEquals("Wrong lookup value", "Value1", feval.evaluate(wb.getSheetAt(0).getRow(3).getCell(1)).getStringValue()); | |||
assertEquals("Lookup should return #N/A", CellType.ERROR, feval.evaluate(wb.getSheetAt(0).getRow(4).getCell(1)).getCellType()); | |||
} | |||
} |