Bug Hunting Session
Bug 91453 - Use configuration of text to number conversion also in arithmetic matrix operations (was: SUMPRODUCT() doesn't work well with 4.4.3)
Summary: Use configuration of text to number conversion also in arithmetic matrix oper...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.3.7.1 rc
Hardware: x86 (IA32) All
: medium major
Assignee: Eike Rathke
URL:
Whiteboard: target:4.4.5 target:5.1.0 target:5.0.4
Keywords:
: 90941 93673 95560 105227 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-05-22 08:32 UTC by LEON REGENT
Modified: 2017-01-09 22:33 UTC (History)
13 users (show)

See Also:
Crash report or crash signature:


Attachments
Demonstrates the sumproduct bug since 4.3.7.1 (17.69 KB, application/vnd.oasis.opendocument.spreadsheet)
2015-05-29 22:44 UTC, Erwan Souben
Details
More explanations on the bug (19.14 KB, application/vnd.oasis.opendocument.spreadsheet)
2015-06-25 12:30 UTC, LEON REGENT
Details

Note You need to log in before you can comment on or make changes to this bug.
Description LEON REGENT 2015-05-22 08:32:04 UTC
=SOMMEPROD(500*C2:C3) doesn't work anymore if a cell, for instance C2, is empty or alphabetic
It works if C2 = 0
=SOMMEPROD(C2:C3) works with C2 empty

With 4.4.2, the problem doen't exist
Comment 1 pierre-yves samyn 2015-05-22 09:27:01 UTC
Hi

I reproduce on windows 7/64 & Version: 4.4.3.2
Build ID: 88805f81e9fe61362df02b9941de8e38a9b5fd16
Locale: fr_FR

With C2 empty =SUMPRODUCT(500*C2:C3) gives #VALUE

I set status to NEW

Not reproduce on Version: 5.0.0.0.beta1
Build ID: 0a16c3dda4150008d9be6f24cbd15ac198d116d3
Locale : fr-FR (fr_FR)

Regards
Pierre-Yves
Comment 2 Erwan Souben 2015-05-29 22:44:14 UTC
Created attachment 116160 [details]
Demonstrates the sumproduct bug since 4.3.7.1
Comment 3 Erwan Souben 2015-05-29 22:54:37 UTC
Hello,

    In my case (Win7/x64), the bug appears since 4.3.7.1.
    The Calc example attached works fine until 4.3.6.2.
    Still doesn't work in 5.0.0.0.beta1 (Build ID: 0a16c3dda4150008d9be6f24cbd15ac198d116d3 / Locale : fr-FR)

    I didn't note down the build numbers for 4.3.7.1 and 4.3.6.2. I can do it if needed.

Regards,
Erwan Souben
Comment 4 raal 2015-05-30 07:22:04 UTC
Works correct in LO 3.5, linux -> regression
Comment 5 Eike Rathke 2015-06-01 18:22:03 UTC
This is due to the changes in matrix error propagation for bug 42481. Using a text string in arithmetic matrix calculations produces an error same as the strict text to number conversion mode in other formula operations.

However, matrix calculations should take the configuration of conversion of text to numbers into account and in this case follow the setting of whether empty strings should be treated as zero.
Comment 6 Luke 2015-06-01 23:19:19 UTC
I'm not sure if this is a bug. Excel, Google Sheet, WPS Sheet, and LO Calc>= 4.4 all produce the same results. Won't "fixing" this break interoperablity? If so, it should be an option that's off by default.
Comment 7 Eike Rathke 2015-06-03 10:15:40 UTC
@Luke:
The result then would depend on the text to number conversion configuration settings under Tools->Options->Calc->Formula "Detailed Calculation Settings", in this case the "Treat empty string as zero" option, which actually is off by default.
Comment 8 Eike Rathke 2015-06-03 10:19:36 UTC
And this is not a regression, it worked in previous versions only by accident.
Comment 9 Erwan Souben 2015-06-04 13:22:16 UTC
So it is not a bug, it is a feature, isn't it ? I don't agree with that, with all the respect to the community.
It looks weird to read that the original behaviour of a software could be considered as an accident.
Libreoffice (and OpenOffice before) have always worked like this. This is their natural behaviour. 
Changing this natural behaviour so deep and without warning (it is not documented) is an accident (or a 'bug' in computing language).
The reason why I have chosen LibO (and OOo before) is because it is different from its concurents. I don't want a clone, just a better software. So breaking a functionality (yes it is a regression) just to behave like other softwares is a very bad idea : why taking the risk to break professional documents made a long time ago ?
One of the reasons why I choosed one day to develop tools with OOo is that it was a *very* stable product. A tool I developped in 2006 was still in production until last month when I had this problem with SUMPRODUCT function. When it happened  I couldn't figure out where the problem was and I gave up the tool for a manual calculation : a lot of stress and time wasted.
So now I would hardly say that LibO is as stable as it used to be.

To be clear, i am not saying LibO is becoming bad. I am sorry if I sound unpleasant but this is important. LibO has nothing to be ashamed of before its concurents. I know it has to go further to get better. But breaking features like SUMPRODUCT without warning was a mistake with potential heavy consequences : for the users and their trust towards LibO.

Long live LibO !
Comment 10 Eike Rathke 2015-06-05 16:22:23 UTC
If you think the previous behaviour where arrays did not propagate these error values was a useful feature, then what result do you expect for =SUMPRODUCT({1,2}/{4,""})
Comment 11 Eike Rathke 2015-06-17 08:58:58 UTC
*** Bug 90941 has been marked as a duplicate of this bug. ***
Comment 12 LEON REGENT 2015-06-25 12:30:51 UTC
Created attachment 116826 [details]
More explanations on the bug

The file SUMPORDUCTbug.ods compares the results of three formulas with OO 4.4.3.2, OO4.4.2.2 and Excel2007.
There is really an annoying regression. Old files doen't work without a correction.
I understand that OO can't be better than Excel (compatibility). But now, OO is worse than Excel !
Comment 13 GerardF 2015-07-02 16:57:37 UTC
Attach file from the last comment show the bug like originally in the 1st comment and this bug is not related to text to number conversion:
The formula in C10 (=SUMPRODUCT(2*C$3:C$5)) should not returns #VALUE whatever "Detailled calculation settings" is.
Cell C4 do not contains an empty string, nor a formula that returns an empty sting.
The cell is really empty and should be treated as 0 in any case.
Note that if the scalar "2" is replaced with an array with the same lenght of C3:C5, the formula works correctly.
Comment 14 Eike Rathke 2015-07-06 12:10:36 UTC
C10 returns 28 in master and 5-0. I'll try to find a solution for 4-4-5.
Comment 15 Eike Rathke 2015-07-06 15:10:02 UTC
Note that this again is not directly related to SUMPRODUCT but to how matrix values (numeric, string, empty, error) are handled; SUMPRODUCT just forces a matrix context on its arguments. The same result (value or error) would be produced with a matrix formula {=2*C$3:C$5}

However, a fix for this particular problem is pending review for 4-4 at https://gerrit.libreoffice.org/16799
Comment 16 Commit Notification 2015-07-08 11:49:11 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

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

use ScMatrix::IsValueOrEmpty() on math operators Mul/Div/Pow, tdf#91453

It will be available in 4.4.6.

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 Commit Notification 2015-07-10 14:41:01 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-4-4-5":

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

use ScMatrix::IsValueOrEmpty() on math operators Mul/Div/Pow, tdf#91453

It will be available in 4.4.5.

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 18 LEON REGENT 2015-07-10 15:44:56 UTC
In version 5.0.0.2 (10 july 2015), the comportment is the same as Excel.
The bug seems to be corrected at best. Thank you !
In some (rare) cases, when the content is alphabetic, it will be necessary to change the syntax of the formula to obtain a correct result.
I give an example at http://lumiere.olympe.in/ComptaPerso.htm
Comment 19 Eike Rathke 2015-08-31 18:51:10 UTC
*** Bug 93673 has been marked as a duplicate of this bug. ***
Comment 20 Commit Notification 2015-10-05 16:49:37 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: tdf#91453 use configuration of text to number conversion

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 21 Commit Notification 2015-10-05 18:08:23 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: tdf#91453 use configuration of text to number conversion

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 Eike Rathke 2015-10-05 19:30:33 UTC
Pending review https://gerrit.libreoffice.org/19172 for 5-0
Comment 23 Commit Notification 2015-10-22 21:29:29 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=0c8d1c04a30ea5df783f758cf6744b2918643c0d&h=libreoffice-5-0

Resolves: tdf#91453 use configuration of text to number conversion

It will be available in 5.0.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 24 libreoffice.20.geckoespresso 2015-10-22 22:32:04 UTC
(In reply to Commit Notification from comment #21)
> Eike Rathke committed a patch related to this issue.
> It has been pushed to "master":
> 
> It will be available in 5.1.0.
> 
> Affected users are encouraged to test the fix and report feedback.

I have tested your patch with
master~2015-10-22_10.37.17_LibreOfficeDev_5.1.0.0.alpha1_Win_x64_en-US_de_ar_ja_ru_qtz.msi

My spreadsheets (using array calculations with empty strings) look fine now if “Treat empty string as zero” is active.

Thanks a lot for your assistance in this matter.
Comment 25 Eike Rathke 2015-11-04 23:32:12 UTC
*** Bug 95560 has been marked as a duplicate of this bug. ***
Comment 26 Mike Kaganski 2017-01-09 22:33:17 UTC
*** Bug 105227 has been marked as a duplicate of this bug. ***