Bug Hunting Session
Bug 38592 - Extra parameters added to ceiling function when saving and opening xlsx
Summary: Extra parameters added to ceiling function when saving and opening xlsx
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.1.1.2 release
Hardware: All All
: low minor
Assignee: Eike Rathke
URL:
Whiteboard: target:4.1.4
Keywords:
: 39615 69072 (view as bug list)
Depends on:
Blocks: mab4.0
  Show dependency treegraph
 
Reported: 2011-06-23 01:12 UTC by Anton
Modified: 2013-11-23 11:34 UTC (History)
9 users (show)

See Also:
Crash report or crash signature:


Attachments
Minimal test case for reproducing this bug consistently (4.17 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2013-11-21 04:08 UTC, Peter Ansell
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Anton 2011-06-23 01:12:17 UTC
If a worksheet containing a CEILING function is saved in .xlsx format,
the function gets saved with 4 arguments instead of the 3 originals,
resulting in Err.504 when the file is opened again.

If the format is .ods, the arguments remain being 3, and the behaviour is the expected when reopening
Comment 1 GerardF 2011-06-23 08:06:41 UTC
Reproduce.

When saving in .xlsx, Calc added an argument instead of removing the last argument.
In Excel, CEILING have only 2 arguments.
Comment 2 Andras Timar 2011-06-26 03:25:49 UTC
When saved to xlsx all 3 parameters of CEILING are exported.
Resulting xlsx file cannot be opened with MS Office. (I tried MS Office 2003 with the compatibility pack.)
On import Calc adds a 4th parameter -> Error:504.
FLOOR has the same bug.
Comment 3 Markus Mohrhard 2011-06-26 17:14:43 UTC
Let me have a look at it.
Comment 4 Markus Mohrhard 2011-06-27 20:01:22 UTC
Need some rework in the formula API. I don't think it is something for the 3.4 release cycle.

I have to talk to Kohei if we really want change the API or if we want to change the export code for xlsx.

Code Pointers:
FormulaCompiler::CreateStringFromTokenArray
XclExpFormulaCell::SaveXml
Comment 5 Markus Mohrhard 2011-07-28 11:02:58 UTC
*** Bug 39615 has been marked as a duplicate of this bug. ***
Comment 6 Jeffrey 2011-07-28 19:13:36 UTC
Saw the duplicate post of this bug (39615). I created an .ods file and used the Floor function with 2 arguments and saved as .xlsx format with no problem. On MS Excel it only demands 2 arguments for the floor function, so perhaps error only occurs when using the three argument format in calc?

On LibreOffice 3.4  340m1(Build:103) for OpenSuse Linux.
Comment 7 Dirk Thomas 2011-07-28 23:59:37 UTC
As far as i have noticed the "error" also occurs when using the two argument format in calc. Then during save a third parameter is added. After saving a reloading and saving a second time the result will be four arguments.
Comment 8 Regina Henschel 2011-11-19 15:47:47 UTC
The problem is that the functions in Excel and in ODF are not full compatible. Some thoughts on that in http://wiki.services.openoffice.org/wiki/Calc/Drafts/Treatment_of_new_Excel_2010_functions#CEILING.PRECISE
Comment 9 Peter Ansell 2013-11-21 04:08:41 UTC
Created attachment 89551 [details]
Minimal test case for reproducing this bug consistently

I noticed this behaviour in 3.5 and upgraded to 4.1 to see if it would go away but it is still present.

In my case the function was:

CEILING(A1/B1,1)

which was replaced on the first save and open cycle with:

CEILING(A1/B1,1,1)

The next time I opened the file and edited the formula to add a space at the end and saved and then opened the file again another 1 was added:

CEILING(A1/B1,1,1,1)

Ditto for the next cycle:

CEILING(A1/B1,1,1,1,1)

etc.

CEILING(A1/B1,1,1,1,1,1)

It seems to add an endless number of extra parameters as long as I make a null edit each time between saving.

I have only ever saved the file as XLSX.
Comment 10 Peter Ansell 2013-11-21 05:19:37 UTC
http://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/formulaparser.cxx#384 :

void FormulaFinalizer::appendCalcOnlyParameter( const FunctionInfo& rFuncInfo, size_t nParam )
{
    (void)nParam;   // prevent 'unused' warning
    switch( rFuncInfo.mnBiff12FuncId )
    {
        case BIFF_FUNC_FLOOR:
        case BIFF_FUNC_CEILING:
            OSL_ENSURE( nParam == 2, "FormulaFinalizer::appendCalcOnlyParameter - unexpected parameter index" );
            maTokens.append< double >( OPCODE_PUSH, 1.0 );
            maTokens.append( OPCODE_SEP );
        break;
    }
}

I am not sure what OSL_ENSURE does, but if it doesn't fail at that line, it looks like this function will add extra parameters to both CEILING and FLOOR without strictly relying on nParam. Naively, it looks like there should be an if(nParam == 2) {} wrapper instead of just OSL_ENSURE.

The bug tracking this issue for the FLOOR function is Bug 69072
Comment 11 Peter Ansell 2013-11-21 05:37:16 UTC
Patch submitted to Gerrit for review:

https://gerrit.libreoffice.org/6746
Comment 12 Eike Rathke 2013-11-22 22:34:37 UTC
*** Bug 69072 has been marked as a duplicate of this bug. ***
Comment 13 Eike Rathke 2013-11-22 22:36:59 UTC
A different change pending review for 4-1 https://gerrit.libreoffice.org/6764
Comment 14 Commit Notification 2013-11-23 00:35:44 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-4-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1f38670b11f7d49505603852f6e259cfc2b7710e&h=libreoffice-4-1

resolved fdo#38592 do not insert extraneous parameter in import


It will be available in LibreOffice 4.1.4.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 15 tommy27 2013-11-23 03:02:34 UTC
@Eike
can we label this bug as FIXED after your patch?
Comment 16 Eike Rathke 2013-11-23 11:34:10 UTC
Yes, done.