<Swing Dev> [11] Review Request: JDK-7007967 DefaultRowSorter: incorrect sorting due to not updating comparator cache

[Pankaj Bansal]

Hello Jay,

 

I have made suggested changes.

webrev: http://cr.openjdk.java.net/~pbansal/7007967/webrev.01/

 

 

<< Test case is working fine before and after the change.

<<I think there is no need for us to verify row index before we delete and update the <<contents again.

 << 65         for (int row = 0; row < model.getRowCount(); row++) {

 << 66             // values are in sorted ascending. so the row index and

 << 67             // view index from sorter should be same

 << 68             if (row != rowSorter.convertRowIndexToView(row)) {

 << 69                 throw new RuntimeException("Wrong sorting");

 << 70             }

 << 71         }

<<We can remove the above code in the test case because we never hit it and throw RuntimeException.

 

This is required to make sure that the row sorted is in proper condition and the test passes before making changes. This makes sure that our changes are not causing the error if there are any.

 

Regards,

Pankaj Bansal

 

 

From: Jayathirth D V 
Sent: Thursday, January 25, 2018 8:32 PM
To: Pankaj Bansal; swing-dev at openjdk.java.net; Sergey Bylokhov; Semyon Sadetsky
Subject: RE: <Swing Dev> [11] Review Request: JDK-7007967 DefaultRowSorter: incorrect sorting due to not updating comparator cache

 

Hello Pankaj,

 

Please find my inputs:

At line 1036 number of characters per line is more than 80, we can divide the conditions into multiple lines.

 

1036         if (!sorted || viewToModel.length == 0 || (lastRow - firstRow) > viewToModel.length / 10) {

1037             // We either weren't sorted, or to much changed, sort it all or

1038             // this is the first row added and we have to update different 

1039             // caches

 

Test case is working fine before and after the change.

I think there is no need for us to verify row index before we delete and update the contents again.

  65         for (int row = 0; row < model.getRowCount(); row++) {

  66             // values are in sorted ascending. so the row index and

  67             // view index from sorter should be same

  68             if (row != rowSorter.convertRowIndexToView(row)) {

  69                 throw new RuntimeException("Wrong sorting");

  70             }

  71         }

 

We can remove the above code in the test case because we never hit it and throw RuntimeException.

 

Thanks,

Jay

 

From: Pankaj Bansal 
Sent: Wednesday, January 17, 2018 5:24 PM
To: HYPERLINK "mailto:swing-dev at openjdk.java.net"swing-dev at openjdk.java.net; Sergey Bylokhov; Semyon Sadetsky
Subject: <Swing Dev> [11] Review Request: JDK-7007967 DefaultRowSorter: incorrect sorting due to not updating comparator cache

 

Hi All,

 

Please review the fix for JDK 11.

 

Bug:

https://bugs.openjdk.java.net/browse/JDK-7007967

 

Webrev:

http://cr.openjdk.java.net/~pbansal/7007967/webrev.00/

 

Issue:

The caching flags like useToString, comparators etc are not always updated in shouldOptimizeChange function. It can cause wrong cache to be used in some cases and produce wrong sorting results. 
 
For example, in the submitted test case, the integer data is added and sorted. This updates the useToString etc cache flags according to the data type provided.  //Sorts 1,2,11 as 1->2->11 (correct).
Then all the data is removed from the model and when the sorter is indicated of this, the cache flags get updated accordingly. Because of this, the useToString is set to true for the column and comparator is set to String Comparator and sorted flag is still true.
Now if Integer data rows are again added one by one, these flags are not updated and it will use the wrong caches of useToString, comparators etc and produce wrong sorting results. //Sorts 1,2,11 as 1->11->2 (Incorrect. The data is sorted according to String representation).

 

 

Fix:

There can be different approaches to solve this issue. In the current approach, changes have been done in shouldOptimizeChange function to call the sort function when first row is added to the model. This will ensure that the cache flags like useToString, comparators are updated according to the data added for the column.

 

Regards,

Pankaj Bansal

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20180129/15d36649/attachment.html>