Created attachment 28636 [details] Spreadsheet for use with the test case. Overview: Summing a range of cells greater the 255 gives an incorrect result. Steps to reproduce: Create an .xlsx, fill a range of columns, eg B1:IZ1 with a value, say "1" Insert a formula to sum these cells, ie SUM(B1:IZ1) Using POI, read the spreadsheet and evaluate the formula. Expected result: 259.0, actual result: 258.0 Example test case: import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.ss.usermodel.FormulaEvaluator; import org.apache.poi.ss.usermodel.Workbook; import org.apache.poi.ss.usermodel.WorkbookFactory; import org.junit.Test; import java.io.IOException; import java.io.InputStream; import static org.junit.Assert.assertEquals; public class PoiTest { @Test public void evaluateExcelUsingPoiApiOnly_expectCorrectEvaluation() { // Arrange InputStream inputStream = this.getClass().getResourceAsStream("/test.xlsx"); Workbook workbook = null; try { workbook = WorkbookFactory.create(inputStream); } catch (IOException e) { e.printStackTrace(); } catch (InvalidFormatException e) { e.printStackTrace(); } finally { try { inputStream.close(); } catch (IOException e) { e.printStackTrace(); } } // Act // evaluate SUM('Skye Lookup Input'!B2:IZ2), cells in range B2:IZ2 each contain "1" FormulaEvaluator evaluator = workbook.getCreationHelper().createFormulaEvaluator(); double numericValue = evaluator.evaluate(workbook.getSheetAt(0).getRow(0.getCell(0)).getNumberValue(); // Assert assertEquals(259.0, numericValue, 0.0); } } Attached is a test.xlsx
There's a typo here: evaluator.evaluate(workbook.getSheetAt(0).getRow(0.getCell(0)).getNumberValue(); It should be: evaluator.evaluate(workbook.getSheetAt(0).getRow(0).getCell(0)).getNumberValue(); apologies...
I have a fix for this. I've modified pgetRelativeValue(..) in LazyAreaEval as follows: public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) { int rowIx = (relativeRowIndex + getFirstRow() ) & 0xFFFF; int colIx = (relativeColumnIndex + getFirstColumn() ) & 0x3FFF; return _evaluator.getEvalForCell(rowIx, colIx); } The existing code contains: int colIx = (relativeColumnIndex + getFirstColumn() ) & 0x00FF; which assigns 0 to colIx if relativeColumnIndex + getFirstColumn() > 255, thus cell numbers > 255 are not evaluated.
Would the proposed fix break things for XLS (HSSF) files though? We do have a set of constants available for the maximum number of rows, columns etc. I wonder if the masking should be set based on the max column count, rather than hard coded?
I confirmed the problem in trunk. At first glance the fix looks safe, at least it does not break existing tests and fixes evaluation of the attached workbook. I'm leaving this bug as NEW. LazyEval is pretty fundamental and I'd like to research it deeper before applying the proposed fix. Yegor
It might be a good idea to look at rowIdx as well: int rowIx = (relativeRowIndex + getFirstRow() ) & 0xFFFF; as this limits the number of rows to 65535.
How about removing the &'s altogether?: public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) { int rowIx = relativeRowIndex + getFirstRow(); int colIx = relativeColumnIndex + getFirstColumn(); return _evaluator.getEvalForCell(rowIx, colIx); } This passes all the test as well.
I thought about removing all &'s too. The purpose of these operators is to guarantee that the the result fits in the .xls grid bounds (2^8 cols and 2^16 rows). We don't need them in general case, but I want research the problem and make sure it is really so. Yegor (In reply to comment #6) > How about removing the &'s altogether?: > > public ValueEval getRelativeValue(int relativeRowIndex, int > relativeColumnIndex) { > > int rowIx = relativeRowIndex + getFirstRow(); > int colIx = relativeColumnIndex + getFirstColumn(); > > return _evaluator.getEvalForCell(rowIx, colIx); > } > > This passes all the test as well.
Fixed in r1328647, junit added. Regards, Yegor
Cheers!