[zeppelin] branch master updated: InterpreterSetting: don't crash when sorting settings.

[zj...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new 8a7e0b8  InterpreterSetting: don't crash when sorting settings.
8a7e0b8 is described below

commit 8a7e0b87144fba81a22055684441906ad1c327af
Author: Vladimir Prus <vl...@gmail.com>
AuthorDate: Thu Mar 11 10:53:00 2021 +0000

    InterpreterSetting: don't crash when sorting settings.
    
    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 <vl...@gmail.com>
    
    Closes #4075 from vprus/interpreter-settings-sorting-v2 and squashes the following commits:
    
    7cd0b6485 [Vladimir Prus] Cosmetics
    47e129a31 [Vladimir Prus] Remove spurious change
    0cbc70fbd [Vladimir Prus] Cosmetrics.
    e032d9990 [Vladimir Prus] InterpreterSetting: don't crash when sorting settings.
---
 .../zeppelin/interpreter/InterpreterSetting.java       | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
index e34fbd1..2cef777 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
@@ -590,19 +590,13 @@ public class InterpreterSetting {
           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);
           }
         });