Bug 44675 - [patch] POI does not distinguish functions with fixed vs variable arguments
Summary: [patch] POI does not distinguish functions with fixed vs variable arguments
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.0-dev
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
Depends on:
Reported: 2008-03-26 00:01 UTC by Josh Micich
Modified: 2008-05-21 20:42 UTC (History)
0 users


Note You need to log in before you can comment on or make changes to this bug.
Description Josh Micich 2008-03-26 00:01:16 UTC
The following test code fails because POI encodes all functions as FuncVarPtg:

        Workbook book = Workbook.createWorkbook();
        Ptg[] ptgs = FormulaParser.parse("countif(A1:A2, 1)", book);
        assertEquals(FuncPtg.class, ptgs[2].getClass());

Excel tolerates the incorrect token FuncVarPtg for many functions like SIN() etc, but COUNTIF() is one example where this error is visible in Excel (#VALUE!).
Comment 1 Josh Micich 2008-03-26 00:54:42 UTC
Applied patch in svn r641185.

The main change of this patch is to take the function metadata which was originally in AbstractFunctionPtg and move it to FunctionMetadataRegistry.  The original metadata had only one arg-count field.  Both min and max are required to distinguish var-arg functions (and will also be handy for parser validations later).  The new metadata used by POI is generated directly from the OOO excelfileformat.odt.

A new sample spreadsheet (missingFuncs44675.xls) was added.  Prior to the patch, POI would fail to display the six formulas correctly.  Some extra bugs were identified and fixed while trying to get this spreadsheet to work:
 - tAttrVolatile needs to be handled in FormulaParser.toFormulaString()
 - NOW() function needs to take zero parameters

There were some (~6) errors in the metadata from excelfileformat.odt.  One extra text file: 'functionMetadata-asGenerated.txt' has been included for comparison with the actual file used by POI: 'functionMetadata.txt'.  The diffs of these two files should be used to fix excelfileformat.odt.

Comment 2 Dmitriy Kumshayev 2008-05-02 16:06:14 UTC
setCellFormula("SUM(A1:A2)") produces #VALUE! error in Excel

This started happening since this patch.

HSSFWorkbook wb = new HSSFWorkbook();
HSSFSheet sh = wb.createSheet("TestSheet");
HSSFRow row1 = sh.createRow(0);	
HSSFCell A1 = row1.createCell((short)0); 

HSSFRow row2 = sh.createRow(1);	
HSSFCell A2 = row2.createCell((short)0); 

HSSFRow row3 = sh.createRow(2);	
HSSFCell A3 = row3.createCell((short)0); 


FuncVarPtg is used to represent SUM function.
FuncVarPtg(String pName, byte pNumOperands) constructor initializes paramClass with Ptg.CLASS_VALUE while it used to be Ptg.CLASS_REF before this patch.

I am not quite sure about the meaning of this, but when I manually changed it in debugger to Ptg.CLASS_REF, the problem disappeared.
Comment 3 Josh Micich 2008-05-02 21:13:25 UTC
Fixed in svn r652994.

Prior to r641185 SUM would get parameter operand class Ptg.CLASS_REF.  When I refactored the function metadata (to recognise var arg functions) I left the parameter operand classes anattached, because there were no junits to confirm this requirement. I added such a junit with this most recent fix.

The 'operand class' stuff is not well tested by POI.  Furthermore Excel often  tolerates incorrect token operand classes, so when POI gets it wrong the errors are not always immediately apparent.

Another enhancement I added here was to validate function arg counts when parsing formulas.  None of the existing junits were disturbed by this (they all supply correct function arg counts in every case).
Comment 4 Josh Micich 2008-05-21 20:42:59 UTC
A new version of "The Microsoft Excel File Format" has been released

Thanks to Daniel Rentz for making several tweaks to the function metadata in this document to help align with POI.  Now the files functionMetadata-asGenerated.txt and functionMetadata.txt are exactly the same (no need to apply manual edits).

This change has been applied in svn r658986.