Bug 87119 - FILEOPEN Loading spreadsheet loses data
Summary: FILEOPEN Loading spreadsheet loses data
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.4.0.0.alpha1
Hardware: x86-64 (AMD64) Linux (All)
: highest critical
Assignee: Tor Lillqvist
URL:
Whiteboard: target:4.4.0.0.beta3
Keywords: bibisected, regression
Depends on:
Blocks: mab4.4
  Show dependency treegraph
 
Reported: 2014-12-08 20:27 UTC by rlk
Modified: 2015-12-17 08:41 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Expected content of the 'T Rank' sheet (1.37 MB, image/png)
2014-12-09 14:42 UTC, Matthew Francis
Details
Broken "T Rank" sheet (370.12 KB, image/png)
2014-12-09 14:58 UTC, Matthew Francis
Details

Note You need to log in before you can comment on or make changes to this bug.
Description rlk 2014-12-08 20:27:06 UTC
Reference https://drive.google.com/file/d/0B-4rl96vSQGGenhwd0wzdFpOMFk/view?usp=sharing

This spreadsheet loads correctly into 4.1.5, 4.1.6, 4.2.7, and 4.3.4.  In 4.4beta (and reported by raal, on 4.5 alpha and 4.3.6) much of the spreadsheet does not load correctly (e. g. look at the T Rank sheet).  Raal reports that it does load correctly on 4.4alpha.

This was determined in conjunction with filing bug 87101 (which is not related).

This spreadsheet is quite complex, using names as unparameterized macros.
Comment 1 raal 2014-12-08 21:11:24 UTC
I can confirm:
with Version: 4.4.0.0.alpha0+
Build ID: dea4a3b9d7182700abeb4dc756a24a9e8dea8474
I can see data on sheet Trank

No data with Version: 4.4.0.0.alpha1+
Build ID: b800d0b6ad74ce4a9adb23b865dd174d1eefa47b; bibisectable with 44alpha2only

Dataloss.

On the sheet are data loaded through names, for example =rr_d
Name :  rr_d
Range: IF(B12="";bk;T(OFFSET($'All results'.$E$1;$P12;0))&T(altern))
Comment 2 rlk 2014-12-08 21:15:41 UTC
Yes, that's what I've done for most of the spreadsheet, used names rather than actual formulas in cells.  That greatly reduces the size of the spreadsheet, and also makes it easy (if slow) to change the definition of a column (or other repetitive expression).
Comment 3 Matthew Francis 2014-12-09 03:18:57 UTC
Bibisect results from 44alpha2only:

0777cd085a7633a48e03d25948cc67fce87b7ac7 is the first bad commit
commit 0777cd085a7633a48e03d25948cc67fce87b7ac7
Author: Bjoern Michaelsen <bjoern.michaelsen@canonical.com>
Date:   Sat Nov 8 08:52:37 2014 +0000

    source-hash-b800d0b6ad74ce4a9adb23b865dd174d1eefa47b
    
    commit b800d0b6ad74ce4a9adb23b865dd174d1eefa47b
    Author:     Noel Grandin <noel@peralex.com>
    AuthorDate: Thu Nov 6 10:13:15 2014 +0200
    Commit:     Noel Grandin <noel@peralex.com>
    CommitDate: Thu Nov 6 13:52:55 2014 +0200
    
        svl: remove unused code
    
        Change-Id: I5a01162d2925eede97f1cdc24aa876179b8b43b4

# bad: [0777cd085a7633a48e03d25948cc67fce87b7ac7] source-hash-b800d0b6ad74ce4a9adb23b865dd174d1eefa47b
# good: [812c4a492375ac47b3557fbb32f5637fc89d60d9] source-hash-dea4a3b9d7182700abeb4dc756a24a9e8dea8474
git bisect start 'latest' 'oldest'
# good: [8677ba6e74a774fb44ec7831f14e53d8663f59ed] source-hash-eb213e490d9a366477b921d1a408d85c4638499e
git bisect good 8677ba6e74a774fb44ec7831f14e53d8663f59ed
# good: [33e2222277083d3a30c0020aa4b05d54aeed6131] source-hash-34fe0d32505f64fdef20828c26547cc7f8453212
git bisect good 33e2222277083d3a30c0020aa4b05d54aeed6131
# good: [c9da176038b97f22406b17ed2f013c596a4fc266] source-hash-91b65e0a73b35a745921831b73f5f4589bb889b7
git bisect good c9da176038b97f22406b17ed2f013c596a4fc266
# good: [4561a0c9b06d62052f401feeba76f01831e17ca3] source-hash-6a4b976bd0818c2f60b879594d393baad9a0f346
git bisect good 4561a0c9b06d62052f401feeba76f01831e17ca3
# good: [cf48d4cf0e32cbdfe968c06bd439e2d6982c9642] source-hash-2ec4c8b07427af868e32e14aaefd20649c1135d6
git bisect good cf48d4cf0e32cbdfe968c06bd439e2d6982c9642
# good: [f856b595e1be50d9af817fab560cbb25f5305b7e] source-hash-f18fd73e287b781e953a3c4bfa05b55f39880a35
git bisect good f856b595e1be50d9af817fab560cbb25f5305b7e
# good: [cc9a130a364bd74934c8a335006a450dabb10101] source-hash-12bcfec04fcbe6425e327109ad47cd2b2b80d2bd
git bisect good cc9a130a364bd74934c8a335006a450dabb10101
# first bad commit: [0777cd085a7633a48e03d25948cc67fce87b7ac7] source-hash-b800d0b6ad74ce4a9adb23b865dd174d1eefa47b
Comment 4 Joel Madero 2014-12-09 04:52:25 UTC
I believe that this is perfectly acceptable as a MAB. Marking appropriately.
Comment 5 Matthew Francis 2014-12-09 05:34:54 UTC
Follow-up plain bisect points to this commit:

ef809ce9480182ea5c4f77843f72d1d45bd48c35 is the first bad commit
commit ef809ce9480182ea5c4f77843f72d1d45bd48c35
Author: Tor Lillqvist <tml@collabora.com>
Date:   Wed Nov 5 00:34:35 2014 +0200

    More work on the new OpenCL options
    
    Now the new options show up in the "Detailed Calculation Settings" dialog and
    are saved and restored from the per-user configuration.
    
    The code that manipulates the "Detailed Calculation Settings" dialog is quite
    ugly with all its manual hiding and showing of widgets depending on which
    detail it is that is being edited. This also means that the dialog cannot be
    designed using Glade. But no time now to re-work this.
    
    Change-Id: I03a3a51d902084e73aab5a787b588d22ea7578f2



And indeed when I disable OpenCL the attached spreadsheet works again
(Options - Calc - Formula - Detailed Calculation Settings - Custom - Enable OpenCL for some formula computation: False)

Adding Cc: tml@iki.fi
Comment 6 Tor Lillqvist 2014-12-09 11:54:45 UTC
This commit (on master) might fix this problem, but I am not able to test because of some unrelated bug:

commit 6eb155dea00a1e312fdc3339406bf168a93a25e7
Author: Tor Lillqvist <tml@collabora.com>
Date:   Tue Dec 9 12:45:41 2014 +0200

    Don't use of the broken "Software" group interpreter
    
    FormulaGroupInterpreterSoftware is known to be broken, says moggi. We should
    not use it as a fallback to OpenCL.
    
    Not sure whether it makes sense, but let's keep it in the code for now.  Make
    using it conditional on setting the environment variable
    SC_ALLOW_BROKEN_SOFTWARE_INTERPRETER (to any value). Only a developer who
    wants to work on it should set that.
    
    sc::FormulaGroupInterpreter::getStatic() can now return NULL, adapt callers
    accordingly.
    
    This might fix fdo#87119, but I am not able to check, as the document from
    that bug causes an unrelated assertion for me.
    
    Change-Id: I24454f46332014cbcbf696252059b9743f3c84d6

 sc/source/core/data/formulacell.cxx  |  4 +++-
 sc/source/core/tool/formulagroup.cxx |  7 +++++--
 sc/source/core/tool/interpr5.cxx     | 12 ++++++++----
 3 files changed, 16 insertions(+), 7 deletions(-)
Comment 7 Tor Lillqvist 2014-12-09 14:36:04 UTC
> e. g. look at the T Rank sheet

How is it supposed to look? Could you attach a screenshot please. I don't have any 4.[123] build handy to compare with.
Comment 8 Matthew Francis 2014-12-09 14:42:46 UTC
Created attachment 110631 [details]
Expected content of the 'T Rank' sheet

I have one of those from earlier - here you go

If it contains more than nothing at all it's probably working
Comment 9 Matthew Francis 2014-12-09 14:58:15 UTC
Created attachment 110633 [details]
Broken "T Rank" sheet

(in contrast, this is what it looks like when broken - empty)
Comment 10 Tor Lillqvist 2014-12-09 16:01:32 UTC
After taking care of the assertions I was able to verify that indeed the fix helps. Will cherry-pick to libreoffice-4-4.
Comment 11 Tor Lillqvist 2014-12-09 16:16:35 UTC
(The unrelated assertion, which prevents loading the test document in an assertions-enabled build, is fixed by the patch at https://gerrit.libreoffice.org/#/c/13396/ I did not commit that directly, I want some Calc expert to approve it.)
Comment 12 Tor Lillqvist 2014-12-09 19:37:08 UTC
The same patch submitted to gerrit for the libreoffice-4-4 branch, too: https://gerrit.libreoffice.org/#/c/13397/
Comment 13 rlk 2014-12-10 16:47:38 UTC
Note that per Raal's comment in https://bugs.freedesktop.org/show_bug.cgi?id=87101 this shows up in 4.3.6 also.
Comment 14 Tor Lillqvist 2014-12-10 17:36:32 UTC
It can't be the same bug in 4.3.6 as the use of OpenCL was not turned on by default in 4.3, and thus there shouldn't be a way to get to the related "Software" Formula Group Interpreter code-path. Unless one turns on OpenCL use intentionally, which I think is not recommended in 4.3.
Comment 15 Commit Notification 2014-12-16 00:02:20 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

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

fdo#87119: Don't use of the broken "Software" group interpreter

It will be available in 4.4.0.0.beta3.

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 16 Eike Rathke 2014-12-18 13:53:21 UTC
Let's call this fixed now.
Comment 17 rlk 2014-12-19 15:54:45 UTC
The current nightly (12/19 linux x86_64) fails to start, so I can't test it.
Comment 18 rlk 2014-12-20 19:14:18 UTC
It works.  The failure to start problem appears to have been due to my .config/libreofficedev directory.  Removing it and allowing libreoffice to recreated it fixed that problem.
Comment 19 Robinson Tryon (qubit) 2015-12-17 08:41:26 UTC
Migrating Whiteboard tags to Keywords: (bibisected)
[NinjaEdit]