<Swing Dev> [11] Review Request: JDK-7007967 DefaultRowSorter: incorrect sorting due to not updating comparator cache
Pankaj Bansal
pankaj.b.bansal at oracle.com
Mon Jan 29 09:00:58 UTC 2018
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>
More information about the swing-dev
mailing list