Skip to content

Commit b2f805f

Browse files
committed
Bug #61841 - Unnecessary long computation when evaluating VLOOKUP on all column reference
1 parent 3082b81 commit b2f805f

File tree

11 files changed

+136
-12
lines changed

11 files changed

+136
-12
lines changed

main/HSSF/UserModel/HSSFEvaluationSheet.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,12 @@ public class HSSFEvaluationSheet : IEvaluationSheet
2929
{
3030

3131
private readonly HSSFSheet _hs;
32+
private int _lastDefinedRow = -1;
3233

3334
public HSSFEvaluationSheet(HSSFSheet hs)
3435
{
3536
_hs = hs;
37+
_lastDefinedRow = _hs.LastRowNum;
3638
}
3739

3840
public HSSFSheet HSSFSheet
@@ -42,6 +44,19 @@ public HSSFSheet HSSFSheet
4244
return _hs;
4345
}
4446
}
47+
48+
/* (non-Javadoc)
49+
* @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum()
50+
* @since POI 4.0.0
51+
*/
52+
public int LastRowNum
53+
{
54+
get
55+
{
56+
return _lastDefinedRow;
57+
}
58+
}
59+
4560
public IEvaluationCell GetCell(int rowIndex, int columnIndex)
4661
{
4762
HSSFRow row = (HSSFRow)_hs.GetRow(rowIndex);
@@ -59,7 +74,7 @@ public IEvaluationCell GetCell(int rowIndex, int columnIndex)
5974

6075
public void ClearAllCachedResultValues()
6176
{
62-
// nothing to do
77+
_lastDefinedRow = _hs.LastRowNum;
6378
}
6479
}
6580
}

main/SS/Formula/CellEvaluationFrame.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,13 @@ private CellCacheEntry[] GetSensitiveInputCells()
7171
}
7272
return _sensitiveInputCells.ToArray();
7373
}
74-
public void AddUsedBlankCell(int bookIndex, int sheetIndex, int rowIndex, int columnIndex)
74+
public void AddUsedBlankCell(IEvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex, int rowIndex, int columnIndex)
7575
{
7676
if (_usedBlankCellGroup == null)
7777
{
7878
_usedBlankCellGroup = new FormulaUsedBlankCellSet();
7979
}
80-
_usedBlankCellGroup.AddCell(bookIndex, sheetIndex, rowIndex, columnIndex);
80+
_usedBlankCellGroup.AddCell(evalWorkbook, bookIndex, sheetIndex, rowIndex, columnIndex);
8181
}
8282

8383
public void UpdateFormulaResult(ValueEval result)

main/SS/Formula/Eval/Forked/ForkedEvaluationSheet.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ public ForkedEvaluationSheet(IEvaluationSheet masterSheet)
4747
_sharedCellsByRowCol = new Dictionary<RowColKey, ForkedEvaluationCell>();
4848
}
4949

50+
/* (non-Javadoc)
51+
* @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum()
52+
* @since POI 4.0.0
53+
*/
54+
public int LastRowNum
55+
{
56+
get
57+
{
58+
return _masterSheet.LastRowNum;
59+
}
60+
}
61+
5062
public IEvaluationCell GetCell(int rowIndex, int columnIndex)
5163
{
5264
RowColKey key = new RowColKey(rowIndex, columnIndex);

main/SS/Formula/EvaluationSheet.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,11 @@ public interface IEvaluationSheet
3939
* @see EvaluationWorkbook#clearAllCachedResultValues()
4040
*/
4141
void ClearAllCachedResultValues();
42+
43+
/**
44+
* @return last row index referenced on this sheet, for evaluation optimization
45+
* @since POI 4.0.0
46+
*/
47+
public int LastRowNum { get; }
4248
}
4349
}

main/SS/Formula/EvaluationTracker.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public void AcceptFormulaDependency(CellCacheEntry cce)
133133
}
134134
}
135135

136-
public void AcceptPlainValueDependency(int bookIndex, int sheetIndex,
136+
public void AcceptPlainValueDependency(IEvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex,
137137
int rowIndex, int columnIndex, ValueEval value)
138138
{
139139
// Tell the currently evaluating cell frame that it Has a dependency on the specified
@@ -147,7 +147,7 @@ public void AcceptPlainValueDependency(int bookIndex, int sheetIndex,
147147
CellEvaluationFrame consumingFrame = (CellEvaluationFrame)_evaluationFrames[prevFrameIndex];
148148
if (value == BlankEval.instance)
149149
{
150-
consumingFrame.AddUsedBlankCell(bookIndex, sheetIndex, rowIndex, columnIndex);
150+
consumingFrame.AddUsedBlankCell(evalWorkbook, bookIndex, sheetIndex, rowIndex, columnIndex);
151151
}
152152
else
153153
{

main/SS/Formula/FormulaUsedBlankCellSet.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,19 @@ private class BlankCellSheetGroup
6262
private int _firstColumnIndex;
6363
private int _lastColumnIndex;
6464
private BlankCellRectangleGroup _currentRectangleGroup;
65+
private int _lastDefinedRow;
6566

66-
public BlankCellSheetGroup()
67+
public BlankCellSheetGroup(int lastDefinedRow)
6768
{
6869
_rectangleGroups = [];
6970
_currentRowIndex = -1;
71+
_lastDefinedRow = lastDefinedRow;
7072
}
7173

7274
public void AddCell(int rowIndex, int columnIndex)
7375
{
76+
if(rowIndex > _lastDefinedRow)
77+
return;
7478
if (_currentRowIndex == -1)
7579
{
7680
_currentRowIndex = rowIndex;
@@ -107,6 +111,8 @@ public void AddCell(int rowIndex, int columnIndex)
107111

108112
public bool ContainsCell(int rowIndex, int columnIndex)
109113
{
114+
if(rowIndex > _lastDefinedRow)
115+
return true;
110116
for (int i = _rectangleGroups.Count - 1; i >= 0; i--)
111117
{
112118
BlankCellRectangleGroup bcrg = (BlankCellRectangleGroup)_rectangleGroups[i];
@@ -203,20 +209,20 @@ public FormulaUsedBlankCellSet()
203209
_sheetGroupsByBookSheet = new Hashtable();
204210
}
205211

206-
public void AddCell(int bookIndex, int sheetIndex, int rowIndex, int columnIndex)
212+
public void AddCell(IEvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex, int rowIndex, int columnIndex)
207213
{
208-
BlankCellSheetGroup sbcg = GetSheetGroup(bookIndex, sheetIndex);
214+
BlankCellSheetGroup sbcg = GetSheetGroup(evalWorkbook, bookIndex, sheetIndex);
209215
sbcg.AddCell(rowIndex, columnIndex);
210216
}
211217

212-
private BlankCellSheetGroup GetSheetGroup(int bookIndex, int sheetIndex)
218+
private BlankCellSheetGroup GetSheetGroup(IEvaluationWorkbook evalWorkbook, int bookIndex, int sheetIndex)
213219
{
214220
BookSheetKey key = new BookSheetKey(bookIndex, sheetIndex);
215221

216222
BlankCellSheetGroup result = (BlankCellSheetGroup)_sheetGroupsByBookSheet[key];
217223
if (result == null)
218224
{
219-
result = new BlankCellSheetGroup();
225+
result = new BlankCellSheetGroup(evalWorkbook.GetSheet(sheetIndex).LastRowNum);
220226
_sheetGroupsByBookSheet[key]= result;
221227
}
222228
return result;

main/SS/Formula/WorkbookEvaluator.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ private ValueEval EvaluateAny(IEvaluationCell srcCell, int sheetIndex,
407407
result = GetValueFromNonFormulaCell(srcCell);
408408
if (shouldCellDependencyBeRecorded)
409409
{
410-
tracker.AcceptPlainValueDependency(_workbookIx, sheetIndex, rowIndex, columnIndex, result);
410+
tracker.AcceptPlainValueDependency(_workbook, _workbookIx, sheetIndex, rowIndex, columnIndex, result);
411411
}
412412
return result;
413413
}

ooxml/XSSF/Streaming/SXSSFEvaluationSheet.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,31 @@ namespace NPOI.XSSF.Streaming
2222
public class SXSSFEvaluationSheet : IEvaluationSheet //XSSFEvaluationSheet
2323
{
2424
private readonly SXSSFSheet _xs;
25+
private int _lastDefinedRow = -1;
2526

2627
public SXSSFEvaluationSheet(SXSSFSheet sheet)
2728
{
2829
_xs = sheet;
30+
_lastDefinedRow = _xs.LastRowNum;
2931
}
3032

3133
public SXSSFSheet GetSXSSFSheet()
3234
{
3335
return _xs;
3436
}
3537

38+
/* (non-Javadoc)
39+
* @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum()
40+
* @since POI 4.0.0
41+
*/
42+
public int LastRowNum
43+
{
44+
get
45+
{
46+
return _lastDefinedRow;
47+
}
48+
}
49+
3650
public IEvaluationCell GetCell(int rowIndex, int columnIndex)
3751
{
3852
SXSSFRow row = (SXSSFRow)_xs.GetRow(rowIndex);
@@ -58,7 +72,7 @@ public IEvaluationCell GetCell(int rowIndex, int columnIndex)
5872

5973
public void ClearAllCachedResultValues()
6074
{
61-
// nothing to do
75+
_lastDefinedRow = _xs.LastRowNum;
6276
}
6377
}
6478
}

ooxml/XSSF/UserModel/XSSFEvaluationSheet.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,26 +33,46 @@ public class XSSFEvaluationSheet : IEvaluationSheet
3333

3434
private readonly XSSFSheet _xs;
3535
private Dictionary<CellKey, IEvaluationCell> _cellCache;
36+
private int _lastDefinedRow = -1;
3637

3738
public XSSFEvaluationSheet(ISheet sheet)
3839
{
3940
_xs = (XSSFSheet)sheet;
41+
_lastDefinedRow = _xs.LastRowNum;
4042
}
4143

4244
public XSSFEvaluationSheet()
4345
{
4446

4547
}
4648

49+
/* (non-Javadoc)
50+
* @see org.apache.poi.ss.formula.EvaluationSheet#getlastRowNum()
51+
* @since POI 4.0.0
52+
*/
53+
public int LastRowNum
54+
{
55+
get
56+
{
57+
return _lastDefinedRow;
58+
}
59+
}
60+
4761
public void ClearAllCachedResultValues()
4862
{
4963
_cellCache = null;
64+
_lastDefinedRow = _xs.LastRowNum;
5065
}
5166

5267
public XSSFSheet XSSFSheet => _xs;
5368

5469
public IEvaluationCell GetCell(int rowIndex, int columnIndex)
5570
{
71+
// shortcut evaluation if reference is outside the bounds of existing data
72+
// see issue #61841 for impact on VLOOKUP in particular
73+
if(rowIndex > _lastDefinedRow)
74+
return null;
75+
5676
// cache for performance: ~30% speedup due to caching
5777
if (_cellCache == null)
5878
{
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/* ====================================================================
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
==================================================================== */
17+
18+
19+
using System;
20+
using System.Collections;
21+
using System.Collections.Generic;
22+
using System.IO;
23+
using System.Text;
24+
25+
namespace TestCases.SS.Formula.Functions
26+
{
27+
using NPOI.SS.UserModel;
28+
using NPOI.XSSF;
29+
using NUnit.Framework;
30+
using NUnit.Framework.Legacy;
31+
32+
/// <summary>
33+
/// Test the VLOOKUP function
34+
/// </summary>
35+
[TestFixture]
36+
public class TestVlookup
37+
{
38+
39+
[Test]
40+
public void TestFullColumnAreaRef61841()
41+
{
42+
IWorkbook wb = XSSFTestDataSamples.OpenSampleWorkbook("VLookupFullColumn.xlsx");
43+
IFormulaEvaluator feval = wb.GetCreationHelper().CreateFormulaEvaluator();
44+
feval.EvaluateAll();
45+
ClassicAssert.AreEqual("Value1", feval.Evaluate(wb.GetSheetAt(0).GetRow(3).GetCell(1)).StringValue, "Wrong lookup value");
46+
ClassicAssert.AreEqual(CellType.Error, feval.Evaluate(wb.GetSheetAt(0).GetRow(4).GetCell(1)).CellType, "Lookup should return #N/A");
47+
}
48+
49+
}
50+
}
51+

0 commit comments

Comments
 (0)