Bug 51448 - Evaluation Cache gets messed up after 256 sheets
Summary: Evaluation Cache gets messed up after 256 sheets
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: POI Overall (show other bugs)
Version: 3.8-dev
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-29 07:07 UTC by Antti Koskimäki
Modified: 2011-07-02 13:36 UTC (History)
0 users



Attachments
PlainCellCache, raise suitable sheet count to 4096 (591 bytes, application/octet-stream)
2011-06-29 07:07 UTC, Antti Koskimäki
Details
Test code demonstrating the problem (2.45 KB, application/octet-stream)
2011-07-01 09:55 UTC, Antti Koskimäki
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koskimäki 2011-06-29 07:07:23 UTC
Created attachment 27226 [details]
PlainCellCache, raise suitable sheet count to 4096

When updating calculations for Workbook having more than 256 sheets, the evaluation cache at some point throws exception:

 java.lang.IllegalStateException: value changed
	at org.apache.poi.ss.formula.EvaluationCache.getPlainValueEntry(EvaluationCache.java:144)
	at org.apache.poi.ss.formula.EvaluationTracker.acceptPlainValueDependency(EvaluationTracker.java:146)
	at org.apache.poi.ss.formula.WorkbookEvaluator.evaluateAny(WorkbookEvaluator.java:258)
	at org.apache.poi.ss.formula.WorkbookEvaluator.evaluateReference(WorkbookEvaluator.java:618)
	at org.apache.poi.ss.formula.SheetRefEvaluator.getEvalForCell(SheetRefEvaluator.java:47)
	at org.apache.poi.ss.formula.LazyRefEval.getInnerValueEval(LazyRefEval.java:44)


Initial reason is PlainCellCache packing sheet-index to 8 bits, i.e. supporting only 256 sheets. Excel does not limit sheet number ("is limited by available memory"), so 256 is far too low limit. Included patch raises the limit to 4096 (12bit), at the same time dropping the workbook count from 256 to 16 (4bit) which seemed to me more suitable compromise.

Documentation should also point out such "invisible limits" and code should Assert the situation when such limits are exceeded, instead of just getting messed up.
Comment 1 Yegor Kozlov 2011-06-30 16:12:06 UTC
A unit test demonstrating the problem, please!

Yegor
Comment 2 Antti Koskimäki 2011-07-01 09:55:10 UTC
Created attachment 27234 [details]
Test code demonstrating the problem

Couldn't find proper documentation on how to write suitable junit for this kind of bug, but simple test-code demonstrating the problem attached.
Comment 3 Yegor Kozlov 2011-07-01 13:26:37 UTC
It is a pretty low-level stuff. Thanks for digging that deep!

The patch looks good, but I'm inclined to change the type of the index to long. This way we don't need to compromise between the number of workbooks and worksheets.

The fix is coming soon - I'm going to write some tests first.

Yegor
Comment 4 Yegor Kozlov 2011-07-02 09:09:51 UTC
Fixed in r1142181, junit added.

I changed the type of the index from int to long. This way the number of workbook and sheets is unlimited (actually less 65536). 

Yegor