Skip to content

Commit 796e483

Browse files
OssianEPPlusOssianEPPlus
authored
Bug/s870 (#2013)
* Added alternative binary search in comments * Attempt at solution based on existing values * Dimension by range start * Functional lookup binary * Fixed failing tests * Comment cleanup * Broke out solution to static class * Changed to simpleAddress * fixed bugs * Removed unnecesary test * fixed comment. Clarified name
1 parent e33d070 commit 796e483

File tree

7 files changed

+215
-24
lines changed

7 files changed

+215
-24
lines changed

‎src/EPPlus/Core/CellStore/CellStore.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,7 +1175,6 @@ public void Dispose()
11751175

11761176
internal bool NextCell(ref int row, ref int col)
11771177
{
1178-
11791178
return NextCell(ref row, ref col, 0, 0, ExcelPackage.MaxRows, ExcelPackage.MaxColumns);
11801179
}
11811180
internal bool NextCell(ref int row, ref int col, int minRow, int minColPos, int maxRow, int maxColPos)
@@ -1590,7 +1589,7 @@ internal bool GetPrevCell(ref int row, ref int colPos, int startRow, int startCo
15901589
}
15911590
else
15921591
{
1593-
return GetPrevCell(ref colPos, ref row, startRow, startColPos, endColPos);
1592+
return GetPrevCell(ref row, ref colPos, startRow, startColPos, endColPos);
15941593
}
15951594
}
15961595
}

‎src/EPPlus/Drawing/Chart/ExcelChartAxis.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -528,12 +528,11 @@ public void RemoveTitle()
528528
/// Or Category Axis to Serie Axis and vice versa.
529529
/// </summary>
530530
/// <param name="type"></param>
531-
/// <param name="throwWarning">Set to false to allow axisTypes with unexpected behaviour</param>
531+
/// <param name="throwException">Set to false to allow axisTypes with unexpected behaviour</param>
532532
/// <exception cref="InvalidOperationException"></exception>
533-
/// <exception cref="InvalidOperationException"></exception>
534-
public void ChangeAxisType(eAxisType type, bool throwWarning = true)
533+
public void ChangeAxisType(eAxisType type, bool throwException = true)
535534
{
536-
if (throwWarning && _chart.IsAxisTypeSupported(type, this) == false)
535+
if (throwException && _chart.IsAxisTypeSupported(type, this) == false)
537536
{
538537
throw new InvalidOperationException($"Chart Name:{_chart.Name} of Type: {_chart.ChartType.ToEnumString()} " +
539538
$"Cannot change axis with ID:{this.Id} from AxisType:{AxisType} To Type: {type.ToString()}. Data would be missrepresented in chart.");

‎src/EPPlus/FormulaParsing/Excel/Functions/RefAndLookup/LookupUtils/LookupBinarySearch.cs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,25 @@ Date Author Change
1010
*************************************************************************************************
1111
22/3/2023 EPPlus Software AB EPPlus v7
1212
*************************************************************************************************/
13-
using System;
13+
14+
using OfficeOpenXml.Utils;
1415
using System.Collections.Generic;
15-
using System.Linq;
16-
using System.Text;
1716

1817
namespace OfficeOpenXml.FormulaParsing.Excel.Functions.RefAndLookup.LookupUtils
1918
{
2019
internal static class LookupBinarySearch
2120
{
2221
private static int SearchAsc(object s, IRangeInfo lookupRange, IComparer<object> comparer, LookupRangeDirection? direction = null)
2322
{
24-
var nRows = lookupRange.Size.NumberOfRows;
25-
var nCols = lookupRange.Size.NumberOfCols;
23+
//Only look at relevant values. We use this to omit null values above and below actual values
24+
var valueRange = ValueFinder.RangeByValue(lookupRange.Worksheet, lookupRange.Address);
25+
26+
var nRows = valueRange.ToRow - valueRange.FromRow + 1;
27+
var nCols = valueRange.ToCol - valueRange.FromCol + 1;
28+
2629
if (nRows == 0 && nCols == 0) return -1;
2730
int low = 0, high = nCols > nRows ? nCols : nRows, mid;
28-
if(direction.HasValue)
31+
if (direction.HasValue)
2932
{
3033
high = direction.Value == LookupRangeDirection.Vertical ? nRows : nCols;
3134
}
@@ -38,18 +41,17 @@ private static int SearchAsc(object s, IRangeInfo lookupRange, IComparer<object>
3841
var row = nRows >= nCols ? mid : 0;
3942
if (direction.HasValue)
4043
{
41-
4244
col = direction.Value == LookupRangeDirection.Vertical ? 0 : mid;
4345
row = direction.Value == LookupRangeDirection.Vertical ? mid : 0;
4446
}
4547

4648
//Row and col are 0-based if equal we will be past the last value due to GetOffset
47-
if(row == nRows || col == nCols)
49+
if (row == nRows || col == nCols)
4850
{
4951
break;
5052
}
51-
52-
var val = lookupRange.GetOffset(row, col);
53+
54+
var val = lookupRange.GetValue(valueRange.FromRow + row, valueRange.FromCol + col);
5355

5456
var result = comparer.Compare(s, val);
5557

@@ -60,8 +62,9 @@ private static int SearchAsc(object s, IRangeInfo lookupRange, IComparer<object>
6062
low = mid + 1;
6163

6264
else
63-
return mid;
65+
return valueRange.FromRow - 1 + mid;
6466
}
67+
6568
return ~low;
6669
}
6770

‎src/EPPlus/Utils/ValueFinder.cs

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
using OfficeOpenXml.FormulaParsing.Excel.Functions.Information;
2+
using OfficeOpenXml.FormulaParsing.Excel.Functions.RefAndLookup;
3+
using OfficeOpenXml.FormulaParsing.LexicalAnalysis;
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Linq;
7+
using System.Text;
8+
9+
10+
namespace OfficeOpenXml.Utils
11+
{
12+
internal static class ValueFinder
13+
{
14+
internal static List<int> FirstValueCell(ExcelWorksheet sheet, FormulaRangeAddress address)
15+
{
16+
var fromRow = address.FromRow;
17+
var fromCol = address.FromCol;
18+
var someValue = sheet._values.GetValue(address.FromRow, address.FromCol);
19+
var valueOfValue = sheet._values.GetValue(address.FromRow, address.FromCol)._value;
20+
if (valueOfValue == null)
21+
{
22+
while (sheet._values.NextCell(ref fromRow, ref fromCol, address.FromRow, address.FromCol, address.ToRow, address.ToCol))
23+
{
24+
if (sheet._values.GetValue(fromRow, fromCol)._value != null)
25+
{
26+
return new List<int> { fromRow, fromCol - 1 };
27+
}
28+
}
29+
}
30+
return new List<int> { fromRow, fromCol };
31+
}
32+
33+
internal static List<int> LastValueCell(ExcelWorksheet sheet, FormulaRangeAddress address)
34+
{
35+
//The range might refer to cells outside the worksheet dimension if no value has been set.
36+
//Therefore take the lower value between toRow and toCol and the dimension version of them
37+
var toRow = sheet.Dimension._toRow < address.ToRow ? sheet.Dimension._toRow : address.ToRow;
38+
var toCol = sheet.Dimension._toCol < address.ToCol ? sheet.Dimension._toCol : address.ToCol;
39+
40+
if (sheet._values.GetValue(toRow, toCol)._value == null)
41+
{
42+
while (sheet._values.PrevCell(ref toRow, ref toCol) && toRow > 0)
43+
{
44+
if (toCol > 0 && sheet._values.GetValue(toRow, toCol)._value != null)
45+
{
46+
return new List<int> { toRow, toCol };
47+
}
48+
}
49+
return null;
50+
}
51+
return new List<int> { toRow, toCol };
52+
}
53+
54+
internal static SimpleAddress RangeByValue(ExcelWorksheet sheet, FormulaRangeAddress address)
55+
{
56+
var fvc = FirstValueCell(sheet, address);
57+
var lvc = LastValueCell(sheet, address);
58+
59+
var fromRow = fvc[0];
60+
var toRow = lvc[0];
61+
int fromCol, toCol;
62+
63+
if (fvc[1] == address.FromRow)
64+
{
65+
fromCol = fvc[1];
66+
}
67+
else
68+
{
69+
int r = fromRow, c = address.FromCol;
70+
while (sheet._values.NextCellByColumn(ref r, ref c, fromRow, toRow, address.ToCol - address.FromCol))
71+
{
72+
if (sheet._values.GetValue(r, c)._value != null)
73+
{
74+
break;
75+
}
76+
r++;
77+
}
78+
fromCol = c;
79+
}
80+
81+
if (lvc[1] == address.ToCol)
82+
{
83+
toCol = lvc[1];
84+
}
85+
else
86+
{
87+
int r = toRow, c = address.ToCol;
88+
while (sheet._values.PrevCellByColumn(ref r, ref c, fromRow, toRow, address.ToCol - address.FromCol))
89+
{
90+
if (sheet._values.GetValue(r, c)._value != null)
91+
{
92+
break;
93+
}
94+
r--;
95+
}
96+
toCol = c;
97+
}
98+
99+
SimpleAddress subRange = new SimpleAddress { FromRow = Math.Min(fromRow, toRow), FromCol = Math.Min(fromCol, toCol), ToRow = Math.Max(fromRow, toRow), ToCol = Math.Max(fromCol, toCol) };
100+
return subRange;
101+
}
102+
}
103+
}

‎src/EPPlusTest/FormulaParsing/Excel/Functions/RefAndLookup/RefAndLookupTests.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,6 @@ public void LookupShouldReturnResultFromMatchingSecondArrayHorizontal()
370370
sheet.Calculate();
371371
var result = sheet.Cells["D1"].Value;
372372
Assert.AreEqual("B", result);
373-
374373
}
375374
}
376375

‎src/EPPlusTest/FormulaParsing/Excel/Functions/RefAndLookup/VLookupTests.cs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Microsoft.VisualStudio.TestTools.UnitTesting;
22
using OfficeOpenXml;
3+
using System.Collections.Generic;
34

45
namespace EPPlusTest.FormulaParsing.Excel.Functions.RefAndLookup
56
{
@@ -290,6 +291,91 @@ public void ApproximateStringsShouldFind()
290291

291292
// }
292293
//}
294+
295+
[TestMethod]
296+
public void SC870_EpplusOnly()
297+
{
298+
using (var p = OpenPackage("EpplusNullAboveAndBelow.xlsx", true))
299+
{
300+
var wb = p.Workbook;
301+
var ws = wb.Worksheets.Add("VLookupTest");
302+
List<int> searchValues = new List<int> { 1, 2, 4, 7, 11, 16, 21, 27 };
303+
List<int> resultValues = new List<int> { 400, 365, 315, 280, 250, 215, 200, 170 };
304+
305+
ws.Cells["B6:B13"].LoadFromCollection(searchValues);
306+
ws.Cells["C6:C13"].LoadFromCollection(resultValues);
307+
308+
ws.Cells["A11"].Value = 1;
309+
310+
//Testing that VLookup (or rather binary search lookup) can handle values of 'null' in a range above and below target.
311+
ws.Cells["F6"].Formula = "VLOOKUP(A11, B:C, 2, TRUE)";
312+
313+
ws.Calculate();
314+
315+
var outputValue = ws.Cells["F6"].Value;
316+
Assert.AreEqual(400, outputValue);
317+
318+
//Ensure it works for each of the values
319+
for (int i = 1; i < searchValues.Count; i++)
320+
{
321+
var formulaCell = ws.Cells[6 + i, 6];
322+
formulaCell.Formula = $"VLOOKUP({searchValues[i]}, B:C, 2, TRUE)";
323+
formulaCell.Calculate();
324+
Assert.AreEqual(resultValues[i], formulaCell.Value);
325+
}
326+
327+
//Save Workbook
328+
SaveAndCleanup(p);
329+
}
330+
}
331+
332+
[TestMethod]
333+
public void SC870()
334+
{
335+
using (var package = OpenTemplatePackage("s870.xlsx"))
336+
{
337+
var wb = package.Workbook;
338+
var worksheet = package.Workbook.Worksheets[0];
339+
340+
foreach (var sheet in package.Workbook.Worksheets)
341+
{
342+
sheet.Hidden = eWorkSheetHidden.Visible;
343+
}
344+
345+
worksheet.Cells["F15"].Formula = "VLOOKUP(B11, Salgsfragt!B:C, 2, TRUE)";
346+
347+
var sWs = package.Workbook.Worksheets.GetByName("Salgsfragt");
348+
sWs.Cells["B4"].Value = null;
349+
sWs.Cells["B2"].Value = null;
350+
351+
worksheet.Cells["F15"].Calculate();
352+
353+
var someVal = worksheet.Cells["F15"].Value;
354+
var errorText = worksheet.Cells["D8"].Text;
355+
356+
var cellEuItemPrice = worksheet.Cells["C18"];
357+
var cellEuTransportPrice = worksheet.Cells["C19"];
358+
var cellEuTotal = worksheet.Cells["C20"];
359+
360+
var cellDKItemPrice = worksheet.Cells["C24"];
361+
var cellDKTransportPrice = worksheet.Cells["C25"];
362+
var cellDKTotal = worksheet.Cells["C26"];
363+
364+
worksheet.Calculate();
365+
decimal tolerance = 0.1M;
366+
367+
Assert.AreEqual(301.01M, (decimal)cellEuItemPrice.GetCellValue<double>(), tolerance);
368+
Assert.AreEqual(53.62M, (decimal)cellEuTransportPrice.GetCellValue<double>(), tolerance);
369+
Assert.AreEqual(354.62M, (decimal)cellEuTotal.GetCellValue<double>(), tolerance);
370+
371+
Assert.AreEqual(2245.50M, (decimal)cellDKItemPrice.GetCellValue<double>(), tolerance);
372+
Assert.AreEqual(400M, (decimal)cellDKTransportPrice.GetCellValue<double>(), tolerance);
373+
Assert.AreEqual(2645.50M, (decimal)cellDKTotal.GetCellValue<double>(), tolerance);
374+
375+
SaveAndCleanup(package);
376+
}
377+
}
378+
293379
[TestMethod]
294380
public void PriorAddressExpressionWorksheetShouldBeCleared()
295381
{

‎src/EPPlusTest/Issues/ChartIssues.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
using Microsoft.VisualStudio.TestTools.UnitTesting;
2+
using OfficeOpenXml;
23
using OfficeOpenXml.Drawing;
34
using OfficeOpenXml.Drawing.Chart;
45
using OfficeOpenXml.Drawing.Chart.Style;
5-
using OfficeOpenXml;
6-
using System.IO;
7-
using System.Drawing;
8-
using System.Text;
96
using System;
7+
using System.Collections.Generic;
8+
using System.Drawing;
9+
using System.Globalization;
10+
using System.IO;
1011
using System.Security.Principal;
12+
using System.Text;
1113

1214
namespace EPPlusTest.Issues
1315
{
@@ -386,9 +388,9 @@ public void i886()
386388
}
387389

388390
[TestMethod]
389-
public void CreateStringLitterals()
391+
public void CreateStringLiterals()
390392
{
391-
using (var package = OpenPackage("LitteralsSetting.xlsx", true))
393+
using (var package = OpenPackage("LiteralsSetting.xlsx", true))
392394
{
393395
var wb = package.Workbook;
394396
var ws = wb.Worksheets.Add("NewWork");

0 commit comments

Comments
 (0)