Skipping binary range search in ST if low doesn't exists

Signed-off-by: expani <[email protected]>
opensearch-project · Jan 23, 2025 · b85183d · b85183d
1 parent 516e3de
commit b85183d
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 3 deletions.
diff --git a/server/src/main/java/org/opensearch/search/startree/filter/RangeMatchDimFilter.java b/server/src/main/java/org/opensearch/search/startree/filter/RangeMatchDimFilter.java
@@ -30,6 +30,8 @@ public class RangeMatchDimFilter implements DimensionFilter {
     private Long lowOrdinal;
     private Long highOrdinal;
 
+    private boolean skipRangeCollection = false;
+
     public RangeMatchDimFilter(String dimensionName, Object low, Object high, boolean includeLow, boolean includeHigh) {
         this.dimensionName = dimensionName;
         this.low = low;
@@ -40,6 +42,7 @@ public RangeMatchDimFilter(String dimensionName, Object low, Object high, boolea
 
     @Override
     public void initialiseForSegment(StarTreeValues starTreeValues, SearchContext searchContext) {
+        skipRangeCollection = false;
         DimensionFilterMapper dimensionFilterMapper = DimensionFilterMapper.Factory.fromMappedFieldType(
             searchContext.mapperService().fieldType(dimensionName)
         );
@@ -51,6 +54,7 @@ public void initialiseForSegment(StarTreeValues starTreeValues, SearchContext se
                 lowOrdinal = lowOrdinalFound.get();
             } else { // This is only valid for Non-numeric fields.
                 lowOrdinal = highOrdinal = Long.MAX_VALUE;
+                skipRangeCollection = true;
                 return; // High can't be found since nothing >= low exists.
             }
         }
@@ -65,7 +69,7 @@ public void initialiseForSegment(StarTreeValues starTreeValues, SearchContext se
     @Override
     public void matchStarTreeNodes(StarTreeNode parentNode, StarTreeValues starTreeValues, StarTreeNodeCollector collector)
         throws IOException {
-        if (parentNode != null) {
+        if (parentNode != null && !skipRangeCollection) {
             parentNode.collectChildrenInRange(lowOrdinal, highOrdinal, collector);
         }
     }

diff --git a/...r/src/main/java/org/opensearch/search/startree/filter/provider/DimensionFilterMapper.java b/...r/src/main/java/org/opensearch/search/startree/filter/provider/DimensionFilterMapper.java
@@ -352,7 +352,9 @@ public Optional<Long> getMatchingOrdinal(
                         if (seekStatus == TermsEnum.SeekStatus.NOT_FOUND) {
                             long ordGreaterThanValue = termsEnum.ord();
                             // Checking if we are in bounds for satisfying LT
-                            return ((ordGreaterThanValue - 1) < sortedSetIterator.getValueCount()) ? Optional.of(ordGreaterThanValue - 1) : Optional.empty();
+                            return ((ordGreaterThanValue - 1) < sortedSetIterator.getValueCount())
+                                ? Optional.of(ordGreaterThanValue - 1)
+                                : Optional.empty();
                         } else {
                             return Optional.of(termsEnum.ord());
                         }
@@ -362,7 +364,7 @@ public Optional<Long> getMatchingOrdinal(
                 throw new RuntimeException(e);
             }
         } else {
-            throw new IllegalArgumentException("Unsupported star tree values iterator " + genericIterator.getClass().getName());
+            throw new IllegalStateException("Unsupported star tree values iterator " + genericIterator.getClass().getName());
         }
     }