Remove feature flag for ApproximatePointRangeQuery

CHANGELOG.md

@@ -62,6 +62,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Removed
 - Remove deprecated `batch_size` parameter from `_bulk` ([#14283](https://github.com/opensearch-project/OpenSearch/issues/14283))
+- Remove `FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY_SETTING` since range query approximation is no longer experimental ([#17769](https://github.com/opensearch-project/OpenSearch/pull/17769))
 
 ### Fixed
 - Fix bytes parameter on `_cat/recovery` ([#17598](https://github.com/opensearch-project/OpenSearch/pull/17598))

modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldTypeTests.java

@@ -51,6 +51,7 @@
 import org.opensearch.index.fielddata.IndexNumericFieldData;
 import org.opensearch.index.fielddata.LeafNumericFieldData;
 import org.opensearch.index.fielddata.SortedNumericDoubleValues;
+import org.opensearch.search.approximate.ApproximateScoreQuery;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -167,8 +168,8 @@ public void testRoundsLowerBoundCorrectly() {
     }
 
     private String getQueryString(Query query) {
-        assertTrue(query instanceof IndexOrDocValuesQuery);
-        return ((IndexOrDocValuesQuery) query).getIndexQuery().toString();
+        assertTrue(query instanceof ApproximateScoreQuery);
+        return ((IndexOrDocValuesQuery) ((ApproximateScoreQuery) query).getOriginalQuery()).getIndexQuery().toString();
     }
 
     public void testValueForSearch() {

server/src/main/java/org/opensearch/common/util/FeatureFlags.java

@@ -122,15 +122,6 @@ public class FeatureFlags {
         Property.NodeScope
     );
 
-    /**
-     * Gates the functionality of ApproximatePointRangeQuery where we approximate query results.
-     */
-    public static final String APPROXIMATE_POINT_RANGE_QUERY = FEATURE_FLAG_PREFIX + "approximate_point_range_query.enabled";
-    public static final Setting<Boolean> APPROXIMATE_POINT_RANGE_QUERY_SETTING = Setting.boolSetting(
-        APPROXIMATE_POINT_RANGE_QUERY,
-        false,
-        Property.NodeScope
-    );
     public static final String TERM_VERSION_PRECOMMIT_ENABLE = OS_EXPERIMENTAL_PREFIX + "optimization.termversion.precommit.enabled";
     public static final Setting<Boolean> TERM_VERSION_PRECOMMIT_ENABLE_SETTING = Setting.boolSetting(
         TERM_VERSION_PRECOMMIT_ENABLE,

server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java

@@ -486,23 +486,16 @@ public Query rangeQuery(
                     } else {
                         query = pointRangeQuery;
                     }
-                    if (FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY_SETTING)) {
-                        return new ApproximateScoreQuery(
-                            query,
-                            new ApproximatePointRangeQuery(
-                                name(),
-                                pack(new long[] { l }).bytes,
-                                pack(new long[] { u }).bytes,
-                                new long[] { l }.length
-                            ) {
-                                @Override
-                                protected String toString(int dimension, byte[] value) {
-                                    return Long.toString(LongPoint.decodeDimension(value, 0));
-                                }
-                            }
-                        );
-                    }
-                    return query;
+                    return new ApproximateScoreQuery(
+                        query,
+                        new ApproximatePointRangeQuery(
+                            name(),
+                            pack(new long[] { l }).bytes,
+                            pack(new long[] { u }).bytes,
+                            new long[] { l }.length,
+                            ApproximatePointRangeQuery.LONG_FORMAT
+                        )
+                    );
                 }
 
                 // Not searchable. Must have doc values.

server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java

@@ -69,6 +69,8 @@
 import org.opensearch.index.fielddata.plain.SortedNumericIndexFieldData;
 import org.opensearch.index.query.QueryShardContext;
 import org.opensearch.search.DocValueFormat;
+import org.opensearch.search.approximate.ApproximatePointRangeQuery;
+import org.opensearch.search.approximate.ApproximateScoreQuery;
 import org.opensearch.search.lookup.SearchLookup;
 import org.opensearch.search.query.BitmapDocValuesQuery;
 import org.opensearch.search.query.BitmapIndexQuery;
@@ -1058,23 +1060,34 @@
                 QueryShardContext context
             ) {
                 return longRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, (l, u) -> {
-                    if (isSearchable && hasDocValues) {
-                        Query query = LongPoint.newRangeQuery(field, l, u);
-                        Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery(field, l, u);
-                        query = new IndexOrDocValuesQuery(query, dvQuery);
-                        if (context.indexSortedOnField(field)) {
-                            query = new IndexSortSortedNumericDocValuesRangeQuery(field, l, u, query);
+                    Query dvQuery = hasDocValues ? SortedNumericDocValuesField.newSlowRangeQuery(field, l, u) : null;
+                    if (isSearchable) {
+                        Query pointRangeQuery = LongPoint.newRangeQuery(field, l, u);
+                        Query query;
+                        if (dvQuery != null) {
+                            query = new IndexOrDocValuesQuery(pointRangeQuery, dvQuery);
+                            if (context.indexSortedOnField(field)) {
+                                query = new IndexSortSortedNumericDocValuesRangeQuery(field, l, u, query);
+                            }
+                        } else {
+                            query = pointRangeQuery;
                         }
-                        return query;
+                        return new ApproximateScoreQuery(
+                            query,
+                            new ApproximatePointRangeQuery(
+                                field,
+                                LongPoint.pack(new long[] { l }).bytes,
+                                LongPoint.pack(new long[] { u }).bytes,
+                                new long[] { l }.length,
+                                ApproximatePointRangeQuery.LONG_FORMAT
+                            )
+                        );
+
                     }
-                    if (hasDocValues) {
-                        Query query = SortedNumericDocValuesField.newSlowRangeQuery(field, l, u);
-                        if (context.indexSortedOnField(field)) {
-                            query = new IndexSortSortedNumericDocValuesRangeQuery(field, l, u, query);
-                        }
-                        return query;
+                    if (context.indexSortedOnField(field)) {
+                        dvQuery = new IndexSortSortedNumericDocValuesRangeQuery(field, l, u, dvQuery);
                     }
-                    return LongPoint.newRangeQuery(field, l, u);
+                    return dvQuery;
 
                 });
             }

server/src/main/java/org/opensearch/index/search/NestedHelper.java

@@ -46,6 +46,7 @@
 import org.apache.lucene.search.TermQuery;
 import org.opensearch.index.mapper.MapperService;
 import org.opensearch.index.mapper.ObjectMapper;
+import org.opensearch.search.approximate.ApproximateScoreQuery;
 
 /** Utility class to filter parent and children clauses when building nested
  * queries.
@@ -85,6 +86,8 @@ public boolean mightMatchNestedDocs(Query query) {
             return mightMatchNestedDocs(((PointRangeQuery) query).getField());
         } else if (query instanceof IndexOrDocValuesQuery) {
             return mightMatchNestedDocs(((IndexOrDocValuesQuery) query).getIndexQuery());
+        } else if (query instanceof ApproximateScoreQuery) {
+            return mightMatchNestedDocs(((ApproximateScoreQuery) query).getOriginalQuery());
         } else if (query instanceof BooleanQuery) {
             final BooleanQuery bq = (BooleanQuery) query;
             final boolean hasRequiredClauses = bq.clauses().stream().anyMatch(BooleanClause::isRequired);
@@ -155,6 +158,8 @@ public boolean mightMatchNonNestedDocs(Query query, String nestedPath) {
             return mightMatchNonNestedDocs(((PointRangeQuery) query).getField(), nestedPath);
         } else if (query instanceof IndexOrDocValuesQuery) {
             return mightMatchNonNestedDocs(((IndexOrDocValuesQuery) query).getIndexQuery(), nestedPath);
+        } else if (query instanceof ApproximateScoreQuery) {
+            return mightMatchNonNestedDocs(((ApproximateScoreQuery) query).getOriginalQuery(), nestedPath);
         } else if (query instanceof BooleanQuery) {
             final BooleanQuery bq = (BooleanQuery) query;
             final boolean hasRequiredClauses = bq.clauses().stream().anyMatch(BooleanClause::isRequired);

server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java

@@ -8,6 +8,7 @@
 
 package org.opensearch.search.approximate;
 
+import org.apache.lucene.document.LongPoint;
 import org.apache.lucene.index.LeafReader;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.PointValues;
@@ -16,6 +17,7 @@
 import org.apache.lucene.search.DocIdSetIterator;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.PointRangeQuery;
+import org.apache.lucene.search.Query;
 import org.apache.lucene.search.QueryVisitor;
 import org.apache.lucene.search.ScoreMode;
 import org.apache.lucene.search.Scorer;
@@ -24,41 +26,51 @@
 import org.apache.lucene.util.ArrayUtil;
 import org.apache.lucene.util.DocIdSetBuilder;
 import org.apache.lucene.util.IntsRef;
-import org.opensearch.index.query.RangeQueryBuilder;
 import org.opensearch.search.internal.SearchContext;
 import org.opensearch.search.sort.FieldSortBuilder;
 import org.opensearch.search.sort.SortOrder;
 
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.Objects;
+import java.util.function.Function;
 
 /**
  * An approximate-able version of {@link PointRangeQuery}. It creates an instance of {@link PointRangeQuery} but short-circuits the intersect logic
  * after {@code size} is hit
  */
-public abstract class ApproximatePointRangeQuery extends ApproximateQuery {
+public class ApproximatePointRangeQuery extends ApproximateQuery {
+    public static final Function<byte[], String> LONG_FORMAT = bytes -> Long.toString(LongPoint.decodeDimension(bytes, 0));
     private int size;
 
     private SortOrder sortOrder;
 
     public final PointRangeQuery pointRangeQuery;
 
-    protected ApproximatePointRangeQuery(String field, byte[] lowerPoint, byte[] upperPoint, int numDims) {
-        this(field, lowerPoint, upperPoint, numDims, 10_000, null);
+    public ApproximatePointRangeQuery(
+        String field,
+        byte[] lowerPoint,
+        byte[] upperPoint,
+        int numDims,
+        Function<byte[], String> valueToString
+    ) {
+        this(field, lowerPoint, upperPoint, numDims, SearchContext.DEFAULT_TRACK_TOTAL_HITS_UP_TO, null, valueToString);
     }
 
-    protected ApproximatePointRangeQuery(String field, byte[] lowerPoint, byte[] upperPoint, int numDims, int size) {
-        this(field, lowerPoint, upperPoint, numDims, size, null);
-    }
-
-    protected ApproximatePointRangeQuery(String field, byte[] lowerPoint, byte[] upperPoint, int numDims, int size, SortOrder sortOrder) {
+    protected ApproximatePointRangeQuery(
+        String field,
+        byte[] lowerPoint,
+        byte[] upperPoint,
+        int numDims,
+        int size,
+        SortOrder sortOrder,
+        Function<byte[], String> valueToString
+    ) {
         this.size = size;
         this.sortOrder = sortOrder;
         this.pointRangeQuery = new PointRangeQuery(field, lowerPoint, upperPoint, numDims) {
             @Override
             protected String toString(int dimension, byte[] value) {
-                return super.toString(field);
+                return valueToString.apply(value);
             }
         };
     }
@@ -79,6 +91,11 @@
         this.sortOrder = sortOrder;
     }
 
+    @Override
+    public Query rewrite(IndexSearcher indexSearcher) throws IOException {
+        return super.rewrite(indexSearcher);
+    }
+
     @Override
     public void visit(QueryVisitor visitor) {
         pointRangeQuery.visit(visitor);
@@ -344,7 +361,6 @@
                 if (checkValidPointValues(values) == false) {
                     return null;
                 }
-                final Weight weight = this;
                 if (size > values.size()) {
                     return pointRangeQueryWeight.scorerSupplier(context);
                 } else {
@@ -426,17 +442,26 @@
         }
         // size 0 could be set for caching
         if (context.from() + context.size() == 0) {
-            this.setSize(10_000);
+            this.setSize(SearchContext.DEFAULT_TRACK_TOTAL_HITS_UP_TO);
+        } else {
+            this.setSize(Math.max(context.from() + context.size(), context.trackTotalHitsUpTo()));
         }
-        this.setSize(Math.max(context.from() + context.size(), context.trackTotalHitsUpTo()));
         if (context.request() != null && context.request().source() != null) {
             FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(context.request().source());
-            if (primarySortField != null
-                && primarySortField.missing() == null
-                && primarySortField.getFieldName().equals(((RangeQueryBuilder) context.request().source().query()).fieldName())) {
-                if (primarySortField.order() == SortOrder.DESC) {
-                    this.setSortOrder(SortOrder.DESC);
+            if (primarySortField != null) {
+                if (!primarySortField.fieldName().equals(pointRangeQuery.getField())) {
+                    return false;
+                }
+                if (primarySortField.missing() != null) {
+                    // Cannot sort documents missing this field.
+                    return false;
                 }
+                if (context.request().source().searchAfter() != null) {
+                    // TODO: We *could* optimize searchAfter, especially when this is the only sort field, but existing pruning is pretty
+                    // good.
+                    return false;
+                }
+                this.setSortOrder(primarySortField.order());
             }
         }
         return true;
@@ -453,56 +478,16 @@
     }
 
     private boolean equalsTo(ApproximatePointRangeQuery other) {
-        return Objects.equals(pointRangeQuery.getField(), other.pointRangeQuery.getField())
-            && pointRangeQuery.getNumDims() == other.pointRangeQuery.getNumDims()
-            && pointRangeQuery.getBytesPerDim() == other.pointRangeQuery.getBytesPerDim()
-            && Arrays.equals(pointRangeQuery.getLowerPoint(), other.pointRangeQuery.getLowerPoint())
-            && Arrays.equals(pointRangeQuery.getUpperPoint(), other.pointRangeQuery.getUpperPoint());
+        return Objects.equals(pointRangeQuery, other.pointRangeQuery);
     }
 
     @Override
     public final String toString(String field) {
         final StringBuilder sb = new StringBuilder();
-        if (pointRangeQuery.getField().equals(field) == false) {
-            sb.append(pointRangeQuery.getField());
-            sb.append(':');
-        }
-
-        // print ourselves as "range per dimension"
-        for (int i = 0; i < pointRangeQuery.getNumDims(); i++) {
-            if (i > 0) {
-                sb.append(',');
-            }
-
-            int startOffset = pointRangeQuery.getBytesPerDim() * i;
-
-            sb.append('[');
-            sb.append(
-                toString(
-                    i,
-                    ArrayUtil.copyOfSubArray(pointRangeQuery.getLowerPoint(), startOffset, startOffset + pointRangeQuery.getBytesPerDim())
-                )
-            );
-            sb.append(" TO ");
-            sb.append(
-                toString(
-                    i,
-                    ArrayUtil.copyOfSubArray(pointRangeQuery.getUpperPoint(), startOffset, startOffset + pointRangeQuery.getBytesPerDim())
-                )
-            );
-            sb.append(']');
-        }
+        sb.append("Approximate(");
+        sb.append(pointRangeQuery.toString());
+        sb.append(")");
 
         return sb.toString();
     }
-
-    /**
-     * Returns a string of a single value in a human-readable format for debugging. This is used by
-     * {@link #toString()}.
-     *
-     * @param dimension dimension of the particular value
-     * @param value     single value, never null
-     * @return human readable value for debugging
-     */
-    protected abstract String toString(int dimension, byte[] value);
 }

server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java

@@ -42,9 +42,10 @@ public ApproximateQuery getApproximationQuery() {
     }
 
     @Override
-    public final Query rewrite(IndexSearcher indexSearcher) throws IOException {
+    public Query rewrite(IndexSearcher indexSearcher) throws IOException {
         if (resolvedQuery == null) {
-            throw new IllegalStateException("Cannot rewrite resolved query without setContext being called");
+            // Default to the original query. This suggests that we were not called from ContextIndexSearcher.
+            return originalQuery.rewrite(indexSearcher);
         }
         return resolvedQuery.rewrite(indexSearcher);
     }

server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java

@@ -191,6 +191,9 @@ public void setAggregatedDfs(AggregatedDfs aggregatedDfs) {
 
     @Override
     public Query rewrite(Query original) throws IOException {
+        if (original instanceof ApproximateScoreQuery) {
+            ((ApproximateScoreQuery) original).setContext(searchContext);
+        }
         if (profiler != null) {
             profiler.startRewriteTime();
         }
@@ -221,9 +224,6 @@ public Weight createWeight(Query query, ScoreMode scoreMode, float boost) throws
                 profiler.pollLastElement();
             }
             return new ProfileWeight(query, weight, profile);
-        } else if (query instanceof ApproximateScoreQuery) {
-            ((ApproximateScoreQuery) query).setContext(searchContext);
-            return super.createWeight(query, scoreMode, boost);
         } else {
             return super.createWeight(query, scoreMode, boost);
         }