Bug 69552 - Function FLOOR and CEILING inconsistent with OpenFormula specification
Summary: Function FLOOR and CEILING inconsistent with OpenFormula specification
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
3.3.0 release
Hardware: Other All
: medium normal
Assignee: Winfried Donkers
QA Contact:
URL:
Whiteboard: target:5.0.0 target:5.1.0 target:5.0...
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-18 22:56 UTC by Mike Kaganski
Modified: 2016-10-25 19:24 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Test file (10.00 KB, application/vnd.oasis.opendocument.spreadsheet)
2013-09-18 22:56 UTC, Mike Kaganski
Details
test file used for CEILING and FLOOR variations (14.66 KB, application/vnd.oasis.opendocument.spreadsheet)
2014-12-24 11:47 UTC, Winfried Donkers
Details
Calc document used to test CEILING and FLOOR variations (16.44 KB, application/vnd.oasis.opendocument.spreadsheet)
2015-06-01 08:16 UTC, Winfried Donkers
Details
Excel document used to test CEILING and FLOOR variations (9.69 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2015-06-01 08:18 UTC, Winfried Donkers
Details
Same Excel document used to test CEILING and FLOOR variations, but saved with Excel2013 (13.96 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2015-06-12 16:59 UTC, Eike Rathke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2013-09-18 22:56:04 UTC
Created attachment 86114 [details]
Test file

OASIS Open Document Format for Office Applications (OpenDocument) Version 1.2 - Part 2: Recalculated Formula (OpenFormula) Format, part 6.17.3 FLOOR:

> Summary: Round a number N down to the nearest multiple of the second parameter,
> significance.
> Syntax: FLOOR( Number N [ ; [ Number significance ] [ ; Number mode ] ] )
> Returns: Number
> Constraints: Both N and significance shall be numeric and have the same sign.
>Semantics: Rounds a number down to a multiple of the second number. If significance
> is omitted or an empty parameter (two consecutive ;; semicolons) it is assumed to be
> -1 if N is negative and +1 if N is non-negative, making the function act like the normal
> mathematical floor function if mode is not given or zero. If mode is given and not equal
> to zero, the absolute value of N is rounded away from zero to a multiple of the absolute
> value of significance and then the sign applied. Otherwise, it rounds toward negative
> infinity, and the result is the largest multiple of significance that is less than or equal
> to N. If any of the two parameters N or significance is zero, the result is zero.
> Note: Many application user interfaces have a FLOOR function with only two parameters, and
> somewhat different semantics than given here (e.g., they operate as if there was a non-zero mode
> value). These FLOOR functions are inconsistent with the standard mathematical definition of FLOOR.

Given these guidelines, the LO FLOOR implementation incorrectly handles omitted second parameter (Significance): if FLOOR is used with only one parameter (like in "=FLOOR(5)"), the function returns Err:511 Variable missing, and if there are consequitive semicolons, like in "=FLOOR(5;)" or "=FLOOR(5;;)" or "=FLOOR(5;;1)", it gives zero, as if the second parameter were zero. Shis contradicts with the second sentence of "Semantics" clause of standard cited above.

Tested with 4.1.1.2 under Win7x64
Comment 1 Winfried Donkers 2013-11-23 10:16:54 UTC
I confirm behaviour.
CEILING has the same.
Working on a patch.
Comment 2 Winfried Donkers 2014-12-24 11:47:33 UTC
Created attachment 111281 [details]
test file used for CEILING and FLOOR variations

This is the test file used by me. 

Current status:
-all CEILING functions work in Calc, import/export is OK except for export of CEILING (ODF1.2) to Excel.
-All FLOOR function are to be evaluated/corrected yet. Intention is to do this once CEILING functions have been pushed to master.
Comment 3 Commit Notification 2015-04-29 23:40:18 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=010b2d2d9be846fb6b10848204e29e1bc00ef1ea

fdo#69552 [part 1] make calc functions CEILING comply with ODF1.2

It will be available in 5.0.0.

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 4 Commit Notification 2015-04-29 23:56:30 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0cd88d27ecd87c8c5791418bcac6ab1684ece74a

simplify logic and less comparisons, fdo#69552 follow-up

It will be available in 5.0.0.

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 5 Winfried Donkers 2015-04-30 06:11:07 UTC
Fix for FLOOR functions will be coming soon, too.
Comment 6 Winfried Donkers 2015-06-01 08:16:37 UTC
Created attachment 116213 [details]
Calc document used to test CEILING and FLOOR variations

This test document reflects the latest changes. The Excel functions have been verified with Excel 2013.
Comment 7 Winfried Donkers 2015-06-01 08:18:24 UTC
Created attachment 116214 [details]
Excel document used to test CEILING and FLOOR variations

This document has been created by Calc and tested with Excel2013.
Comment 8 Commit Notification 2015-06-11 11:19:14 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=cb13c46da1cdd1e2b95fd473cc0ff54f776dfe9c

tdf#69552 [part 2] support all ODFF1.2 and Excel2013 variations

It will be available in 5.1.0.

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 9 Commit Notification 2015-06-11 11:20:53 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=67360b28c0965992d67afb9356c5ce8ca3bea11d

keep old CEILING opcode, tdf#69552 follow-up

It will be available in 5.1.0.

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 10 Commit Notification 2015-06-11 13:15:51 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=586d8308ca61c79c8fa669592f5d9abe50f841a2&h=libreoffice-5-0

tdf#69552 [part 2] support all ODFF1.2 and Excel2013 variations

It will be available in 5.0.0.0.beta4.

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 11 Commit Notification 2015-06-11 13:15:55 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e49836c5a3904b77e5474c176836a4cd018c6a0a&h=libreoffice-5-0

keep old CEILING opcode, tdf#69552 follow-up

It will be available in 5.0.0.0.beta4.

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 12 Eike Rathke 2015-06-12 11:49:16 UTC
Actually the current implementation does not calculate all cases of attachment 116214 [details] the same as Excel. This needs some further work, I'm at it.
Comment 13 Commit Notification 2015-06-12 12:29:55 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f3419973d4e0f3b5cd9fc729a3c026a789febca5&h=libreoffice-5-0

FLOOR.MATH has 3 parameters, show them in UI, tdf#69552 follow-up

It will be available in 5.0.0.0.beta4.

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 14 Commit Notification 2015-06-12 12:58:52 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=688995bbb296e875511403fcce9276183f2545b0

copypasta error, tdf#69552 follow-up

It will be available in 5.1.0.

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 Eike Rathke 2015-06-12 13:00:04 UTC
my bad..
Comment 16 Commit Notification 2015-06-12 13:01:40 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=b5b619e67e57af60d9ac0edbba7b5ca21c08dcd7&h=libreoffice-5-0

copypasta error, tdf#69552 follow-up

It will be available in 5.0.0.0.beta4.

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 17 Eike Rathke 2015-06-12 16:59:18 UTC
Created attachment 116488 [details]
Same Excel document used to test CEILING and FLOOR variations, but saved with Excel2013

Just to ensure we have original Excel document test cases.
Comment 18 Commit Notification 2015-06-18 20:07:06 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ba70050dc37f82306a8a3c5815372a4b9fff18fc

add test case document to unit tests, tdf#69552

It will be available in 5.1.0.

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 19 Eike Rathke 2015-06-18 20:16:52 UTC
@Winfried:
In sc/qa/unit/subsequent_export-test.cxx I commented out the .xls export/reload because

void ScExportTest::testCeilingFloorXLS()
{
    // CEILING.PRECISE() and FLOOR.PRECISE() with one parameter only currently
    // (2015-06-18) don't survive .xls save/reload and give NA()
    //testCeilingFloor(XLS);
}

Probably either the function definitions or the code generation need some tweaking for the missing parameter case, if you want to take a look at it?
Comment 20 Winfried Donkers 2015-06-24 07:51:10 UTC
(In reply to Eike Rathke from comment #19)

@Eike:
I will take a look at it.

And thank you for the changes you made; Excel has some weird characteristics with FLOOR/CEILING and their variations which are depended on the version of Excel too.
Comment 21 Commit Notification 2015-07-09 11:31:31 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=570852376464f238d7c3c0b58b37250ac5944a1c

fix export to xls, tdf#69552

It will be available in 5.1.0.

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 22 Commit Notification 2015-07-09 12:10:18 UTC
Winfried Donkers committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0ca691f1999bda3638db67bdcc11569e1145563c&h=libreoffice-5-0

fix export to xls, tdf#69552

It will be available in 5.0.0.3.

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.