Skip to content

Commit

Permalink
InterpreterSetting: don't crash when sorting settings.
Browse files Browse the repository at this point in the history
The existing comparator would return -1 for both (a, b) and (b, a),
if both of these settings keys are not present in the template,
and such behaviour violates required comparator properties.

The issue manifests with exception in the form

  java.lang.IllegalArgumentException: Comparison method violates its general contract!
  at java.util.TimSort.mergeLo(TimSort.java:777)

and prevents server from starting up.

### What type of PR is it?
Bug Fix

### Questions:
* Does the licenses files need update? I don't think, the change is trivial.
* Is there breaking changes for older versions? No.
* Does this needs documentation? No.

Author: Vladimir Prus <[email protected]>

Closes apache#4075 from vprus/interpreter-settings-sorting-v2 and squashes the following commits:

7cd0b64 [Vladimir Prus] Cosmetics
47e129a [Vladimir Prus] Remove spurious change
0cbc70f [Vladimir Prus] Cosmetrics.
e032d99 [Vladimir Prus] InterpreterSetting: don't crash when sorting settings.
  • Loading branch information
vprus authored and zjffdu committed Mar 15, 2021
1 parent 8528170 commit 8a7e0b8
Showing 1 changed file with 6 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -590,19 +590,13 @@ public void sortPropertiesByTemplate(Object propertiesInTemplate) {
int i1 = sortedKeys.indexOf(o1);
int i2 = sortedKeys.indexOf(o2);
if (i1 != -1 && i2 != -1) {
if (i1 < i2) {
return -1;
} else if (i1 > i2) {
return 1;
} else {
return 0;
}
// If both are present in the template, use natural order of indexes
return (i1 - i2);
} else {
if (i1 == -1) {
return 1;
} else {
return -1;
}
// If one, or both are not in the template, use reverse order, so that missing
// elements are placed at the end. Note that if both are missing, we return 0
// to full the contract of comparison function.
return (i2 - i1);
}
});

Expand Down

0 comments on commit 8a7e0b8

Please sign in to comment.