ASF Bugzilla – Attachment 36319 Details for
Bug 62993
SUMIF returns wrong result (POI 4.0.x)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch with bugfix and testcase
file_62993.txt (text/plain), 8.72 KB, created by
Axel Howind
on 2018-12-10 16:58:36 UTC
(
hide
)
Description:
Patch with bugfix and testcase
Filename:
MIME Type:
Creator:
Axel Howind
Created:
2018-12-10 16:58:36 UTC
Size:
8.72 KB
patch
obsolete
>Index: src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java >=================================================================== >--- src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java (revision 1848549) >+++ src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationSheet.java (working copy) >@@ -34,11 +34,9 @@ > > 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() { >@@ -51,7 +49,7 @@ > */ > @Override > public int getLastRowNum() { >- return _lastDefinedRow; >+ return _xs.getLastRowNum(); > } > > /* (non-JavaDoc), inherit JavaDoc from EvaluationWorkbook >@@ -60,7 +58,6 @@ > @Override > public void clearAllCachedResultValues() { > _cellCache = null; >- _lastDefinedRow = _xs.getLastRowNum(); > } > > @Override >@@ -67,7 +64,9 @@ > 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; >+ if (rowIndex > getLastRowNum()) { >+ return null; >+ } > > // cache for performance: ~30% speedup due to caching > if (_cellCache == null) { >Index: src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java >=================================================================== >--- src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java (revision 1848549) >+++ src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java (working copy) >@@ -27,7 +27,6 @@ > import java.util.ArrayList; > import java.util.Collection; > import java.util.Collections; >-import java.util.Comparator; > import java.util.HashMap; > import java.util.Iterator; > import java.util.LinkedHashMap; >@@ -131,7 +130,6 @@ > import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTTablePart; > import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTTableParts; > import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTWorksheet; >-import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTWorksheetSource; > import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCalcMode; > import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCellFormulaType; > import org.openxmlformats.schemas.spreadsheetml.x2006.main.STPane; >@@ -165,6 +163,7 @@ > protected CTWorksheet worksheet; > > private final SortedMap<Integer, XSSFRow> _rows = new TreeMap<>(); >+ private int _lastRowNum=0; > private List<XSSFHyperlink> hyperlinks; > private ColumnHelper columnHelper; > private CommentsTable sheetComments; >@@ -268,9 +267,9 @@ > arrayFormulas = new ArrayList<>(); > for (CTRow row : worksheetParam.getSheetData().getRowArray()) { > XSSFRow r = new XSSFRow(row, this); >- // Performance optimization: explicit boxing is slightly faster than auto-unboxing, though may use more memory >- final Integer rownumI = Integer.valueOf(r.getRowNum()); // NOSONAR >- _rows.put(rownumI, r); >+ int rownum = r.getRowNum(); >+ _rows.put(rownum, r); >+ _lastRowNum = Math.max(_lastRowNum, rownum); > } > } > >@@ -782,6 +781,7 @@ > XSSFRow r = new XSSFRow(ctRow, this); > r.setRowNum(rownum); > _rows.put(rownumI, r); >+ _lastRowNum = Math.max(_lastRowNum, rownum); > return r; > } > >@@ -1211,7 +1211,7 @@ > > @Override > public int getLastRowNum() { >- return _rows.isEmpty() ? 0 : _rows.lastKey(); >+ return _lastRowNum; > } > > @Override >@@ -2016,6 +2016,7 @@ > // this is not the physical row number! > final int idx = _rows.headMap(rowNumI).size(); > _rows.remove(rowNumI); >+ _lastRowNum = _rows.isEmpty() ? 0 : _rows.lastKey(); > worksheet.getSheetData().removeRow(idx); > > // also remove any comment located in that row >@@ -3058,10 +3059,12 @@ > //rebuild the _rows map > List<XSSFRow> rowList = new ArrayList<>(_rows.values()); > _rows.clear(); >+ _lastRowNum = 0; > for(XSSFRow r : rowList) { > // Performance optimization: explicit boxing is slightly faster than auto-unboxing, though may use more memory >- final Integer rownumI = new Integer(r.getRowNum()); // NOSONAR >- _rows.put(rownumI, r); >+ int rownum = r.getRowNum(); >+ _rows.put(rownum, r); >+ _lastRowNum = Math.max(_lastRowNum, rownum); > } > } > >Index: src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java >=================================================================== >--- src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java (revision 1848549) >+++ src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java (working copy) >@@ -40,6 +40,7 @@ > import java.util.HashSet; > import java.util.List; > import java.util.Map; >+import java.util.Map.Entry; > import java.util.Set; > import java.util.TimeZone; > import java.util.TreeMap; >@@ -103,6 +104,7 @@ > import org.apache.poi.ss.usermodel.Name; > import org.apache.poi.ss.usermodel.PrintSetup; > import org.apache.poi.ss.usermodel.Row; >+import org.apache.poi.ss.usermodel.Row.MissingCellPolicy; > import org.apache.poi.ss.usermodel.Sheet; > import org.apache.poi.ss.usermodel.SheetConditionalFormatting; > import org.apache.poi.ss.usermodel.Workbook; >@@ -3388,4 +3390,80 @@ > assertEquals("E11", sheet.getActiveCell().formatAsString()); > wbBack.close(); > } >+ >+ @Test >+ public void bug62993() throws IOException { >+ try (Workbook wb = WorkbookFactory.create(true)) { >+ FormulaEvaluator evaluator = wb.getCreationHelper() >+ .createFormulaEvaluator(); >+ >+ Sheet sheet = wb.createSheet("test SUM_IF"); >+ >+ int r = 0; >+ >+ Row row = sheet.createRow(r++); >+ row.createCell(0).setCellValue("Key"); >+ row.createCell(1).setCellValue("Value"); >+ >+ double[] keys = { 1.75, 2.25, 3.25, 1.75, 3.25, 3.25, 0.9, 1.25 }; >+ double[] values = { 123, 456, 789, 10, 20, 30, 40, 50 }; >+ assert keys.length == values.length; >+ >+ final int n = keys.length; >+ Map<Double, Double> expected = new HashMap<>(); >+ >+ final int firstRow = r + 1; >+ final int lastRow = r + n; >+ >+ for (int i = 0; i < keys.length; i++) { >+ // write data to sheet >+ Double key = keys[i]; >+ Double value = values[i]; >+ >+ row = sheet.createRow(r++); >+ row.getCell(0, MissingCellPolicy.CREATE_NULL_AS_BLANK) >+ .setCellValue(key); >+ row.getCell(1, MissingCellPolicy.CREATE_NULL_AS_BLANK) >+ .setCellValue(value); >+ >+ // update sums (if key is not present or mapped to null, store >+ // value. Otherwise store sum.) >+ expected.merge(key, value, (a, b) -> a + b); >+ } >+ >+ // insert a blank row >+ row = sheet.createRow(r++); >+ >+ String keyRange = String.format("%1$s%2$d:%1$s%3$d", "A", firstRow, >+ lastRow); >+ String valueRange = String.format("%1$s%2$d:%1$s%3$d", "B", >+ firstRow, lastRow); >+ >+ row = sheet.createRow(r++); >+ row.createCell(0).setCellValue("Key"); >+ row.createCell(1).setCellValue("Sum"); >+ >+ for (Entry<Double, Double> entry : expected.entrySet()) { >+ row = sheet.createRow(r++); >+ >+ Cell cellKey = row.createCell(0); >+ Cell cellFormula = row.createCell(1); >+ >+ String formula = String.format("SUMIF(%s,%s,%s)", keyRange, >+ cellKey.getAddress(), valueRange); >+ >+ double resultExpected = entry.getValue(); >+ >+ cellKey.setCellValue(entry.getKey()); >+ cellFormula.setCellFormula(formula); >+ >+ CellType resultType = evaluator >+ .evaluateFormulaCell(cellFormula); >+ assertEquals(CellType.NUMERIC, resultType); >+ >+ double resultPoi = cellFormula.getNumericCellValue(); >+ assertEquals(resultExpected, resultPoi, Double.MIN_VALUE); >+ } >+ } >+ } > } >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 62993
:
36312
|
36313
|
36315
| 36319