Bug 44364 - [patch] Added Excel functions MATCH, NA, SUMPRODUCT and other fixes
Summary: [patch] Added Excel functions MATCH, NA, SUMPRODUCT and other fixes
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.0-dev
Hardware: Other other
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
Depends on:
Reported: 2008-02-05 21:01 UTC by Josh Micich
Modified: 2008-02-14 07:58 UTC (History)
0 users

svn diff (58.77 KB, patch)
2008-02-05 21:03 UTC, Josh Micich
Details | Diff
tar bzip2 of newly added files (5 junit classes) (7.06 KB, patch)
2008-02-05 21:04 UTC, Josh Micich
Details | Diff
new version of TestDataValidation.xls (131.50 KB, patch)
2008-02-05 21:14 UTC, Josh Micich
Details | Diff
junit report on failing TestFormulasFromSpreadsheet (14.09 KB, text/plain)
2008-02-07 06:37 UTC, Yegor Kozlov

Note You need to log in before you can comment on or make changes to this bug.
Description Josh Micich 2008-02-05 21:01:55 UTC
Added Implementations for Excel functions MATCH(), NA(), SUMPRODUCT(). Added 

Rewrote 3 classes(GenericFormulaTestCase, TestEverything, TestEverything) into 
TestFormulasFromSpreadsheet.  This served a few purposes:
 - to allow for more conventional structuring of the test suites
 - to formalise and simplify the interface between the test spreadsheet and 
the test code.
 - to identify and fix a few bugs (related to comparing expected vs actual 
evaluation results)
Those 3 classes should be deleted.  The most correct appropriate predecessor 
of TestFormulasFromSpreadsheet is GenericFormulaTestCase.

Added cases for COUNTA, COUNTIF, MATCH, NA, and SUMPRODUCT to the spreadsheet

As a result of fixing the spreadsheet and related test, a few other bugs 
became visible. 

Fixed Excel error code propagation throughout HSSFFormulaEvaluator.   
Fixed error code handling in ROUNDDOWN(), ROUNDUP() and T(). Added junits.
Note - TestFormulasFromSpreadsheet has a disabled assert which confirms 
expected error codes.  This can be enabled after another 45 functions like 
ROUNDDOWN and ROUNDUP are fixed wrt. error handling.

Fixed bug (involving column area refs) in UnaryPlusEval.  Added junit.

Fixed FormulaParser.toFormulaString() for the case of a single no-arg function 
e.g. "=NOW()". Added junit.
Comment 1 Josh Micich 2008-02-05 21:03:54 UTC
Created attachment 21477 [details]
svn diff
Comment 2 Josh Micich 2008-02-05 21:04:40 UTC
Created attachment 21478 [details]
tar bzip2 of newly added files (5 junit classes)
Comment 3 Josh Micich 2008-02-05 21:14:36 UTC
Created attachment 21479 [details]
new version of TestDataValidation.xls 

svn diff skips binary files
This spreadsheet belongs here:
Comment 4 Nick Burch 2008-02-06 06:10:16 UTC
Thanks for this, it's quite the mammoth patch!

I've done a visual review of the patch, and it does seem fine from that. Given
the size, I'll wait a few days for other people to cast an eye over it, and if
no-one shouts I'll then apply it
Comment 5 Yegor Kozlov 2008-02-07 06:36:00 UTC
I exercised your code and TestFormulasFromSpreadsheet fails:


Test spreadsheet cell empty on row (79). Expected function name or
junit.framework.AssertionFailedError: Test spreadsheet cell empty on row (79).
Expected function name or '<END-OF-FUNCTIONS>'
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)

See the attached junit report.

All other tests pass.

Comment 6 Yegor Kozlov 2008-02-07 06:37:11 UTC
Created attachment 21492 [details]
junit report on failing TestFormulasFromSpreadsheet
Comment 7 Josh Micich 2008-02-07 11:38:46 UTC
You need to copy the new version of TestDataValidation.xls to 

I wasn't sure about how to include modified binary files in patch submissions.

The last revision of this file seems to be 615859 (Jan 28), and my work was 
after that so there shouldn't be any need to merge.
Comment 8 Nick Burch 2008-02-14 07:07:40 UTC
Request for next time...

If you are uploading a new version of TestDataValidation.xls again, any chance
you could gzip / bzip it first? It's pretty beasting, and takes an age to
download over GPRS, which is an issue when I decide to work on POI from a cafe!

(Just doing a final review before applying now)
Comment 9 Nick Burch 2008-02-14 07:58:07 UTC
Thanks for this, applied to svn

Had to copy the new TestDataValidation.xls to FormulaEvalTestData.xls to get it
to work. I think we need to review the fact that we have these two, and also try
to figure out why TestDataValidation.xls sometimes gets changed