Bug 57465 - Calc sorts columns incorrectly
Summary: Calc sorts columns incorrectly
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
3.6.0.0.beta1
Hardware: All All
: medium normal
Assignee: Markus Mohrhard
URL:
Whiteboard: BSA target:4.1.0 target:4.0.0.2 targe...
Keywords: regression
: 57797 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-11-23 21:34 UTC by war
Modified: 2021-02-18 14:14 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Testfile to demonstrate behaviour (10.66 KB, application/vnd.oasis.opendocument.spreadsheet)
2012-11-23 21:34 UTC, war
Details

Note You need to log in before you can comment on or make changes to this bug.
Description war 2012-11-23 21:34:54 UTC
Created attachment 70488 [details]
Testfile to demonstrate behaviour

Problem description: 
Calc sorts columns the wrong way

Steps to reproduce:
There is an error in sorting columns in calc.

How to test:
Create a spread sheet similar to this

   C A D B F E
R1   a d
R2       b   e
R3 c       f

Mark the columns labled C, A...E
Sort the columns ascending

Expected behavior:

   A B C D E F
R1 a     d
R2   b     e
R3     c     f

Current behavior:

   C A D B F E
R1 c       f
R2   a d
R3       b   e


Platform (if different from the browser): 
Macbook Pro 6,2, Ubuntu 12.10

Browser: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:17.0) Gecko/17.0 Firefox/17.0
Comment 1 war 2012-11-23 21:51:01 UTC
This does not happen in Calc 3.5.4.2
Comment 2 Urmas 2012-11-23 22:00:05 UTC
Confirmed in 4.0-master, though the result is ADCBFE.
Comment 3 Markus Mohrhard 2012-11-23 22:22:01 UTC
Can you please test with 3.6.4.1 or 3.6.4.2 because they should be fixed there.
Comment 4 war 2012-11-24 14:50:23 UTC
@Markus Mohrhard:
I'd love to whenever the update is downstreamed into Ubuntu repositories.

Q: How can such a regression happen? Any ideas or explanations?
Comment 5 Markus Mohrhard 2012-11-24 17:14:16 UTC
(In reply to comment #4)
> @Markus Mohrhard:
> I'd love to whenever the update is downstreamed into Ubuntu repositories.
> 
> Q: How can such a regression happen? Any ideas or explanations?

Calc has about 1 million lines of own source code + several more million lines of shared source code. And how much time did you spend yourself testing master to check for regressions.

Marking as fixed as I think it is fixed in 3.6.4.1 and therefore in the 3.6.4 release. Reopen if it is not fixed there.
Comment 6 Chris Peñalver 2012-11-24 18:29:53 UTC
Reproducible in:
Microsoft Windows Vista Business x86
6.0.6002 Service Pack 2 Build 6002
Version 3.6.4.1 (Build ID: a9a0717)
Comment 7 war 2012-11-25 17:50:29 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > @Markus Mohrhard:
> > I'd love to whenever the update is downstreamed into Ubuntu repositories.
> > 
> > Q: How can such a regression happen? Any ideas or explanations?
> 
> Calc has about 1 million lines of own source code + several more million
> lines of shared source code. And how much time did you spend yourself
> testing master to check for regressions.
> 
> Marking as fixed as I think it is fixed in 3.6.4.1 and therefore in the
> 3.6.4 release. Reopen if it is not fixed there.

First I am pretty sure that not all of the 1 million lines deal with sorting. Second, in my development projects we used test suites to prevent from regression and third there are statistical methods to identify areas of unstable code based on previous bug reports.

Assuming that all this is in place here as well, I was wondering what could be the cause of those regressions. If you had an idea I could direct some research effort into how this can be improved. Maybe we can discuss this on a different channel (mail?)
Comment 8 Markus Mohrhard 2012-11-27 15:06:15 UTC
(In reply to comment #7)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > @Markus Mohrhard:
> > > I'd love to whenever the update is downstreamed into Ubuntu repositories.
> > > 
> > > Q: How can such a regression happen? Any ideas or explanations?
> > 
> > Calc has about 1 million lines of own source code + several more million
> > lines of shared source code. And how much time did you spend yourself
> > testing master to check for regressions.
> > 
> > Marking as fixed as I think it is fixed in 3.6.4.1 and therefore in the
> > 3.6.4 release. Reopen if it is not fixed there.
> 
> First I am pretty sure that not all of the 1 million lines deal with
> sorting. Second, in my development projects we used test suites to prevent
> from regression and third there are statistical methods to identify areas of
> unstable code based on previous bug reports.

We have highly coupled code with to less test code. Sadly our code is so old and written by so many people that even simple tasks like sorting affect more places than it should. For example sorting is highly integrated into calc core in ScTable so that a change to fix another bug can introduce a new one in parts that nobody expects.

We are writing new tests for fixed bugs but since we are mainly only three developers doing all the work on calc this means that there are thousand of untested cases. However we are always happy if someone helps us out with extending the test cases which is actually not that difficult.

> 
> Assuming that all this is in place here as well, I was wondering what could
> be the cause of those regressions. If you had an idea I could direct some
> research effort into how this can be improved. Maybe we can discuss this on
> a different channel (mail?)

We were removing some limitations for sorting in 3.6 and therefore had to change some internal data structure and change the UI code. This was done by a volunteer and sadly not enough tests were written at the time. I would be very glad if you would be intersted in helping with that.
Comment 9 war 2012-12-03 20:26:20 UTC
Hello Markus

I tried the same sorting on a version 3.6.4.3 downloaded from the web site and installed on a plain machine.

The sorting does not work here either.

Colums are sorted: ADCBFE

I did not have the chance to get a look into the sources. So I'm afraid I'm of little help here.
Comment 10 PeterR 2012-12-14 17:50:38 UTC
I was trying to sort columns in ascending/descending order by the row of totals at the bottom of the columns.  I'm not sure what order LibreOffice 3.6.4.3 is sorting but it's certainly not numerical.  

OpenOffice.org works fine for this. I currently have 3.3.0 installed because of an unrelated bug in 3.4.1.

This issue apparently goes back at least as far as LibreOffice 3.6.0.2. - see http://ask.libreoffice.org/en/question/5898/libreoffice-calc-sorts-numerical-data-as-text/

LibreOffice sorts vertically just fine.

I'm on Windows 7 Home Premium (64 bit).
Comment 11 war 2012-12-15 12:16:32 UTC
I'v tried LOdev 4.0.

The same problem applies there as well. However, resorting immediately after the first sort produces the correct results.
Comment 12 Not Assigned 2013-01-18 09:28:31 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "master":

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

reset one of the sort containers before refilling, fdo#57465



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 13 Not Assigned 2013-01-18 23:30:29 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

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

reset one of the sort containers before refilling, fdo#57465


It will be available in LibreOffice 4.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 14 Not Assigned 2013-01-19 00:14:26 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "libreoffice-3-6":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c3f7589fb540072f3a2434f6de99fe2472dad3b1&h=libreoffice-3-6

reset one of the sort containers before refilling, fdo#57465


It will be available in LibreOffice 3.6.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 15 Not Assigned 2013-01-21 17:49:02 UTC
Markus Mohrhard committed a patch related to this issue.
It has been pushed to "libreoffice-3-6-5":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=238f6797e47a5029ad8bd5df3dfc36bbeff6532c&h=libreoffice-3-6-5

reset one of the sort containers before refilling, fdo#57465


It will be available already in LibreOffice 3.6.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 16 Joel Madero 2013-04-26 01:34:31 UTC
As part of regular FDO cleanup we are checking the version of regressions to see if it's the oldest version that displays the incorrect behavior.

In this case I have been able to verify the bug existence in 
Version 3.6.0.0.beta1 (Build ID: 1f1cdd)

Changing version to reflect this
Comment 17 Joel Madero 2013-04-26 03:29:58 UTC
*** Bug 57797 has been marked as a duplicate of this bug. ***
Comment 18 Commit Notification 2021-02-18 14:14:32 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

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

tdf#57465: sc: Add UItest

It will be available in 7.2.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.