Bug 53642 - [PATCH] XLS formula evaluation logging
Summary: [PATCH] XLS formula evaluation logging
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: POI Overall (show other bugs)
Version: 3.9-dev
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-02 12:58 UTC by Thies Wellpott
Modified: 2012-09-05 17:05 UTC (History)
0 users



Attachments
ant-generated patch file (no new files, only changes to existing source) (11.39 KB, patch)
2012-08-02 12:58 UTC, Thies Wellpott
Details | Diff
Patch for "ss" package level interface (6.15 KB, patch)
2012-09-05 13:00 UTC, Thies Wellpott
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thies Wellpott 2012-08-02 12:58:20 UTC
Created attachment 29152 [details]
ant-generated patch file (no new files, only changes to existing source)

For a detailed analysis of the formula evalution - especially in cases of exceptions or wrong results - a detailed logging of the recursive evaluation is very helpful.
I needed this for a very complex XLS to find the missing POI implementations (I will add them as separate patch).

Usage documentation with source extract is given in documentation/content/xdocs/spreadsheet/eval.xml at the end (I did not test the HTML generation for this).

Changes in detail:

- org/apache/poi/ss/formula/WorkbookEvaluator.java
  * The main extension go here, inside evaluateFormula().
  * see spreadsheet/eval.xml and source comments for details
  * Note: I know, the use of a public static attribute is a no-go for real code, but this is for specific development use only, simple to use in this way, and worst case situation is missing or surplus logging.

- SystemOutLogger.java
  * added a shortcut of the log level in the output (helps to find good logging configuration)

- POILogger.java
  * made log level constants final
  * added string representation for the log levels (used by SystemOutLogger)

- org/apache/poi/hssf/usermodel/HSSFSheet.java
  * Bugfix for logging because "cval" is not always an instance of Record (results in ClassCastExceptions when logging is on debug level).
  * added {..} around some not very simple one-line if-blocks

Tested with HSSFWorkbook only.
Comment 1 Yegor Kozlov 2012-08-11 17:26:19 UTC
>* Note: I know, the use of a public static attribute is a no-go for real code, 
>but this is for specific development use only, simple to use in this way, and 
>worst case situation is missing or surplus logging.

Why did you make dbgEvaluationOutputForNextEval static ? It is an instance-scope variable and it is much better to set it per-evaluator.

That is, user code woulde change from

WorkbookEvaluator.dbgEvaluationOutputForNextEval = true;
evaluator.evaluateFormulaCell(cell);

to

evaluator.dbgEvaluationOutputForNextEval = true;
evaluator.evaluateFormulaCell(cell);

or even better:

evaluator.setDebugEvaluationOutputForNextEval(true);
evaluator.evaluateFormulaCell(cell);

I agree that a "public static" modifier is a "no go" for production but since POI is a public program API we should minimize possibilities of its incorrect usage. 

Regards,
Yegor
Comment 2 Evgeniy Berlog 2012-09-04 21:05:01 UTC
Thanks for your patch.

Applied in revision 53642.
I've added changes that Yegor proposed.

Regards, Evgeniy.
Comment 3 Thies Wellpott 2012-09-05 13:00:41 UTC
Created attachment 29332 [details]
Patch for "ss" package level interface

Thanks for your changes Evgeniy. I did not see the connection from the user API (FormulaEvaluator) to the implementation (WorkbookEvaluator) - this private instance variable is better!

This patch adds generic user model ("ss" package) support, i.e. in the FormulaEvaluator interface and the XSSF implementation. The usage docs are adjusted accordingly.
Comment 4 Evgeniy Berlog 2012-09-05 17:05:35 UTC
Thanks, for updated patch

Applied in revision 1381249

Regards, Evgeniy