diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java index f1b8d7bf3e..841daa6d33 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationSheet.java @@ -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(); } } diff --git a/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java b/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java index 7abfce3c80..46ab79b10f 100644 --- a/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java +++ b/src/java/org/apache/poi/ss/formula/CellEvaluationFrame.java @@ -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) { diff --git a/src/java/org/apache/poi/ss/formula/EvaluationSheet.java b/src/java/org/apache/poi/ss/formula/EvaluationSheet.java index b3e60e2b3a..e155d0f68f 100644 --- a/src/java/org/apache/poi/ss/formula/EvaluationSheet.java +++ b/src/java/org/apache/poi/ss/formula/EvaluationSheet.java @@ -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(); } diff --git a/src/java/org/apache/poi/ss/formula/EvaluationTracker.java b/src/java/org/apache/poi/ss/formula/EvaluationTracker.java index 35b7f32ba0..a69386389c 100644 --- a/src/java/org/apache/poi/ss/formula/EvaluationTracker.java +++ b/src/java/org/apache/poi/ss/formula/EvaluationTracker.java @@ -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); diff --git a/src/java/org/apache/poi/ss/formula/FormulaUsedBlankCellSet.java b/src/java/org/apache/poi/ss/formula/FormulaUsedBlankCellSet.java index 69069f9056..18e88a8be9 100644 --- a/src/java/org/apache/poi/ss/formula/FormulaUsedBlankCellSet.java +++ b/src/java/org/apache/poi/ss/formula/FormulaUsedBlankCellSet.java @@ -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; diff --git a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java index 3c12dc5709..9ffbb8e09b 100644 --- a/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java +++ b/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java @@ -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; } diff --git a/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java b/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java index bc5fdf1741..698fda5649 100644 --- a/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java +++ b/src/java/org/apache/poi/ss/formula/eval/forked/ForkedEvaluationSheet.java @@ -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); diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java index 57d7cc57b9..ccb685690c 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java @@ -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(); } } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java index d8391ebf78..debb6c5dcb 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java @@ -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); diff --git a/src/ooxml/testcases/org/apache/poi/ss/formula/functions/TestVlookup.java b/src/ooxml/testcases/org/apache/poi/ss/formula/functions/TestVlookup.java new file mode 100644 index 0000000000..bfe31ea496 --- /dev/null +++ b/src/ooxml/testcases/org/apache/poi/ss/formula/functions/TestVlookup.java @@ -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()); + } + +} diff --git a/test-data/spreadsheet/VLookupFullColumn.xlsx b/test-data/spreadsheet/VLookupFullColumn.xlsx new file mode 100644 index 0000000000..cf6b82b809 Binary files /dev/null and b/test-data/spreadsheet/VLookupFullColumn.xlsx differ