Strlen sort spec ignores dimension

processing/src/main/java/io/druid/query/search/search/StrlenSearchSortSpec.java

@@ -19,6 +19,8 @@
 
 package io.druid.query.search.search;
 
+import com.google.common.primitives.Ints;
+
 import java.util.Comparator;
 
 /**
@@ -36,9 +38,14 @@ public class StrlenSearchSortSpec implements SearchSortSpec
       @Override
       public int compare(SearchHit s, SearchHit s1)
       {
-        int res = s.getValue().length() - s1.getValue().length();
+        final String v1 = s.getValue();
+        final String v2 = s1.getValue();
+        int res = Ints.compare(v1.length(), v2.length());
         if (res == 0) {
-          return (s.getValue().compareTo(s1.getValue()));
+          res = v1.compareTo(v2);
+        }
+        if (res == 0) {
+          res = s.getDimension().compareTo(s1.getDimension());
         }
         return res;
       }

processing/src/test/java/io/druid/query/search/SearchBinaryFnTest.java

@@ -29,7 +29,9 @@ import org.joda.time.DateTime;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.List;
 
 /**
  */
@@ -220,42 +222,17 @@ public class SearchBinaryFnTest
   {
     Result<SearchResultValue> r1 = new Result<SearchResultValue>(
         currTime,
-        new SearchResultValue(
+        new SearchResultValue(toHits("blah:thisislong"))
-            ImmutableList.<SearchHit>of(
-                new SearchHit(
-                    "blah",
-                    "thisislong"
-                )
-            )
-        )
     );
 
     Result<SearchResultValue> r2 = new Result<SearchResultValue>(
         currTime,
-        new SearchResultValue(
+        new SearchResultValue(toHits("blah:short"))
-            ImmutableList.<SearchHit>of(
-                new SearchHit(
-                    "blah",
-                    "short"
-                )
-            )
-        )
     );
 
     Result<SearchResultValue> expected = new Result<SearchResultValue>(
         currTime,
-        new SearchResultValue(
+        new SearchResultValue(toHits("blah:short", "blah:thisislong"))
-            ImmutableList.<SearchHit>of(
-                new SearchHit(
-                    "blah",
-                    "short"
-                ),
-                new SearchHit(
-                    "blah",
-                    "thisislong"
-                )
-            )
-        )
     );
 
     Result<SearchResultValue> actual = new SearchBinaryFn(new StrlenSearchSortSpec(), QueryGranularity.ALL, Integer.MAX_VALUE).apply(r1, r2);
@@ -263,6 +240,39 @@ public class SearchBinaryFnTest
     assertSearchMergeResult(expected.getValue(), actual.getValue());
   }
 
+  @Test
+  public void testStrlenMerge2()
+  {
+    Result<SearchResultValue> r1 = new Result<SearchResultValue>(
+        currTime,
+        new SearchResultValue(toHits("blah:thisislong", "blah:short", "blah2:thisislong"))
+    );
+
+    Result<SearchResultValue> r2 = new Result<SearchResultValue>(
+        currTime,
+        new SearchResultValue(toHits("blah:short", "blah2:thisislong"))
+    );
+
+    Result<SearchResultValue> expected = new Result<SearchResultValue>(
+        currTime,
+        new SearchResultValue(toHits("blah:short", "blah:thisislong", "blah2:thisislong"))
+    );
+
+    Result<SearchResultValue> actual = new SearchBinaryFn(new StrlenSearchSortSpec(), QueryGranularity.ALL, Integer.MAX_VALUE).apply(r1, r2);
+    Assert.assertEquals(expected.getTimestamp(), actual.getTimestamp());
+    System.out.println("[SearchBinaryFnTest/testStrlenMerge2] " + actual.getValue());
+    assertSearchMergeResult(expected.getValue(), actual.getValue());
+  }
+
+  private List<SearchHit> toHits(String... hits) {
+    List<SearchHit> result = new ArrayList<>();
+    for (String hit : hits) {
+      int index = hit.indexOf(':');
+      result.add(new SearchHit(hit.substring(0, index), hit.substring(index + 1)));
+    }
+    return result;
+  }
+
   @Test
   public void testMergeUniqueResults()
   {