Bug 128914 - Drag-copy of formulas referencing labels gives wrong result in rows (copying vertically); works in columns (copying horizontally)
Summary: Drag-copy of formulas referencing labels gives wrong result in rows (copying ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.3.0.4 release
Hardware: All All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:7.3.0 target:7.2.5
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2019-11-20 11:55 UTC by Mike Kaganski
Modified: 2022-02-24 10:58 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Sample with labels (8.91 KB, application/vnd.oasis.opendocument.spreadsheet)
2019-11-20 11:55 UTC, Mike Kaganski
Details
bibisect-42max, tail of terminal output (3.79 KB, text/plain)
2019-11-21 15:44 UTC, Terrence Enger
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2019-11-20 11:55:15 UTC
Created attachment 155970 [details]
Sample with labels

Given this in A1:C6:

> jan   feb   mar
>  1     2     3
>  2     3     4
>  3     4     5
>  4     5     6
>  5     6     7

defining labels in A1:C1 for columns in A2:C6 (Sheets->Named Ranges and Expressions->Labels...)

and putting this formula into D2: `='jan'*'feb'*'mar'`

the result is correct 6. But drag-copying the formula down into D3:D6 produces the column of 6s, while values must be different for each row (6, 24, 60, 120, 210). Hard recalc doesn't help, but minimal editing of the formula makes it work OK; also if you try to drag the wrong value 6 from, e.g., D3 into D4, the value in D4 would be 60; dragging the 6 from D3 into D4:D5 gives two 24s.

But the same works fine when the labels are for rows, not columns. Consider this in A12:F14:

> apr   1   2   3   4   5   6
> may   2   3   4   5   6   7
> jun   3   4   5   6   7   8

set labels in A12:A14 for rows B12:F14; and put `='apr'*'may'*'jun'` into B15. The cell then drag-copies fine into C15:F15.

See sample file with pre-configured data, where only drag-copy is left.

Version: 6.3.3.2 (x64)
Build ID: a64200df03143b798afd1ec74a12ab50359878ed
CPU threads: 12; OS: Windows 10.0; UI render: GL; VCL: win; 
Locale: ru-RU (ru_RU); UI-Language: en-US
Calc: threaded
Comment 1 Oliver Brinzing 2019-11-20 19:24:17 UTC
(In reply to Mike Kaganski from comment #0)
> the result is correct 6. But drag-copying the formula down into D3:D6
> produces the column of 6s, while values must be different for each row (6,
> 24, 60, 120, 210). Hard recalc doesn't help, but minimal editing of the
> formula makes it work OK; also if you try to drag the wrong value 6 from,
> e.g., D3 into D4, the value in D4 would be 60; dragging the 6 from D3 into
> D4:D5 gives two 24s.

reproducible with:

Version: 6.5.0.0.alpha0+ (x64)
Build ID: d04eef858250f97690f32dba17f42d157a8767fc
CPU threads: 4; OS: Windows 10.0 Build 18363; UI render: default; VCL: win; 
Locale: de-DE (de_DE); UI-Language: en-US
Calc: threaded

btw: a save & reload will fix it too, even with [Never Calculate] on load enabled
Comment 2 Mike Kaganski 2019-11-20 19:44:51 UTC
Regression between 4.2.0.4 (works fine) and 4.3.0.4 (problem reproducible).
Comment 3 Terrence Enger 2019-11-21 15:44:32 UTC
Created attachment 156003 [details]
bibisect-42max, tail of terminal output

Contrary to comment 2, bibisect-linux-64-releases on debian-buster
shows the bug starting between libreoffice-4.1.6.2 and
libreoffice-4.2.0.0.beta1.  I wonder what Mike and I did differently.

Working in bibisect-42max on debian-buster shows the bug entering
LibreOffice:

      commit    s-h       date
      --------  --------  -------------------
good  7d71309c  a592b815  2013-08-12 23:46:28
bad   095d00d8  bbb2d8c6  2013-08-12 23:46:29

From git log:

    commit bbb2d8c660b515c98624a41a39ffe94d3a7ecc00
    Author: Kohei Yoshida <kohei.yoshida@gmail.com>
    Date:   Fri Aug 9 16:41:22 2013 -0400

        If the formula cell is grouped, update reference only on the top cell.
    
        Change-Id: I5e2e9db621a61deba39a46962e0ca877235d7c90

I am removing keyword bibisectRequest and adding bisected, bibisected.
Comment 4 Mike Kaganski 2019-11-21 16:18:01 UTC
(In reply to Terrence Enger from comment #3)
> I wonder what Mike and I did differently.

The only likely thing is that I tried on Windows: could it be that some engine (grouping?) had been enabled on Windows later than on Linux?

Thank you very much! Kohei, Eike: what do you think, does the patch need reverting (performance regressions?), or is some clever processing needed?
Comment 5 Mike Kaganski 2019-11-22 08:21:43 UTC
However, ScFormulaCell::UpdateReference doesn't seem to get called when drag-copying; so https://git.libreoffice.org/core/+/bbb2d8c660b515c98624a41a39ffe94d3a7ecc00 doesn't seem like a real problem here. Could the problem be in ScColumn::CloneFormulaCell?
Comment 6 QA Administrators 2021-11-22 04:01:48 UTC Comment hidden (obsolete)
Comment 7 Kohei Yoshida 2021-11-22 14:36:17 UTC
I'll leave that up to you, Mike (revert or not).  I personally don't see value in keeping this label feature since it was a feature copied from Excel but Excel itself dropped long ago (in 2007).  This was one feature that never meshed well with the rest of the formula engine code...  But that's just my selfish opinion.
Comment 8 Eike Rathke 2021-11-24 17:29:03 UTC
Add my selfish opinion to make that 2 ;p
Probably the worst feature in spreadsheet world Excel came up with ever..

Anyhow, the indicated commit is not the culprit, I think I know where it goes wrong.
Comment 9 Commit Notification 2021-11-24 21:42:44 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/89c4bc5220810dc3684a473f87d001f2c55438a1

Resolves: tdf#128914 Create copies for non-shareable token arrays

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Eike Rathke 2021-11-24 21:45:48 UTC
Mike was perfectly right with ScColumn::CloneFormulaCell()
Comment 11 Eike Rathke 2021-11-24 21:46:49 UTC
Pending review https://gerrit.libreoffice.org/c/core/+/125725 for 7-2
Comment 12 Commit Notification 2021-11-25 11:45:26 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/fabed8f7b9a5e57cbb62e6570349c33f000c2291

tdf#128914: sc_uicalc: Add unittest

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2021-11-25 12:36:12 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/2997c5c2604cd5054710ad35e1e0a9fd18f8ae79

Resolves: tdf#128914 Create copies for non-shareable token arrays

It will be available in 7.2.4.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Christian Lohmaier 2021-12-06 13:28:48 UTC
7.2.4 was a hotfix release, updating target in status-whiteboard