Fix sort in case of different numeric field types between indices (#6926

) * Fixing index merge regression with different numeric field type while sorting shard results Signed-off-by: gashutos <[email protected]> * Fixing index merge regression with different numeric field type while sorting shard results Signed-off-by: gashutos <[email protected]> * Inheriting back from SortedNumericSortField Signed-off-by: gashutos <[email protected]> --------- Signed-off-by: gashutos <[email protected]>
opensearch-project · Apr 1, 2023 · c85d33e · c85d33e
1 parent f266204
commit c85d33e
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 12 deletions.
diff --git a/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java b/server/src/main/java/org/opensearch/action/search/SearchPhaseController.java
@@ -615,14 +615,13 @@ private static Sort createSort(TopFieldDocs[] topFieldDocs) {
         final SortField[] newFields = new SortField[firstTopDocFields.length];
 
         for (int i = 0; i < firstTopDocFields.length; i++) {
-            if (firstTopDocFields[i] instanceof SortedNumericSortField && isSortWideningRequired(topFieldDocs, i)) {
-                final SortedNumericSortField delegate = (SortedNumericSortField) firstTopDocFields[i];
-                newFields[i] = new SortedWiderNumericSortField(
-                    delegate.getField(),
-                    delegate.getNumericType(),
-                    delegate.getReverse(),
-                    delegate.getSelector()
-                );
+            final SortField delegate = firstTopDocFields[i];
+            final SortField.Type type = delegate instanceof SortedNumericSortField
+                ? ((SortedNumericSortField) delegate).getNumericType()
+                : delegate.getType();
+
+            if (SortedWiderNumericSortField.isTypeSupported(type) && isSortWideningRequired(topFieldDocs, i)) {
+                newFields[i] = new SortedWiderNumericSortField(delegate.getField(), type, delegate.getReverse());
             } else {
                 newFields[i] = firstTopDocFields[i];
             }

diff --git a/server/src/main/java/org/opensearch/search/sort/SortedWiderNumericSortField.java b/server/src/main/java/org/opensearch/search/sort/SortedWiderNumericSortField.java
@@ -16,7 +16,6 @@
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.FieldComparator;
 import org.apache.lucene.search.LeafFieldComparator;
-import org.apache.lucene.search.SortedNumericSelector;
 import org.apache.lucene.search.SortedNumericSortField;
 import org.apache.lucene.search.comparators.NumericComparator;
 
@@ -36,10 +35,9 @@ public class SortedWiderNumericSortField extends SortedNumericSortField {
      * @param field    Name of field to sort by. Must not be null.
      * @param type     Type of values
      * @param reverse  True if natural order should be reversed.
-     * @param selector custom selector type for choosing the sort value from the set.
      */
-    public SortedWiderNumericSortField(String field, Type type, boolean reverse, SortedNumericSelector.Type selector) {
-        super(field, type, reverse, selector);
+    public SortedWiderNumericSortField(String field, Type type, boolean reverse) {
+        super(field, type, reverse);
     }
 
     /**
@@ -84,4 +82,23 @@ public int compareValues(Number first, Number second) {
             }
         };
     }
+
+    /**
+     * The only below types would be considered for widening during merging topDocs results for sort,
+     * This will support indices having different Numeric types to be sorted together.
+     * @param type SortField.Type
+     * @return returns true if type is supported for widened numeric comparisons
+     */
+    public static boolean isTypeSupported(Type type) {
+        // Only below 4 numeric types supported as of now for widened merge
+        switch (type) {
+            case INT:
+            case LONG:
+            case FLOAT:
+            case DOUBLE:
+                return true;
+            default:
+                return false;
+        }
+    }
 }